[PATCH] D50569: Change how we handle -wrap.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 21 02:47:03 PDT 2018


ruiu added inline comments.


================
Comment at: lld/ELF/InputFiles.h:89
   // function on files of other types.
-  ArrayRef<Symbol *> getSymbols() {
+  std::vector<Symbol *> &getSymbols() {
     assert(FileKind == BinaryKind || FileKind == ObjKind ||
----------------
ikudrin wrote:
> Don't you consider to add a method like `void remapSymbols(const DenseMap<Symbol *, Symbol *>&)` instead of this change?
> I believe that granting the access to a protected member through a `get` method makes the behavior a bit unexpected. Note, that the original `getSymbols` method might be declared as `const`.
I want to write a logic in one place rather than scatter them as `remapSymbols`, but I can see your concern, so I made a new function `getMutableVector` so that it is explicit that the function returns a mutable symbol array.


================
Comment at: lld/ELF/SymbolTable.cpp:162
+  Idx2 = Idx1;
+  Idx1 = Idx3;
 
----------------
grimar wrote:
> It is technically correct I think, but no tests are failing without these 2 lines.
> Can it be tested?
Done.


https://reviews.llvm.org/D50569





More information about the llvm-commits mailing list