[PATCH] D118530: [ELF] Update flag propagation rule to ignore discarded output sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/LinkerScript.cpp:1152
+    bool isEmpty = (getFirstInputSection(sec) == nullptr);
+    bool discardable = isEmpty && isDiscardable(*sec);
+    // If sec has at least an input section and not discarded, remember its
----------------
andrewng wrote:
> I believe that all uses of `isDiscardable()` are now combined with `getFirstInputSection() == nullptr`, so perhaps `getFirstInputSection() == nullptr` should be part of `isDiscardable()`? Although that doesn't really "help" here as you need `isEmpty`.
Agree with your analysis.

`isEmpty` makes the code sharing not much helpful. Two factors make me think we may need to postpone the code share:

* The code sharing causes `getFirstInputSection(sec)` to be called one extra time.
* I think we may need to refactor this logic once again. Adding `getFirstInputSection(sec)` might make refactoring difficult.


================
Comment at: lld/ELF/LinkerScript.cpp:1153
+    bool discardable = isEmpty && isDiscardable(*sec);
+    // If sec has at least an input section and not discarded, remember its
+    // flags to be inherited by subsequent output sections. (sec may contain
----------------
peter.smith wrote:
> Trivial language nit, suggest one instead of an. // If sec has at least one input section and is not discarded.
Thanks. Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118530