[llvm-commits] [llvm] r162657 - in /llvm/trunk: include/llvm/DebugInfo/DIContext.h lib/DebugInfo/DIContext.cpp lib/DebugInfo/DWARFContext.cpp lib/DebugInfo/DWARFContext.h lib/DebugInfo/DWARFDebugRangeList.cpp lib/DebugInfo/DWARFDebugRangeList.h t

[Eric Christopher]

On Aug 29, 2012, at 9:14 AM, Alexey Samsonov <samsonov at google.com> wrote:

> 
> 
> On Tue, Aug 28, 2012 at 5:53 AM, Eric Christopher <echristo at apple.com> wrote:
> 
> 
>> +bool DWARFDebugRangeList::extract(DataExtractor data, uint32_t *offset_ptr) {
>> +  clear();
>> +  if (!data.isValidOffset(*offset_ptr))
> 
> Another nitpick, this (all of the DebugInfo stuff) code largely doesn't conform to the coding standards (variable names especially) but if you could avoid putting more stuff in that doesn't conform when you add new files it'd be nice.
> 
> Yeah, thanks for pointing at this, I'll be more careful in the next patches.
> 

Thanks.

> Also might want to think about starting to add some test cases (I'm guilty of not doing it as well, but a thought).
> 
> Extensive testing of this is somewhat tricky, as we need chunks of valid DWARF data for testing (or completely separate parsing from logic, but this is a _significant_ refactoring job).
> Writing source files, compiling them and making sure that compiler will create the DWARF we need, uploading these binaries as the inputs for test cases is... ugly.
> I haven't taken a close look at tools like yaml2obj to see if they can help with this, though.

We could do that, I was thinking of just more round tripping through the writer and then disassembler. Just have a .ll file or .s file that has dwarf and assemble it up and disassemble it. Since the assembler has more support for dwarf at the moment than the disassembler it'd be a start.

-eric