[PATCH] D73254: [ELF] Rename relocateOne() to relocate() and pass `Relocation` to it

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: lld/ELF/Target.h:91
+    relocate(loc, rel, val);
+  }
 
----------------
nickdesaulniers wrote:
> It looks like there's just a few more callers of `relocateOne` in:
> * ELF/AArch64ErrataFix.cpp
> * ELF/ARMErrataFix.cpp
> * ELF/InputSection.cpp
> 
> I think if you updated those too, you could drop this change to `relocateOne` and then this patch would be simply renaming `relocateOne` to `relocate` and taking a full `Relocation` object rather than a `RelType`.
> 
> You might even give `struct Relocation` an non-`explicit` constructor that way you could implicitly construct a `Relocation` via a `RelType`.  Then calls to `relocateOne` with a literal `RelType` would only have to be renamed, and you wouldn't have to add code to contruct `Relocations` at callsites.
> 
> (It looks like `struct Relocation` in llvm/tools/llvm-objcopy/COFF/Object.h already does something to this effect, for prior art). Oh man, so many different `Relocation` structs across the codebase...
> I think if you updated those too, you could drop this change to relocateOne and then this patch would be simply renaming relocateOne to relocate and taking a full Relocation object rather than a RelType.

As @peter.smith explained above, " there are quite a few callsites that are in files unaffected by this diff or in the context, for example Thunks, PLT sequences".

We can probably pass `Symbol *` to relocateOne() in Thunks.cpp, but some call sites like writePltHeader() there is no meaningful `Symbol *`, so we will have to use `nullptr`.

An alternative signature is:

`void relocate(uint8_t *loc, RelType type, uint64_t val, const Symbol *sym = nullptr) const;`

I considered it but still preferred the current one:

`void relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const;`

because the signature will be consistent with relaxGot() and relaxTls* (https://reviews.llvm.org/D73250). We do need to be careful and not use `Relocation` members other than `type` and `sym`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73254