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

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 8 inline comments as done.
abrachet added a comment.

I've been using `::testing::ElementsAre(...)` against a vector of `StringRef` a lot, when it fails the output is really bad. Like this: `element #1 is equal to { '.' (46, 0x2E), 's' (115, 0x73), 'e' (101, 0x65), 'c' (99, 0x63), '0' (48, 0x30) },` Is there a way to provide formatting for types to gtest? Or should I just be using `const char *`?



================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:28
+  if (MatchLen)
+    if ((size_t)std::distance(Begin, End) != List.size())
+      return false;
----------------
jhenderson wrote:
> labath wrote:
> > abrachet wrote:
> > > jhenderson wrote:
> > > > `std::distance` already returns a size_t. No need to cast it.
> > > I thought so too but it returns the iterators `difference_type` which on most containers is `ptrdiff_t`. That's a really strange decision by the committee I think, I don't see the rational of not using `size_t` given `distance` will never be negative.
> > For random-access iterators, it is valid to plug the begin and end iterators into std::distance in reverse order and get a negative value in return (but only since c++11).
> You're both right. My bad.
Wow I never knew that


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:150-166
 
   auto Iter = MutableObject.section_begin();
   compareSectionName(Iter, nullptr);
 
   compareSectionName(++Iter, ".sec0");
   compareSectionName(++Iter, ".sec1");
   compareSectionName(++Iter, ".sec2");
----------------
Should I just remove all of this?


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

https://reviews.llvm.org/D65633