[PATCH] D91533: [lib/Object] - Generalize the RelocationResolver API.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/RelocationResolver.cpp:757
+  // contains the addend, provided by the caller.
+  return Resolver(/*Type=*/0, /*Offset=*/0, S, LocData,
+                  R.getRawDataRefImpl().p);
----------------
grimar wrote:
> jhenderson wrote:
> > Is it simply not possible to provide Type and Offset information here? Otherwise, this code seems a little fragile to LLD deciding to try to use those fields for something.
> Yes, it is not possible without an owning object :(
> 
> ```
> inline uint64_t RelocationRef::getOffset() const {
>   return OwningObject->getRelocationOffset(RelocationPimpl);
> }
> 
> inline uint64_t RelocationRef::getType() const {
>   return OwningObject->getRelocationType(RelocationPimpl);
> }
> ```
I think we need some sort of note added to the LLD resolver then, indicating that the values will always be 0 then (possibly could be an assertion?), because the information isn't available. That would at least hint to future developers to not try to use them.


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

https://reviews.llvm.org/D91533