[llvm] r196178 - Make ranges and range lists be a discrete entity that can be located

[David Blaikie]

On Wed, Dec 4, 2013 at 1:29 PM, Eric Christopher <echristo at gmail.com> wrote:
>> You could use llvm_move when passing the list to addRangeList here.
>
> Fixed this and a few other things in follow up commits. Thanks :)

Thanks - looks great!

>
>>> +      for (SmallVectorImpl<RangeSpan>::const_iterator
>>> +               I = List->getRanges().begin(),
>>> +               E = List->getRanges().end();
>>> +           I != E; ++I) {
>>> +        RangeSpan Range = *I;
>>> +        // We occasionally have ranges without begin/end labels.
>>> +        // FIXME: Verify and fix.
>>
>> Any test cases for that? Would be nice to have them committed and
>> XFAILed, ideally. (yeah, I don't often do this either, so hardly a
>> requirement)
>
> Yeah, it's been in the code forever. The "Verify and fix" part is
> "well, we have this check, but there aren't any test cases that I can
> find that fail if we remove it" - it's a note that someone should look
> into this.

Fair enough.

>
>>> -  // Flag if we've emitted any ranges and their location for the compile unit.
>>> -  if (DebugRangeSymbols.size()) {
>>> +  // Attribute if we've emitted any ranges and their location for the compile unit.
>>> +  if (CU->getRangeLists().size()) {
>>
>> This looks like a !empty() test.
>
> Fair :)
>
>>> +
>>> +class RangeSpanList {
>>
>> I think the DWARF spec refers to each top level entry in the ranges
>> section as a "range set", possibly? Maybe reusing that terminology
>> would be helpful?
>
> Possibly. I was trying to avoid overlapping names with common data
> structures as well. :\
>
> Not sure what's more clear.

Honestly it'd probably be useful to put all this stuff in a debuginfo
namespace or something (& a corresponding directory) - the mismatch
between file names and the classes they contain is already weird.

To reduce ambiguity I'd probably err towards an 'Address' prefix,
perhaps ("AddressRange", "AddressRangeSet", "AddressRangeSets"
(plurality versus a container suffix is also interesting - I don't
think the standard gives us a term for the "set of sets")).

And admittedly the confusion of using "set" for a structure backed by
a vector is questionable.

- David