[PATCH] D38012: Compact EhSectionPiece from 32 bytes to 16 bytes.

[Rafael Avila de Espindola via llvm-commits]

Rui Ueyama <ruiu at google.com> writes:

> On Tue, Sep 19, 2017 at 2:09 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>>
>> > -struct CieRecord {
>> > -  EhSectionPiece *Piece = nullptr;
>> > -  std::vector<EhSectionPiece *> FdePieces;
>> > -};
>> > -
>> >  // Section for .eh_frame.
>> >  template <class ELFT> class EhFrameSection final : public
>> SyntheticSection {
>> >    typedef typename ELFT::Shdr Elf_Shdr;
>> > @@ -88,19 +83,27 @@
>> >    std::vector<EhInputSection *> Sections;
>> >
>> >  private:
>> > +  struct CieRecord {
>> > +    EhInputSection *Sec = nullptr;
>> > +    EhSectionPiece *Cie = nullptr;
>> > +    std::vector<EhSectionPiece *> Fdes;
>> > +  };
>>
>> Why move the struct? It is now in a template, which makes it less
>> convenient to use.
>>
>
> No strong reason. Since the struct was used only by this class, so I
> thought moving it inside a local scope of this class would improve
> readability a bit, but we could keep it as-is.

Please do, having to type

typename EhFrameSection<ELFT>::CieRecord

is unfortunated.

>> I think this is OK, but it includes a few renames which make things
>> harder to read. Can you for example commit
>>
>> -  std::vector<CieRecord *> Cies;
>> +  std::vector<CieRecord *> CieRecords;
>>
>> First and rebase?
>>
>
> Will do.

Thanks,
Rafael