[llvm] r174463 - Initial support for DWARF CFI parsing and dumping in LLVM

[Eli Bendersky]

On Tue, Feb 5, 2013 at 3:38 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> Some nitpicking:
>
>> +  FrameEntry(FrameKind K, DataExtractor D, uint64_t Offset, uint64_t
>> Length)
>> +    : Kind(K), Data(D), Offset(Offset), Length(Length)
>> +  {}
>> +
>
>
> Probably want to move the  {} to the previous line. There are a couple of
> places in this file.

Done

>
>>
>> +  uint64_t LinkedCIEOffset;
>> +  uint64_t InitialLocation;
>> +  uint64_t AddressRange;
>> +  CIE *LinkedCIE;
>
>
> Some more comments for instance variables.
>

Done

>>
>> +DWARFDebugFrame::DWARFDebugFrame()
>> +{
>> +}
>> +
>
>
> Formatting.
>

Done

>>
>> +
>> +DWARFDebugFrame::~DWARFDebugFrame()
>> +{
>> +  for (EntryVector::iterator I = Entries.begin(), E = Entries.end();
>> +       I != E; ++I) {
>> +    delete *I;
>> +  }
>> +}
>> +
>
>
> Ditto.
>

Done

>>
>> +
>> +static void LLVM_ATTRIBUTE_UNUSED dumpDataAux(DataExtractor Data,
>> +                                              uint32_t Offset, int
>> Length) {
>> +
>
>
> Double check this formatting?

Y u use proportional fonts when reviewing whitespace  ಠ_ಠ ? :-)

>
>>
>> +    // TODO: For honest DWARF64 support, DataExtractor will have to treat
>> +    //       offset_ptr as uint64_t*
>> +    uint32_t EndStructureOffset = Offset + static_cast<uint32_t>(Length);
>> +
>
>
> File a bug for this please?

Done -  http://llvm.org/bugs/show_bug.cgi?id=15173

Thanks for the review. And I remember I still have to add them tests.

Eli