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
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to