Handle correctly a very ugly part of the C++ syntax. We cannot disambiguate between a parenthesized type-id and

a paren expression without considering the context past the parentheses.

Behold:
  (T())x;  - type-id
  (T())*x; - type-id
  (T())/x; - expression
  (T());   - expression

llvm-svn: 72260

clang/include/clang/Parse/Parser.h

@@ -660,6 +660,9 @@ private:
 
   OwningExprResult ParseRHSOfBinaryExpression(OwningExprResult LHS,
                                               unsigned MinPrec);
+  OwningExprResult ParseCastExpression(bool isUnaryExpression,
+                                       bool isAddressOfOperand,
+                                       bool &NotCastExpr);
   OwningExprResult ParseCastExpression(bool isUnaryExpression,
                                        bool isAddressOfOperand = false);
   OwningExprResult ParsePostfixExpressionSuffix(OwningExprResult LHS);
@@ -690,6 +693,11 @@ private:
                                         TypeTy *&CastTy,
                                         SourceLocation &RParenLoc);
   
+  OwningExprResult ParseCXXAmbiguousParenExpression(ParenParseOption &ExprType,
+                                                    TypeTy *&CastTy,
+                                                    SourceLocation LParenLoc,
+                                                    SourceLocation &RParenLoc);
+  
   OwningExprResult ParseCompoundLiteralExpression(TypeTy *Ty,
                                                   SourceLocation LParenLoc,
                                                   SourceLocation RParenLoc);
@@ -918,11 +926,16 @@ private:
   /// isTypeIdInParens - Assumes that a '(' was parsed and now we want to know
   /// whether the parens contain an expression or a type-id.
   /// Returns true for a type-id and false for an expression.
