[PATCH] D30507: Remove DefinedSynthetic

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

This is interesting. It seems almost neutral to me as it removes one class and instead introduces one class hierarchy. I wonder if this opens up other opportunities to simplify things. E.g. we have a bunch of lambdas in LinkerScript.cpp to compute addresses lazily. If you land this patch, can you simplify that, for example?



================
Comment at: ELF/InputSection.h:37
+class SectionBase {
+  unsigned SectionKind : 3;
+
----------------
It actually uses 64 bits, no? I don't think we need to use a bit field.


================
Comment at: ELF/Symbols.cpp:50
+    if (auto *ISB = dyn_cast_or_null<InputSectionBase>(IS))
+      IS = ISB->Repl;
 
----------------
Ah, okay, this.


================
Comment at: ELF/Symbols.cpp:272-276
+         (cast<InputSectionBase>(Section)
+              ->template getFile<ELFT>()
+              ->getObj()
+              .getHeader()
+              ->e_flags &
----------------
Is this what clang-format formatted?


================
Comment at: ELF/Symbols.h:182
       : Defined(SymbolBody::DefinedRegularKind, Name, IsLocal, StOther, Type),
-        Value(Value), Size(Size),
-        Section(Section ? Section->Repl : NullInputSection) {
+        Value(Value), Size(Size), Section(Section) {
     this->File = File;
----------------
->Repl is for ICF. Does ICF still work without that?


https://reviews.llvm.org/D30507