[PATCH] D62117: Make replaceSymbol a member function of Symbol.

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.
Herald added subscribers: ormris, hiraditya.


================
Comment at: lld/trunk/ELF/Symbols.h:86
 
-  // Symbol binding. This is not overwritten by replaceSymbol to track
+  // Symbol binding. This is not overwritten by replace() to track
   // changes during resolution. In particular:
----------------
Sorry for commenting on an old diff.

I'm trying to understand this comment and if it's still true. As far as I can tell, `[replace()](https://github.com/llvm/llvm-project/blob/204d301bb1921431a853c0bfba32007c018df1d5/lld/ELF/Symbols.h#L529)` is just doing a `memcpy` from the new symbol into the old symbol, and the `binding` field isn't restored afterwards, so `replace` will in fact overwrite the binding, right?

I imagine this comment is from a time when the behavior was different (particularly when SymbolBody used to be distinct from Symbol and the binding was part of Symbol and not SymbolBody), but it doesn't seem to be accurate anymore, as far as I can tell.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62117