[PATCH] D77807: [LLD][ELF] Implement --discard-* for cases when -r or --emit-relocs are used.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

I'd suggest to wait to hear what other reviewers think about the latest changes and my suggestions in general before continuing.



================
Comment at: lld/ELF/Symbols.h:241
         isInIplt(false), gotInIgot(false), isPreemptible(false),
-        used(!config->gcSections), needsTocRestore(false),
-        scriptDefined(false) {}
+        used(!config->gcSections && binding != llvm::ELF::STB_LOCAL),
+        needsTocRestore(false), scriptDefined(false) {}
----------------
So, when `!gcSections` and the symbol is local, it bow sets `used` to false,
though the previous code set it to `true` in this case.

Seems this doesn't break anything in the current implementation,
though I wonder if we might want to have `config->copyRelocs` involved in the condition.
It perhaps would be a bit cleaner way to work with this flag.

Also, perhaps it can be initialized in the constructor body now, since the condition is not trivial.

nit: You can probably use `isLocal()` instead of checking the `binding` field directly.




================
Comment at: lld/ELF/Symbols.h:261
   // True if an undefined or shared symbol is used from a live section.
   uint8_t used : 1;
 
----------------
The comment needs an update.


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

https://reviews.llvm.org/D77807