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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 6 11:54:57 PST 2016


ruiu marked 2 inline comments as done.

================
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
----------------
grimar wrote:
> 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 ?
Yeah, there's a confusion on the naming scheme here. The root problem is that we split a symbol into Symbol and SymbolBody, but we often just say "symbol" to talk about the latter. getSymbol() is the right name because it's a member function of SymbolBody to retrieve a Symbol for that SymbolBody.

I have no clear idea about what to do here. We might want to change setBackref to setSymbol, but I'm not sure if it helps improve readability. I'd like to keep it as is for now until we find a better name.


http://reviews.llvm.org/D15896





More information about the llvm-commits mailing list