[PATCH] D30507: Remove DefinedSynthetic

[Rafael Avila de Espindola via llvm-commits]

Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:

> 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?

It might make some of them redundant. Note that currently they are not
sufficient, which is why I started this patch to try to fix pr32031.

> ================
> 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.

Yes, the hierarchy needs better compaction. I will work on it a bit and
upload a new patch.

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

Yes.

> ================
> 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?

Yes, it does. We have explicit ".Repl" in a few places. I will try to
factor them a bit better, but I would like to really avoid having Repl
in the base.

Cheers,
Rafael