[cfe-commits] Declarator::getSourceRange()

[Chris Lattner]

On Feb 9, 2009, at 12:23 AM, Sebastian Redl wrote:
>> +    if (Tok.is(tok::kw___attribute)) {
>> +      SourceLocation Loc;
>> +      D.AddAttributes(ParseAttributes(&Loc));
>> +      D.SetRangeEnd(Loc);
>>
>> Please change this to:
>>
>> D.AddAttributes(ParseAttributes(&Loc), Loc);
>>
>> And have AddAttributes update SetRangeEnd, this makes it so we can't
>> forget to update the endloc when adding attributes.
>>
>
> I'm 95% sure this is unspecified behavior. The compiler *could* take  
> the
> value of Loc for the second attribute before calling  
> ParseAttributes(),
> thus losing the update.
> But I can change it to
>
> AttributeList *Attrs = ParseAttributes(&Loc);
> D.AddAttributes(Attrs, Loc);

Oh right, I guess you could also pass Loc by const& as a subtle and  
horrible workaround :)

>> +        if (OverloadedOperatorKind Op =
>> TryParseOperatorFunctionId(&EndLoc)) {
>>
>> Why pass EndLoc by pointer instead of byref?
>>
>
> The other call site of the function doesn't care about the end  
> location.

Ok.

>> +  // FIXME: Since the return type isn't actually parsed, need some  
>> fake
>> +  // location info for the declarator.
>> +  ParamInfo.SetSourceRange(SourceRange(Tok.getLocation(),
>> Tok.getLocation()));
>>
>> Does it make sense to just leave it as 'invalid'?
>
> No, this leads to extremely subtle bugs in error reporting, if  
> something
> then decides to actually use the source range. The location info  
> isn't as
> fake as the comment makes it seem.

What is a sensible location for a declspec that doesn't exist (e.g.  
implicit int)?  Shouldn't any diagnostics relating to empty declspecs  
be pointing to the decl itself?

-Chris