[llvm-commits] [llvm] r169484 - in /llvm/trunk: include/llvm/DebugInfo.h lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp lib/CodeGen/AsmPrinter/DwarfCompileUnit.h test/DebugInfo/X86/nondefault-subrange-array.ll

[Bill Wendling]

On Dec 5, 2012, at 11:51 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Wed, Dec 5, 2012 at 11:38 PM, Bill Wendling <isanbard at gmail.com> wrote:
>> -  // The L value defines the lower bounds which is typically zero for C/C++. The
>> -  // Count value is the number of elements.  Values are 64 bit. If Count == -1
>> -  // then the array is unbounded and we do not emit DW_AT_lower_bound and
>> -  // DW_AT_upper_bound attributes. If L == 0 and Count == 0, then the array has
>> -  // zero elements in which case we do not emit an upper bound.
>> -  uint64_t L = SR.getLo();
>> +  // The LowerBound value defines the lower bounds which is typically zero for
>> +  // C/C++. The Count value is the number of elements.  Values are 64 bit. If
>> +  // Count == -1 then the array is unbounded and we do not emit
>> +  // DW_AT_lower_bound and DW_AT_upper_bound attributes. If LowerBound == 0 and
>> +  // Count == 0, then the array has zero elements in which case we do not emit
>> +  // an upper bound.
>> +  int64_t LowerBound = SR.getLo();
>> +  int64_t DefaultLowerBound = getLowerBoundDefault();
> 
> Just a minor nit, but this kind of inconsistency (DefaultLowerBound v
> LowerBoundDefault) can be a bit of a pain when coming back to work
> with such code. Should they be just use the same name?
> 
Okay.

>>   int64_t Count = SR.getCount();
>> 
>> -  if (Count != -1) {
>> +  if (LowerBound != DefaultLowerBound || DefaultLowerBound == -1)
> 
> Probably just me, but I'd find this marginally more legible written
> the other way around (DefaultLowerBound == -1 || LowerBound !=
> DefaultLowerBound) - just a bit weird to deliberately compare the
> invalid DefaultLowerBound value (took me a moment to convince myself
> that couldn't be problematic).

I expect the first bit to be hit more than the second bit, which is why I put it first. :)

-bw