[PATCH] D15896: ELF: Implement --wrap.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 6 06:35:12 PST 2016


grimar added inline comments.

================
Comment at: ELF/Options.td:114
@@ -112,1 +113,3 @@
+  HelpText<"Use a wrapper function for symbol">;
+
 def z : JoinedOrSeparate<["-"], "z">, MetaVarName<"<option>">,
----------------
gold and bfd help descriptions says "Use wrapper function**s** for SYMBOL".
Thats probably not just consistent but also a bit closer to what flag do since there are 2 wrappers used: __real_XXX and __wrap_XXX, not just only one

================
Comment at: ELF/Symbols.h:109
@@ -108,2 +108,3 @@
+  Symbol *getSymbol() { return Backref; }
 
   // Decides which symbol should "win" in the symbol table, this or
----------------
Not sure here about naming. We already have setter named:
```
 void setBackref(Symbol *P) { Backref = P; }
```
but your getter is named getSymbol. May be would be better to name them consistently (setBackref,getBackref or setSymbol,getSymbol) or even just do Backref member to be public and remove getter and setter ?

================
Comment at: test/ELF/wrap.s:5
@@ +4,3 @@
+
+// RUN: ld.lld -o %t3 %t %t2 -wrap foo -wrap nosuchsym
+// RUN: llvm-objdump -d -print-imm-hex %t3 | FileCheck %s
----------------
What about also testing alias and --wrap forms ?


http://reviews.llvm.org/D15896





More information about the llvm-commits mailing list