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:

  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);
}


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

Reply via email to