[PATCH] D64281: [Object] Create MutableObject class for doing Object Mutations [Part 1]

[Alex Brachet via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:47
+
+  using MappingType =
+      typename MutableRange<MutableELFSection<ELFT>>::MappingType;
----------------
jhenderson wrote:
> Do you anticipate this being used in other future functions within this class? If not, move it inside the function, to reduce its effect to where it's actually used.
Yes all of the getSectionX methods will need this type. Those haven't been implemented in this patch because I didn't want to overload this patch. But calling those methods for an updated section is undefined behavior. I'm not sure if that means it needs to be in this patch or if it makes sense to move it to the part 2.


================
Comment at: llvm/include/llvm/Object/MutableObject.h:47
+        [&Extract](typename Iter::value_type Value) -> MappingType {
+          auto Extracted = Extract(Value);
+          assert(Extracted <= (UINT64_MAX >> 1) && "returned type too large");
----------------
jhenderson wrote:
> Another instance of too much auto.
I really don't know the return type of Extractors::operator(), though. I could just make it a function_ref returning a uintptr though I suppose.


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

https://reviews.llvm.org/D64281