Downgrade the "when type is in parentheses, array cannot have dynamic

size" error for code like 

  new (int [size])

to a warning, add a Fix-It to remove the parentheses, and make this
diagnostic work properly when it occurs in a template
instantiation. <rdar://problem/8018245>.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@108242 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed, 96 insertions, 47 deletions

diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h
index ff499d362e..77b366ed01 100644
--- a/include/clang/AST/ExprCXX.h
+++ b/include/clang/AST/ExprCXX.h
@@ -922,8 +922,6 @@ public:
 class CXXNewExpr : public Expr {
   // Was the usage ::new, i.e. is the global new to be used?
   bool GlobalNew : 1;
-  // Was the form (type-id) used? Otherwise, it was new-type-id.
-  bool ParenTypeId : 1;
   // Is there an initializer? If not, built-ins are uninitialized, else they're
   // value-initialized.
   bool Initializer : 1;
@@ -947,12 +945,18 @@ class CXXNewExpr : public Expr {
   // Must be null for all other types.
   CXXConstructorDecl *Constructor;
 
+  /// \brief If the allocated type was expressed as a parenthesized type-id, 
+  /// the source range covering the parenthesized type-id.
+  SourceRange TypeIdParens;
+  
   SourceLocation StartLoc;
   SourceLocation EndLoc;
 
+  friend class PCHStmtReader;
 public:
   CXXNewExpr(ASTContext &C, bool globalNew, FunctionDecl *operatorNew,
-             Expr **placementArgs, unsigned numPlaceArgs, bool ParenTypeId,
+             Expr **placementArgs, unsigned numPlaceArgs,
+             SourceRange TypeIdParens,
              Expr *arraySize, CXXConstructorDecl *constructor, bool initializer,
              Expr **constructorArgs, unsigned numConsArgs,
              FunctionDecl *operatorDelete, QualType ty,
@@ -995,10 +999,11 @@ public:
     return cast<Expr>(SubExprs[Array + i]);
   }
 
+  bool isParenTypeId() const { return TypeIdParens.isValid(); }
+  SourceRange getTypeIdParens() const { return TypeIdParens; }
+
   bool isGlobalNew() const { return GlobalNew; }
   void setGlobalNew(bool V) { GlobalNew = V; }
-  bool isParenTypeId() const { return ParenTypeId; }
-  void setParenTypeId(bool V) { ParenTypeId = V; }
   bool hasInitializer() const { return Initializer; }
   void setHasInitializer(bool V) { Initializer = V; }
 
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index d73f80a2e9..8fac1edecf 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2343,7 +2343,7 @@ def err_new_array_nonconst : Error<
   "only the first dimension of an allocated array may have dynamic size">;
 def err_new_array_init_args : Error<
   "array 'new' cannot have initialization arguments">;
-def err_new_paren_array_nonconst : Error<
+def ext_new_paren_array_nonconst : ExtWarn<
   "when type is in parentheses, array cannot have dynamic size">;
 def err_placement_new_non_placement_delete : Error<
   "'new' expression with placement arguments refers to non-placement "
diff --git a/include/clang/Parse/Action.h b/include/clang/Parse/Action.h
index 16a1d46b13..9cb47aa8da 100644
--- a/include/clang/Parse/Action.h
+++ b/include/clang/Parse/Action.h
@@ -1651,16 +1651,39 @@ public:
     return move(SubExpr);
   }
   
-  /// ActOnCXXNew - Parsed a C++ 'new' expression. UseGlobal is true if the
-  /// new was qualified (::new). In a full new like
-  /// @code new (p1, p2) type(c1, c2) @endcode
-  /// the p1 and p2 expressions will be in PlacementArgs and the c1 and c2
-  /// expressions in ConstructorArgs. The type is passed as a declarator.
+  /// \brief Parsed a C++ 'new' expression.
+  ///
+  /// \param StartLoc The start of the new expression, which is either the
+  /// "new" keyword or the "::" preceding it, depending on \p UseGlobal.
+  ///
+  /// \param UseGlobal True if the "new" was qualified with "::".
+  ///
+  /// \param PlacementLParen The location of the opening parenthesis ('(') for
+  /// the placement arguments, if any.
+  /// 
+  /// \param PlacementArgs The placement arguments, if any.
+  ///
+  /// \param PlacementRParen The location of the closing parenthesis (')') for
+  /// the placement arguments, if any.
+  ///
+  /// \param TypeIdParens If the type was expressed as a type-id in parentheses,
+  /// the source range covering the parenthesized type-id.
+  ///
+  /// \param D The parsed declarator, which may include an array size (for 
+  /// array new) as the first declarator.
+  ///
+  /// \param ConstructorLParen The location of the opening parenthesis ('(') for
+  /// the constructor arguments, if any.
+  ///
+  /// \param ConstructorArgs The constructor arguments, if any.
+  ///
+  /// \param ConstructorRParen The location of the closing parenthesis (')') for
+  /// the constructor arguments, if any.
   virtual OwningExprResult ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
                                        SourceLocation PlacementLParen,
                                        MultiExprArg PlacementArgs,
                                        SourceLocation PlacementRParen,
-                                       bool ParenTypeId, Declarator &D,
+                                       SourceRange TypeIdParens, Declarator &D,
                                        SourceLocation ConstructorLParen,
                                        MultiExprArg ConstructorArgs,
                                        SourceLocation ConstructorRParen) {
diff --git a/lib/AST/ExprCXX.cpp b/lib/AST/ExprCXX.cpp
index 4bbf441ef3..c2548eca65 100644
--- a/lib/AST/ExprCXX.cpp
+++ b/lib/AST/ExprCXX.cpp
@@ -85,16 +85,16 @@ Stmt::child_iterator CXXScalarValueInitExpr::child_end() {
 // CXXNewExpr
 CXXNewExpr::CXXNewExpr(ASTContext &C, bool globalNew, FunctionDecl *operatorNew,
                        Expr **placementArgs, unsigned numPlaceArgs,
-                       bool parenTypeId, Expr *arraySize,
+                       SourceRange TypeIdParens, Expr *arraySize,
                        CXXConstructorDecl *constructor, bool initializer,
                        Expr **constructorArgs, unsigned numConsArgs,
                        FunctionDecl *operatorDelete, QualType ty,
                        SourceLocation startLoc, SourceLocation endLoc)
   : Expr(CXXNewExprClass, ty, ty->isDependentType(), ty->isDependentType()),
-    GlobalNew(globalNew), ParenTypeId(parenTypeId),
+    GlobalNew(globalNew),
     Initializer(initializer), SubExprs(0), OperatorNew(operatorNew),
     OperatorDelete(operatorDelete), Constructor(constructor),
-    StartLoc(startLoc), EndLoc(endLoc) {
+    TypeIdParens(TypeIdParens), StartLoc(startLoc), EndLoc(endLoc) {
       
   AllocateArgsArray(C, arraySize != 0, numPlaceArgs, numConsArgs);
   unsigned i = 0;
diff --git a/lib/Frontend/PCHReaderStmt.cpp b/lib/Frontend/PCHReaderStmt.cpp
index f6313633f5..ace62d787e 100644
--- a/lib/Frontend/PCHReaderStmt.cpp
+++ b/lib/Frontend/PCHReaderStmt.cpp
@@ -1042,7 +1042,6 @@ void PCHStmtReader::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *E) {
 void PCHStmtReader::VisitCXXNewExpr(CXXNewExpr *E) {
   VisitExpr(E);
   E->setGlobalNew(Record[Idx++]);
-  E->setParenTypeId(Record[Idx++]);
   E->setHasInitializer(Record[Idx++]);
   bool isArray = Record[Idx++];
   unsigned NumPlacementArgs = Record[Idx++];
@@ -1052,6 +1051,10 @@ void PCHStmtReader::VisitCXXNewExpr(CXXNewExpr *E) {
                     cast_or_null<FunctionDecl>(Reader.GetDecl(Record[Idx++])));
   E->setConstructor(
                cast_or_null<CXXConstructorDecl>(Reader.GetDecl(Record[Idx++])));
+  SourceRange TypeIdParens;
+  TypeIdParens.setBegin(SourceLocation::getFromRawEncoding(Record[Idx++]));
+  TypeIdParens.setEnd(SourceLocation::getFromRawEncoding(Record[Idx++]));
+  E->TypeIdParens = TypeIdParens;
   E->setStartLoc(SourceLocation::getFromRawEncoding(Record[Idx++]));
   E->setEndLoc(SourceLocation::getFromRawEncoding(Record[Idx++]));
   
diff --git a/lib/Frontend/PCHWriterStmt.cpp b/lib/Frontend/PCHWriterStmt.cpp
index d6beda8bab..75377286e5 100644
--- a/lib/Frontend/PCHWriterStmt.cpp
+++ b/lib/Frontend/PCHWriterStmt.cpp
@@ -1070,7 +1070,6 @@ void PCHStmtWriter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *E) {
 void PCHStmtWriter::VisitCXXNewExpr(CXXNewExpr *E) {
   VisitExpr(E);
   Record.push_back(E->isGlobalNew());
-  Record.push_back(E->isParenTypeId());
   Record.push_back(E->hasInitializer());
   Record.push_back(E->isArray());
   Record.push_back(E->getNumPlacementArgs());
@@ -1078,6 +1077,7 @@ void PCHStmtWriter::VisitCXXNewExpr(CXXNewExpr *E) {
   Writer.AddDeclRef(E->getOperatorNew(), Record);
   Writer.AddDeclRef(E->getOperatorDelete(), Record);
   Writer.AddDeclRef(E->getConstructor(), Record);
+  Writer.AddSourceRange(E->getTypeIdParens(), Record);
   Writer.AddSourceLocation(E->getStartLoc(), Record);
   Writer.AddSourceLocation(E->getEndLoc(), Record);
   for (CXXNewExpr::arg_iterator I = E->raw_arg_begin(), e = E->raw_arg_end();
diff --git a/lib/Frontend/PrintParserCallbacks.cpp b/lib/Frontend/PrintParserCallbacks.cpp
index b032233b3d..9220677483 100644
--- a/lib/Frontend/PrintParserCallbacks.cpp
+++ b/lib/Frontend/PrintParserCallbacks.cpp
@@ -819,7 +819,8 @@ namespace {
                                          SourceLocation PlacementLParen,
                                          MultiExprArg PlacementArgs,
                                          SourceLocation PlacementRParen,
-                                         bool ParenTypeId, Declarator &D,
+                                         SourceRange TypeIdParens, 
+                                         Declarator &D,
                                          SourceLocation ConstructorLParen,
                                          MultiExprArg ConstructorArgs,
                                          SourceLocation ConstructorRParen) {
diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp
index 9d4389b1ff..579d3bde49 100644
--- a/lib/Parse/ParseExprCXX.cpp
+++ b/lib/Parse/ParseExprCXX.cpp
@@ -1572,7 +1572,7 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
   ExprVector PlacementArgs(Actions);
   SourceLocation PlacementLParen, PlacementRParen;
 
-  bool ParenTypeId;
+  SourceRange TypeIdParens;
   DeclSpec DS;
   Declarator DeclaratorInfo(DS, Declarator::TypeNameContext);
   if (Tok.is(tok::l_paren)) {
@@ -1591,17 +1591,17 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
 
     if (PlacementArgs.empty()) {
       // Reset the placement locations. There was no placement.
+      TypeIdParens = SourceRange(PlacementLParen, PlacementRParen);
       PlacementLParen = PlacementRParen = SourceLocation();
-      ParenTypeId = true;
     } else {
       // We still need the type.
       if (Tok.is(tok::l_paren)) {
-        SourceLocation LParen = ConsumeParen();
+        TypeIdParens.setBegin(ConsumeParen());
         ParseSpecifierQualifierList(DS);
         DeclaratorInfo.SetSourceRange(DS.getSourceRange());
         ParseDeclarator(DeclaratorInfo);
-        MatchRHSPunctuation(tok::r_paren, LParen);
-        ParenTypeId = true;
+        TypeIdParens.setEnd(MatchRHSPunctuation(tok::r_paren, 
+                                                TypeIdParens.getBegin()));
       } else {
         if (ParseCXXTypeSpecifierSeq(DS))
           DeclaratorInfo.setInvalidType(true);
@@ -1610,7 +1610,6 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
           ParseDeclaratorInternal(DeclaratorInfo,
                                   &Parser::ParseDirectNewDeclarator);
         }
-        ParenTypeId = false;
       }
     }
   } else {
@@ -1623,7 +1622,6 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
       ParseDeclaratorInternal(DeclaratorInfo,
                               &Parser::ParseDirectNewDeclarator);
     }
-    ParenTypeId = false;
   }
   if (DeclaratorInfo.isInvalidType()) {
     SkipUntil(tok::semi, /*StopAtSemi=*/true, /*DontConsume=*/true);
@@ -1651,7 +1649,7 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
 
   return Actions.ActOnCXXNew(Start, UseGlobal, PlacementLParen,
                              move_arg(PlacementArgs), PlacementRParen,
-                             ParenTypeId, DeclaratorInfo, ConstructorLParen,
+                             TypeIdParens, DeclaratorInfo, ConstructorLParen,
                              move_arg(ConstructorArgs), ConstructorRParen);
 }
 
diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 886a6b4e91..ddbe7e0770 100644
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -2366,7 +2366,7 @@ public:
                                        SourceLocation PlacementLParen,
                                        MultiExprArg PlacementArgs,
                                        SourceLocation PlacementRParen,
-                                       bool ParenTypeId, Declarator &D,
+                                       SourceRange TypeIdParens, Declarator &D,
                                        SourceLocation ConstructorLParen,
                                        MultiExprArg ConstructorArgs,
                                        SourceLocation ConstructorRParen);
@@ -2374,7 +2374,7 @@ public:
                                SourceLocation PlacementLParen,
                                MultiExprArg PlacementArgs,
                                SourceLocation PlacementRParen,
-                               bool ParenTypeId,
+                               SourceRange TypeIdParens,
                                QualType AllocType,
                                SourceLocation TypeLoc,
                                SourceRange TypeRange,
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 5c693ab358..a5abfe851b 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -589,7 +589,7 @@ Sema::ActOnCXXTypeConstructExpr(SourceRange TypeRange, TypeTy *TypeRep,
 Action::OwningExprResult
 Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
                   SourceLocation PlacementLParen, MultiExprArg PlacementArgs,
-                  SourceLocation PlacementRParen, bool ParenTypeId,
+                  SourceLocation PlacementRParen, SourceRange TypeIdParens, 
                   Declarator &D, SourceLocation ConstructorLParen,
                   MultiExprArg ConstructorArgs,
                   SourceLocation ConstructorRParen) {
@@ -605,17 +605,6 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
       return ExprError(Diag(Chunk.Loc, diag::err_array_new_needs_size)
         << D.getSourceRange());
 
-    if (ParenTypeId) {
-      // Can't have dynamic array size when the type-id is in parentheses.
-      Expr *NumElts = (Expr *)Chunk.Arr.NumElts;
-      if (!NumElts->isTypeDependent() && !NumElts->isValueDependent() &&
-          !NumElts->isIntegerConstantExpr(Context)) {
-        Diag(D.getTypeObject(0).Loc, diag::err_new_paren_array_nonconst)
-          << NumElts->getSourceRange();
-        return ExprError();
-      }
-    }
-
     ArraySize = static_cast<Expr*>(Chunk.Arr.NumElts);
     D.DropFirstTypeObject();
   }
@@ -649,7 +638,7 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
                      PlacementLParen,
                      move(PlacementArgs),
                      PlacementRParen,
-                     ParenTypeId,
+                     TypeIdParens,
                      AllocType,
                      D.getSourceRange().getBegin(),
                      R,
@@ -664,7 +653,7 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
                   SourceLocation PlacementLParen,
                   MultiExprArg PlacementArgs,
                   SourceLocation PlacementRParen,
-                  bool ParenTypeId,
+                  SourceRange TypeIdParens,
                   QualType AllocType,
                   SourceLocation TypeLoc,
                   SourceRange TypeRange,
@@ -728,6 +717,14 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
           return ExprError(Diag(ArraySize->getSourceRange().getBegin(),
                            diag::err_typecheck_negative_array_size)
             << ArraySize->getSourceRange());
+      } else if (TypeIdParens.isValid()) {
+        // Can't have dynamic array size when the type-id is in parentheses.
+        Diag(ArraySize->getLocStart(), diag::ext_new_paren_array_nonconst)
+          << ArraySize->getSourceRange()
+          << FixItHint::CreateRemoval(TypeIdParens.getBegin())
+          << FixItHint::CreateRemoval(TypeIdParens.getEnd());
+          
+        TypeIdParens = SourceRange();
       }
     }
     
@@ -845,7 +842,7 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
   
   // FIXME: The TypeSourceInfo should also be included in CXXNewExpr.
   return Owned(new (Context) CXXNewExpr(Context, UseGlobal, OperatorNew,
-                                        PlaceArgs, NumPlaceArgs, ParenTypeId,
+                                        PlaceArgs, NumPlaceArgs, TypeIdParens,
                                         ArraySize, Constructor, Init,
                                         ConsArgs, NumConsArgs, OperatorDelete,
                                         ResultType, StartLoc,
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index f3ad6c8b4b..132d04927b 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -1596,7 +1596,7 @@ public:
                                      SourceLocation PlacementLParen,
                                      MultiExprArg PlacementArgs,
                                      SourceLocation PlacementRParen,
-                                     bool ParenTypeId,
+                                     SourceRange TypeIdParens,
                                      QualType AllocType,
                                      SourceLocation TypeLoc,
                                      SourceRange TypeRange,
@@ -1608,7 +1608,7 @@ public:
                                  PlacementLParen,
                                  move(PlacementArgs),
                                  PlacementRParen,
-                                 ParenTypeId,
+                                 TypeIdParens,
                                  AllocType,
                                  TypeLoc,
                                  TypeRange,
@@ -5356,7 +5356,7 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
                                         /*FIXME:*/E->getLocStart(),
                                         move_arg(PlacementArgs),
                                         /*FIXME:*/E->getLocStart(),
-                                        E->isParenTypeId(),
+                                        E->getTypeIdParens(),
                                         AllocType,
                                         /*FIXME:*/E->getLocStart(),
                                         /*FIXME:*/SourceRange(),
diff --git a/test/SemaCXX/new-delete.cpp b/test/SemaCXX/new-delete.cpp
index 34588c2ae1..25bf823b25 100644
--- a/test/SemaCXX/new-delete.cpp
+++ b/test/SemaCXX/new-delete.cpp
@@ -68,7 +68,7 @@ void bad_news(int *ip)
   (void)new int[1.1]; // expected-error {{array size expression must have integral or enumerated type, not 'double'}}
   (void)new int[1][i]; // expected-error {{only the first dimension}}
   (void)new (int[1][i]); // expected-error {{only the first dimension}}
-  (void)new (int[i]); // expected-error {{when type is in parentheses}}
+  (void)new (int[i]); // expected-warning {{when type is in parentheses}}
   (void)new int(*(S*)0); // expected-error {{no viable conversion from 'S' to 'int'}}
   (void)new int(1, 2); // expected-error {{excess elements in scalar initializer}}
   (void)new S(1); // expected-error {{no matching constructor}}
@@ -288,3 +288,25 @@ void test(S1* s1, S2* s2) {
 }
 }
 
+namespace rdar8018245 {
+  struct X0 {
+    static const int value = 17;
+  };
+
+  const int X0::value;
+
+  struct X1 {
+    static int value;
+  };
+
+  int X1::value;
+
+  template<typename T>
+  int *f() {
+    return new (int[T::value]); // expected-warning{{when type is in parentheses, array cannot have dynamic size}}
+  }
+
+  template int *f<X0>();
+  template int *f<X1>(); // expected-note{{in instantiation of}}
+
+}