-  bool isTypeIdInParens() {
+  bool isTypeIdInParens(bool &isAmbiguous) {
     if (getLang().CPlusPlus)
-      return isCXXTypeId(TypeIdInParens);
+      return isCXXTypeId(TypeIdInParens, isAmbiguous);
+    isAmbiguous = false;
     return isTypeSpecifierQualifier();
   }
+  bool isTypeIdInParens() {
+    bool isAmbiguous;
+    return isTypeIdInParens(isAmbiguous);
+  }
 
   /// isCXXDeclarationStatement - C++-specialized function that disambiguates
   /// between a declaration or an expression statement, when parsing function
@@ -951,7 +964,11 @@ private:
   /// the function returns true to let the declaration parsing code handle it.
   bool isCXXConditionDeclaration();
 
-  bool isCXXTypeId(TentativeCXXTypeIdContext Context);
+  bool isCXXTypeId(TentativeCXXTypeIdContext Context, bool &isAmbiguous);
+  bool isCXXTypeId(TentativeCXXTypeIdContext Context) {
+    bool isAmbiguous;
+    return isCXXTypeId(Context, isAmbiguous);
+  }
 
   /// TPResult - Used as the result value for functions whose purpose is to
   /// disambiguate C++ constructs by "tentatively parsing" them.

clang/lib/Parse/ParseExpr.cpp

@@ -400,6 +400,23 @@ Parser::ParseRHSOfBinaryExpression(OwningExprResult LHS, unsigned MinPrec) {
 /// id-expression that is the operand of address-of gets special treatment
 /// due to member pointers.
 ///
+Parser::OwningExprResult Parser::ParseCastExpression(bool isUnaryExpression,
+                                                     bool isAddressOfOperand) {
+  bool NotCastExpr;
+  OwningExprResult Res = ParseCastExpression(isUnaryExpression,
+                                             isAddressOfOperand,
+                                             NotCastExpr);
+  if (NotCastExpr)
+    Diag(Tok, diag::err_expected_expression);
+  return move(Res);
+}
+
+/// ParseCastExpression - Parse a cast-expression, or, if isUnaryExpression is
+/// true, parse a unary-expression. isAddressOfOperand exists because an
+/// id-expression that is the operand of address-of gets special treatment
+/// due to member pointers. NotCastExpr is set to true if the token is not the
+/// start of a cast-expression, and no diagnostic is emitted in this case.
+///
 ///       cast-expression: [C99 6.5.4]
 ///         unary-expression
 ///         '(' type-name ')' cast-expression
@@ -506,9 +523,11 @@ Parser::ParseRHSOfBinaryExpression(OwningExprResult LHS, unsigned MinPrec) {
 ///                   '__is_base_of'                          [TODO]
 ///
 Parser::OwningExprResult Parser::ParseCastExpression(bool isUnaryExpression,
-                                                     bool isAddressOfOperand) {
+                                                     bool isAddressOfOperand,
+                                                     bool &NotCastExpr) {
   OwningExprResult Res(Actions);
   tok::TokenKind SavedKind = Tok.getKind();
+  NotCastExpr = false;
   
   // This handles all of cast-expression, unary-expression, postfix-expression,
   // and primary-expression.  We handle them together like this for efficiency
@@ -797,7 +816,7 @@ Parser::OwningExprResult Parser::ParseCastExpression(bool isUnaryExpression,
       return ParsePostfixExpressionSuffix(ParseObjCMessageExpression());
     // FALL THROUGH.      
   default:
-    Diag(Tok, diag::err_expected_expression);
+    NotCastExpr = true;
     return ExprError();
   }
 
@@ -1216,6 +1235,7 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
   GreaterThanIsOperatorScope G(GreaterThanIsOperator, true);
   SourceLocation OpenLoc = ConsumeParen();
   OwningExprResult Result(Actions, true);
+  bool isAmbiguousTypeId;
   CastTy = 0;
 
   if (ExprType >= CompoundStmt && Tok.is(tok::l_brace)) {
@@ -1227,9 +1247,20 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr,
     if (!Stmt.isInvalid() && Tok.is(tok::r_paren))
       Result = Actions.ActOnStmtExpr(OpenLoc, move(Stmt), Tok.getLocation());
 
-  } else if (ExprType >= CompoundLiteral && isTypeIdInParens()) {
+  } else if (ExprType >= CompoundLiteral &&
+             isTypeIdInParens(isAmbiguousTypeId)) {
     
     // Otherwise, this is a compound literal expression or cast expression.
+    
+    // In C++, if the type-id is ambiguous we disambiguate based on context.
+    // If stopIfCastExpr is true the context is a typeof/sizeof/alignof
+    // in which case we should treat it as type-id.
+    // if stopIfCastExpr is false, we need to determine the context past the
+    // parens, so we defer to ParseCXXAmbiguousParenExpression for that.
+    if (isAmbiguousTypeId && !stopIfCastExpr)
+      return ParseCXXAmbiguousParenExpression(ExprType, CastTy,
+                                              OpenLoc, RParenLoc);
+    
     TypeResult Ty = ParseTypeName();
 
     // Match the ')'.

clang/lib/Parse/ParseExprCXX.cpp

@@ -1038,3 +1038,110 @@ Parser::OwningExprResult Parser::ParseUnaryTypeTrait()
 
   return Actions.ActOnUnaryTypeTrait(UTT, Loc, LParen, Ty.get(), RParen);
 }
+
+/// ParseCXXAmbiguousParenExpression - We have parsed the left paren of a
+/// parenthesized ambiguous type-id. This uses tentative parsing to disambiguate
+/// based on the context past the parens.
+Parser::OwningExprResult
+Parser::ParseCXXAmbiguousParenExpression(ParenParseOption &ExprType,
+                                         TypeTy *&CastTy,
+                                         SourceLocation LParenLoc,
+                                         SourceLocation &RParenLoc) {
+  assert(getLang().CPlusPlus && "Should only be called for C++!");
+  assert(ExprType == CastExpr && "Compound literals are not ambiguous!");
+  assert(isTypeIdInParens() && "Not a type-id!");
+
+  OwningExprResult Result(Actions, true);
+  CastTy = 0;
+
+  // We need to disambiguate a very ugly part of the C++ syntax:
+  //
+  // (T())x;  - type-id
+  // (T())*x; - type-id
+  // (T())/x; - expression
+  // (T());   - expression
+  //
+  // The bad news is that we cannot use the specialized tentative parser, since
+  // it can only verify that the thing inside the parens can be parsed as
+  // type-id, it is not useful for determining the context past the parens.
+  //
+  // The good news is that the parser can disambiguate this part without
+  // making any unnecessary Action calls (apart from isTypeName).
+
+  // Start tentantive parsing.
+  TentativeParsingAction PA(*this);
+
+  // Parse the type-id but don't create a type with ActOnTypeName yet.
+  DeclSpec DS;
+  ParseSpecifierQualifierList(DS);
+
+  // Parse the abstract-declarator, if present.
+  Declarator DeclaratorInfo(DS, Declarator::TypeNameContext);
+  ParseDeclarator(DeclaratorInfo);
+
+  if (!Tok.is(tok::r_paren)) {
+    PA.Commit();
+    MatchRHSPunctuation(tok::r_paren, LParenLoc);
+    return ExprError();
+  }
+
+  RParenLoc = ConsumeParen();
+
+  if (Tok.is(tok::l_brace)) {
+    // Compound literal. Ok, we can commit the parsed tokens and continue
+    // normal parsing.
+    ExprType = CompoundLiteral;
+    PA.Commit();
+    TypeResult Ty = true;
+    if (!DeclaratorInfo.isInvalidType())
+      Ty = Actions.ActOnTypeName(CurScope, DeclaratorInfo);
+    return ParseCompoundLiteralExpression(Ty.get(), LParenLoc, RParenLoc);
+  }
+
+  // We parsed '(' type-name ')' and the thing after it wasn't a '{'.
+
+  if (DeclaratorInfo.isInvalidType()) {
+    PA.Commit();
+    return ExprError();
+  }
+
+  bool NotCastExpr;
+  // Parse the cast-expression that follows it next.
+  Result = ParseCastExpression(false/*isUnaryExpression*/,
+                               false/*isAddressofOperand*/,
+                               NotCastExpr);
+
+  if (NotCastExpr == false) {
+    // We parsed a cast-expression. That means it's really a type-id, so commit
+    // the parsed tokens and continue normal parsing.
+    PA.Commit();
+    TypeResult Ty = Actions.ActOnTypeName(CurScope, DeclaratorInfo);
+    CastTy = Ty.get();
+    if (!Result.isInvalid())
+      Result = Actions.ActOnCastExpr(LParenLoc, CastTy, RParenLoc,move(Result));
+    return move(Result);
+  }
+
+  // If we get here, it means the things after the parens are not the start of
+  // a cast-expression. This means we must actually parse the tokens inside
+  // the parens as an expression.
+  PA.Revert();
+
+  Result = ParseExpression();
+  ExprType = SimpleExpr;
+  if (!Result.isInvalid() && Tok.is(tok::r_paren))
+    Result = Actions.ActOnParenExpr(LParenLoc, Tok.getLocation(), move(Result));
+
+  // Match the ')'.
+  if (Result.isInvalid()) {
+    SkipUntil(tok::r_paren);
+    return ExprError();
+  }
+
+  if (Tok.is(tok::r_paren))
+    RParenLoc = ConsumeParen();
+  else
+    MatchRHSPunctuation(tok::r_paren, LParenLoc);
+
+  return move(Result);
+}

clang/lib/Parse/ParseTentative.cpp

@@ -287,7 +287,9 @@ bool Parser::isCXXConditionDeclaration() {
   /// type-id:
   ///   type-specifier-seq abstract-declarator[opt]
   ///
-bool Parser::isCXXTypeId(TentativeCXXTypeIdContext Context) {
+bool Parser::isCXXTypeId(TentativeCXXTypeIdContext Context, bool &isAmbiguous) {
+  
+  isAmbiguous = false;
 
   // C++ 8.2p2:
   // The ambiguity arising from the similarity between a function-style cast and
@@ -326,16 +328,20 @@ bool Parser::isCXXTypeId(TentativeCXXTypeIdContext Context) {
   if (TPR == TPResult::Ambiguous()) {
     // We are supposed to be inside parens, so if after the abstract declarator
     // we encounter a ')' this is a type-id, otherwise it's an expression.
-    if (Context == TypeIdInParens && Tok.is(tok::r_paren))
+    if (Context == TypeIdInParens && Tok.is(tok::r_paren)) {
       TPR = TPResult::True();
+      isAmbiguous = true;
+
     // We are supposed to be inside a template argument, so if after
     // the abstract declarator we encounter a '>', '>>' (in C++0x), or
     // ',', this is a type-id. Otherwise, it's an expression.
-    else if (Context == TypeIdAsTemplateArgument &&
+    } else if (Context == TypeIdAsTemplateArgument &&
                (Tok.is(tok::greater) || Tok.is(tok::comma) ||
-              (getLang().CPlusPlus0x && Tok.is(tok::greatergreater))))
+                (getLang().CPlusPlus0x && Tok.is(tok::greatergreater)))) {
       TPR = TPResult::True();
-    else
+      isAmbiguous = true;
+
+    } else
       TPR = TPResult::False();
   }
 

clang/test/Parser/cxx-ambig-paren-expr.cpp

@@ -0,0 +1,15 @@
+// RUN: clang-cc -fsyntax-only -verify %s
+
+void f() {
+  typedef int T;
+  int x, *px;
+  
+  // Type id.
+  (T())x;    // expected-error {{used type 'T (void)'}}
+  (T())+x;   // expected-error {{used type 'T (void)'}}
+  (T())*px;  // expected-error {{used type 'T (void)'}}
+  
+  // Expression.
+  x = (T());
+  x = (T())/x;
+}