[PATCH] D66130: [ELF] Initialize 2 fields of Symbol in SymbolTable::insert

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D66130#1626705 <https://reviews.llvm.org/D66130#1626705>, @peter.smith wrote:

> LGTM too, I agree with George that making the comment specific isn't helpful but a general one isn't doing any harm.
>
> An alternative is to make an initialization function in Symbol and have the default constructor call that for the bitfields. This could be called from SymbolTable::insert. This would have the benefit that we'd only have to update one place for a new field. I notice that canInline is given a different default value from the default constructor, but if this is important it could be a parameter to the initialization function.


Yes, canInline is weird. The member initializer arbitrarily sets it to false, then SymbolTable::insert changes it to true, in --wrap handling it gets assigned to false (true->false semilattice).

> A further alternative is to wrap the bitfields in a struct that can be initialized and copied as a whole, but would require every use of one of the fields to be updated.

The problem is that I can't give the member function a reasonable name :( Conceptually any field referenced when the symbol is still a placeholder should be initialized (these fields are all bit fields currently). Note, there are many other fields that are not referenced - they are not initialized in SymbolTable::insert. Some bit fields are initialized differently in the member initializer list and SymbolTable::insert

- visibility
- isUsedInRegularObj
- canInline
- exportDynamic

So a member function may not save us a lot of trouble.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D66130