[PATCH] D65633: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 3]

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 20 inline comments as done.
abrachet added inline comments.


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:420
+    Value:    0
+    Binding:  STB_GLOBAL)");
+
----------------
jhenderson wrote:
> Not sure, but it might be interesting to add some dynamic symbols and show that iterating over those isn't affected by the regular table changes.
Wow! Great catch! There was actually a bug here with my `moveSymbolNext` not differentiating between the two symbol tables. Thanks :)


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:435
+  EXPECT_EQ(*NameOrErr, "first");
+  MutableObject.removeSymbol(FirstSym);
+
----------------
jhenderson wrote:
> I think it would be interesting to see what happens when you remove the second symbol too, leaving only the null entry. I also think it would be interesting to test what happens if you try to remove the null entry (I think this should probably be an error).
I haven't done this or the similar comment for sections yet. Do you think the `removeSymbol` method should return `Error` or should this just be an `assert`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65633/new/

https://reviews.llvm.org/D65633