[cfe-commits] r41200 - in /cfe/trunk: Parse/ParseObjc.cpp Parse/Parser.cpp include/clang/Basic/DiagnosticKinds.def include/clang/Parse/Parser.h

[Chris Lattner]

>> If this is an objc2 feature, should it check getLang().ObjC2?   
>> Likewise in other places that handle objc2 features in various ways.
>>
>
> I don't have a problem with this. I think it's worth a TODO...not  
> important for now.

Please add a fixme so we can find it later.

>> This is a common idiom.  Do you think it would be worthwhile to  
>> add a:
>>
>> bool Parser::ConsumeIfToken(tokenkind), which could be used like:
>>
>>   if (ConsumeIfToken(tok::colon)) {
>>     ...
>> ?
>>
>> However, one bad thing about this is that it loses the location of  
>> the token if consumed.
>
> I don't have a strong opinion on this. I am not bothered by the  
> above code.

Ok, fair enough!

>>
>>> ==================================================================== 
>>> ==========
>>> --- cfe/trunk/Parse/Parser.cpp (original)
>>> +++ cfe/trunk/Parse/Parser.cpp Mon Aug 20 16:31:48 2007
>>> @@ -370,6 +370,13 @@
>>>      return Actions.ParsedFreeStandingDeclSpec(CurScope, DS);
>>>    }
>>>
>>> +  // OBJC: This grammar hack allows prefix attributes on class  
>>> interfaces.
>>
>> How is this a hack?
>
> Bad terminology. I was a little bothered by calling  
> ParseObjCAtInterfaceDeclaration() from this place in the C grammar.
>
> I wish class attributes were after the @interface, but that ship  
> has already sailed.
>
> I'll remove the comment.

Sounds good, thanks.

>>> +  if (Tok.getKind() == tok::at) {
>>> +    SourceLocation AtLoc = ConsumeToken(); // the "@"
>>> +    if (Tok.getIdentifierInfo()->getObjCKeywordID() ==  
>>> tok::objc_interface)
>>
>> This is another common idiom.  It seems like it would be nice to  
>> add a:
>>
>> bool Token::isObjCAtKeyword(...)
>>
>> method, allowing:
>>
>> if (Tok.isObjCAtKeyword(tok::objc_interface))
>
> I totally agree...this would be a very nice improvement (something  
> I thought of as I was writing the code).
>
> In fact, the above code is flawed ("@<non-identifier>" will go boom).
>
> I believe I fixed most of these today, however the code below is  
> even more ugly to look at.
>
> +  assert((Tok.getKind() == tok::identifier &&
> +          Tok.getIdentifierInfo()->getObjCKeywordID() ==  
> tok::objc_interface) &&
>
> So...having a predicate will simplify the code and make sure we  
> don't do boom:-)

Woohoo, thanks Steve,

-Chris