[PATCH] D113228: [RFC][ELF] Refactor relocation processing

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Relocations.cpp:224
 // dynamic relocation so that the relocation will be fixed at load-time.
 static bool isStaticLinkTimeConstant(RelExpr e, RelType type, const Symbol &sym,
                                      InputSectionBase &s, uint64_t relOff) {
----------------
peter.smith wrote:
> Is it worth moving closer to the only caller processRelocAux. Perhaps worth emphasisng that this only handles relocations that are expected in processRelocaAux. Could do with a comment or perhaps renaming to something like `customRelocIsStaticLinkTimeConstant`
> 
> If there were a way to know if a particular expr code could be CUSTOM outside of scanReloc then we could assert that which would make it clear, but I'm not sure if there is an easily maintainable way to do that.
Thanks for the suggestion. I just pushed the isStaticLinkTimeConstant move and comment adding separately.


================
Comment at: lld/ELF/Relocations.cpp:952
 
+static void cantUseRelocation(InputSectionBase &sec, uint64_t offset,
+                              RelType type, Symbol &sym) {
----------------
peter.smith wrote:
> suggest `cantUseRelocationAgainstLocal` or `invalidRelocToLocal`
Used `invalidRelocAgainstSym`.

I was thinking of extracting this out as a separate function to be shared, but currently it is only used once. I may inline it again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113228