[cfe-commits] r152137 - in /cfe/trunk: include/clang/AST/ include/clang/Basic/ include/clang/Driver/ include/clang/Frontend/ include/clang/Parse/ include/clang/Sema/ include/clang/Serialization/ lib/AST/ lib/CodeGen/ lib/Driver/ lib/Lex/ lib/Parse/ lib/Sema/ lib/Serialization/ test/CodeGenObjC/ test/CodeGenObjC/Inputs/ test/CodeGenObjCXX/ test/CodeGenObjCXX/Inputs/ test/Driver/ test/PCH/ test/SemaObjC/ test/SemaObjCXX/
jahanian
fjahanian at apple.com
Mon Mar 19 10:47:53 PDT 2012
On Mar 19, 2012, at 10:43 AM, jahanian wrote:
>
> On Mar 19, 2012, at 10:36 AM, Jordan Rose wrote:
>
>>
>> On Mar 19, 2012, at 10:13, jahanian wrote:
>>
>>>
>>> On Mar 17, 2012, at 5:40 PM, Jordan Rose wrote:
>>>
>>>>
>>>> On Mar 6, 2012, at 12:05, Ted Kremenek wrote:
>>>>
>>>>> +ExprResult Sema::ActOnObjCBoolLiteral(SourceLocation AtLoc,
>>>>> + SourceLocation ValueLoc,
>>>>> + bool Value) {
>>>>> + ExprResult Inner;
>>>>> + if (getLangOptions().CPlusPlus) {
>>>>> + Inner = ActOnCXXBoolLiteral(ValueLoc, Value? tok::kw_true : tok::kw_false);
>>>>> + } else {
>>>>> + // C doesn't actually have a way to represent literal values of type
>>>>> + // _Bool. So, we'll use 0/1 and implicit cast to _Bool.
>>>>> + Inner = ActOnIntegerConstant(ValueLoc, Value? 1 : 0);
>>>>> + Inner = ImpCastExprToType(Inner.get(), Context.BoolTy,
>>>>> + CK_IntegralToBoolean);
>>>>> + }
>>>>> +
>>>>> + return BuildObjCNumericLiteral(AtLoc, Inner.get());
>>>>
>>>> Late with code review, but since you have ActOnObjCBoolLiteral(SourceLocation OpLoc, tok::TokenKind Kind), why not use that now? I assume that's why it was even introduced...to AVOID this cast dance.
>>>
>>> Actions.ActOnObjCBoolLiteral(ConsumeToken(), Kind) produces the AST for __objc_yes/__objc_no scalar values while ObjCNumericLiteral is for @__objc_yes/@__objc_no objects.
>>
>> This is actually not true, but it probably should be. In ParseObjCAtExpression (ParseObjc.cpp), there's the following code:
>
> If you dump the ast for __objc_yes and @__objc_no, they will come out as ObjCBoolLiteral and ObjCNumericLiteral, respectively.
>
>>
>> case tok::kw_true: // Objective-C++, etc.
>> case tok::kw___objc_yes: // c/c++/objc/objc++ __objc_yes
>> return ParsePostfixExpressionSuffix(ParseObjCBooleanLiteral(AtLoc, true));
>> case tok::kw_false: // Objective-C++, etc.
>> case tok::kw___objc_no: // c/c++/objc/objc++ __objc_no
>> return ParsePostfixExpressionSuffix(ParseObjCBooleanLiteral(AtLoc, false));
>>
>>
>> And this version of ParseObjCBooleanLiteral looks like this:
>>
>> /// ParseObjCBooleanLiteral -
>> /// objc-scalar-literal : '@' boolean-keyword
>> /// ;
>> /// boolean-keyword: 'true' | 'false' | '__objc_yes' | '__objc_no'
>> /// ;
>> ExprResult Parser::ParseObjCBooleanLiteral(SourceLocation AtLoc,
>> bool ArgValue) {
>> SourceLocation EndLoc = ConsumeToken(); // consume the keyword.
>> return Actions.ActOnObjCBoolLiteral(AtLoc, EndLoc, ArgValue);
>> }
>>
>
> Here it is clear what ObjCBoolLiteral is for.
Sorry, I meant to say what ObjCNumericLiteral is for. Naming two methods as ObjCBoolLiteral is surely confusing :).
- Fariborz
> - Fariborz
>
>>
>> The ParseObjCBooleanLiteral that just handles __objc_yes (without the @) is in ParseExpr.cpp.
>>
>> Having two versions of ParseObjCBooleanLiteral makes sense, but maybe the @-version should look more like ParseObjCNumericLiteral?
>>
>>>
>>> I agree that names are confusing.
>>>
>>> - fariborz
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list