Revert "Make CXXNewExpr contain only a single initialier, and not hold the used constructor itself."

It leads to a compiler crash in the Bullet benchmark.

This reverts commit r12014.

llvm-svn: 150684

3 files changed, 125 insertions, 113 deletions

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6b66a42ad34..7fee518d2cc 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -10148,13 +10148,15 @@ namespace {
     }
     
     void VisitCXXNewExpr(CXXNewExpr *E) {
+      if (E->getConstructor())
+        S.MarkFunctionReferenced(E->getLocStart(), E->getConstructor());
       if (E->getOperatorNew())
         S.MarkFunctionReferenced(E->getLocStart(), E->getOperatorNew());
       if (E->getOperatorDelete())
         S.MarkFunctionReferenced(E->getLocStart(), E->getOperatorDelete());
       Inherited::VisitCXXNewExpr(E);
     }
-
+    
     void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
       if (E->getOperatorDelete())
         S.MarkFunctionReferenced(E->getLocStart(), E->getOperatorDelete());
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index dfa2148fc31..4ffdb3e5c15 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -914,7 +914,9 @@ ExprResult
 Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
                   SourceLocation PlacementLParen, MultiExprArg PlacementArgs,
                   SourceLocation PlacementRParen, SourceRange TypeIdParens,
-                  Declarator &D, Expr *Initializer) {
+                  Declarator &D, SourceLocation ConstructorLParen,
+                  MultiExprArg ConstructorArgs,
+                  SourceLocation ConstructorRParen) {
   bool TypeContainsAuto = D.getDeclSpec().getTypeSpecType() == DeclSpec::TST_auto;
 
   Expr *ArraySize = 0;
@@ -959,10 +961,6 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
   if (D.isInvalidType())
     return ExprError();
 
-  SourceRange DirectInitRange;
-  if (ParenListExpr *List = dyn_cast_or_null<ParenListExpr>(Initializer))
-    DirectInitRange = List->getSourceRange();
-
   return BuildCXXNew(StartLoc, UseGlobal,
                      PlacementLParen,
                      move(PlacementArgs),
@@ -971,27 +969,12 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
                      AllocType,
                      TInfo,
                      ArraySize,
-                     DirectInitRange,
-                     Initializer,
+                     ConstructorLParen,
+                     move(ConstructorArgs),
+                     ConstructorRParen,
                      TypeContainsAuto);
 }
 
-static bool isLegalArrayNewInitializer(Expr *Init) {
-  if (!Init)
-    return true;
-  if (ParenListExpr *PLE = dyn_cast<ParenListExpr>(Init)) {
-    if (PLE->getNumExprs() != 1)
-      return PLE->getNumExprs() == 0;
-    Init = PLE->getExpr(0);
-  }
-  if (isa<ImplicitValueInitExpr>(Init))
-    return true;
-  else if (CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(Init))
-    return !CCE->isListInitialization() &&
-           CCE->getConstructor()->isDefaultConstructor();
-  return false;
-}
-
 ExprResult
 Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
                   SourceLocation PlacementLParen,
@@ -1001,56 +984,29 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
                   QualType AllocType,
                   TypeSourceInfo *AllocTypeInfo,
                   Expr *ArraySize,
-                  SourceRange DirectInitRange,
-                  Expr *Initializer,
+                  SourceLocation ConstructorLParen,
+                  MultiExprArg ConstructorArgs,
+                  SourceLocation ConstructorRParen,
                   bool TypeMayContainAuto) {
   SourceRange TypeRange = AllocTypeInfo->getTypeLoc().getSourceRange();
 
-  CXXNewExpr::InitializationStyle initStyle;
-  if (DirectInitRange.isValid()) {
-    assert(Initializer && "Have parens but no initializer.");
-    initStyle = CXXNewExpr::CallInit;
-  } else if (Initializer && isa<InitListExpr>(Initializer))
-    initStyle = CXXNewExpr::ListInit;
-  else {
-    assert((!Initializer || isa<ImplicitValueInitExpr>(Initializer) ||
-            isa<CXXConstructExpr>(Initializer)) &&
-           "Initializer expression that cannot have been implicitly created.");
-    initStyle = CXXNewExpr::NoInit;
-  }
-
-  Expr **Inits = &Initializer;
-  unsigned NumInits = Initializer ? 1 : 0;
-  if (initStyle == CXXNewExpr::CallInit) {
-    if (ParenListExpr *List = dyn_cast<ParenListExpr>(Initializer)) {
-      Inits = List->getExprs();
-      NumInits = List->getNumExprs();
-    } else if (CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(Initializer)){
-      if (!isa<CXXTemporaryObjectExpr>(CCE)) {
-        // Can happen in template instantiation. Since this is just an implicit
-        // construction, we just take it apart and rebuild it.
-        Inits = CCE->getArgs();
-        NumInits = CCE->getNumArgs();
-      }
-    }
-  }
-
   // C++0x [decl.spec.auto]p6. Deduce the type which 'auto' stands in for.
   if (TypeMayContainAuto && AllocType->getContainedAutoType()) {
-    if (initStyle == CXXNewExpr::NoInit || NumInits == 0)
+    if (ConstructorArgs.size() == 0)
       return ExprError(Diag(StartLoc, diag::err_auto_new_requires_ctor_arg)
                        << AllocType << TypeRange);
-    if (initStyle == CXXNewExpr::ListInit)
-      return ExprError(Diag(Inits[0]->getSourceRange().getBegin(),
-                            diag::err_auto_new_requires_parens)
-                       << AllocType << TypeRange);
-    if (NumInits > 1) {
-      Expr *FirstBad = Inits[1];
+    if (ConstructorArgs.size() != 1) {
+      Expr *FirstBad = ConstructorArgs.get()[1];
       return ExprError(Diag(FirstBad->getSourceRange().getBegin(),
                             diag::err_auto_new_ctor_multiple_expressions)
                        << AllocType << TypeRange);
     }
-    Expr *Deduce = Inits[0];
+    Expr *Deduce = ConstructorArgs.get()[0];
+    if (ConstructorLParen.isInvalid()) {
+      return ExprError(Diag(Deduce->getSourceRange().getBegin(),
+                            diag::err_auto_new_requires_parens)
+                       << AllocType << TypeRange);
+    }
     TypeSourceInfo *DeducedType = 0;
     if (DeduceAutoType(AllocTypeInfo, Deduce, DeducedType) ==
             DAR_Failed)
@@ -1079,10 +1035,15 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
   if (CheckAllocatedType(AllocType, TypeRange.getBegin(), TypeRange))
     return ExprError();
 
-  if (initStyle == CXXNewExpr::ListInit && isStdInitializerList(AllocType, 0)) {
+  bool ListInitialization = ConstructorLParen.isInvalid() &&
+                            ConstructorArgs.size() > 0;
+  assert((!ListInitialization || (ConstructorArgs.size() == 1 &&
+                                  isa<InitListExpr>(ConstructorArgs.get()[0])))
+         && "List initialization means a braced-init-list for arguments.");
+  if (ListInitialization && isStdInitializerList(AllocType, 0)) {
     Diag(AllocTypeInfo->getTypeLoc().getBeginLoc(),
          diag::warn_dangling_std_initializer_list)
-      << /*at end of FE*/0 << Inits[0]->getSourceRange();
+      << /*at end of FE*/0 << ConstructorArgs.get()[0]->getSourceRange();
   }
 
   // In ARC, infer 'retaining' for the allocated 
@@ -1240,18 +1201,25 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
     }
   }
 
-  // Array 'new' can't have any initializers except empty parentheses.
-  if (!isLegalArrayNewInitializer(Initializer) &&
-      (ResultType->isArrayType() || ArraySize)) {
-    SourceRange InitRange(Inits[0]->getLocStart(),
-                          Inits[NumInits - 1]->getLocEnd());
+  bool Init = ConstructorLParen.isValid() || ConstructorArgs.size() > 0;
+  // --- Choosing a constructor ---
+  CXXConstructorDecl *Constructor = 0;
+  bool HadMultipleCandidates = false;
+  Expr **ConsArgs = (Expr**)ConstructorArgs.get();
+  unsigned NumConsArgs = ConstructorArgs.size();
+  ASTOwningVector<Expr*> ConvertedConstructorArgs(*this);
+
+  // Array 'new' can't have any initializers.
+  if (NumConsArgs && (ResultType->isArrayType() || ArraySize)) {
+    SourceRange InitRange(ConsArgs[0]->getLocStart(),
+                          ConsArgs[NumConsArgs - 1]->getLocEnd());
 
     Diag(StartLoc, diag::err_new_array_init_args) << InitRange;
     return ExprError();
   }
 
   if (!AllocType->isDependentType() &&
-      !Expr::hasAnyTypeDependentArguments(Inits, NumInits)) {
+      !Expr::hasAnyTypeDependentArguments(ConsArgs, NumConsArgs)) {
     // C++11 [expr.new]p15:
     //   A new-expression that creates an object of type T initializes that
     //   object as follows:
@@ -1259,31 +1227,49 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
     //     - If the new-initializer is omitted, the object is default-
     //       initialized (8.5); if no initialization is performed,
     //       the object has indeterminate value
-      = initStyle == CXXNewExpr::NoInit
-          ? InitializationKind::CreateDefault(TypeRange.getBegin())
+      = !Init? InitializationKind::CreateDefault(TypeRange.getBegin())
     //     - Otherwise, the new-initializer is interpreted according to the
     //       initialization rules of 8.5 for direct-initialization.
-          : initStyle == CXXNewExpr::ListInit
-              ? InitializationKind::CreateDirectList(TypeRange.getBegin())
-              : InitializationKind::CreateDirect(TypeRange.getBegin(),
-                                                 DirectInitRange.getBegin(),
-                                                 DirectInitRange.getEnd());
+             : ListInitialization ? InitializationKind::CreateDirectList(
+                                                          TypeRange.getBegin())
+                                  : InitializationKind::CreateDirect(
+                                                          TypeRange.getBegin(),
+                                                          ConstructorLParen,
+                                                          ConstructorRParen);
 
     InitializedEntity Entity
       = InitializedEntity::InitializeNew(StartLoc, AllocType);
-    InitializationSequence InitSeq(*this, Entity, Kind, Inits, NumInits);
+    InitializationSequence InitSeq(*this, Entity, Kind, ConsArgs, NumConsArgs);
     ExprResult FullInit = InitSeq.Perform(*this, Entity, Kind,
-                                          MultiExprArg(Inits, NumInits));
+                                                move(ConstructorArgs));
     if (FullInit.isInvalid())
       return ExprError();
 
-    // FullInit is our initializer; strip off CXXBindTemporaryExprs, because
-    // we don't want the initialized object to be destructed.
-    if (CXXBindTemporaryExpr *Binder =
-            dyn_cast_or_null<CXXBindTemporaryExpr>(FullInit.get()))
-      FullInit = Owned(Binder->getSubExpr());
+    // FullInit is our initializer; walk through it to determine if it's a
+    // constructor call, which CXXNewExpr handles directly.
+    if (Expr *FullInitExpr = (Expr *)FullInit.get()) {
+      if (CXXBindTemporaryExpr *Binder
+            = dyn_cast<CXXBindTemporaryExpr>(FullInitExpr))
+        FullInitExpr = Binder->getSubExpr();
+      if (CXXConstructExpr *Construct
+                    = dyn_cast<CXXConstructExpr>(FullInitExpr)) {
+        Constructor = Construct->getConstructor();
+        HadMultipleCandidates = Construct->hadMultipleCandidates();
+        for (CXXConstructExpr::arg_iterator A = Construct->arg_begin(),
+                                         AEnd = Construct->arg_end();
+             A != AEnd; ++A)
+          ConvertedConstructorArgs.push_back(*A);
+      } else {
+        // Take the converted initializer.
+        ConvertedConstructorArgs.push_back(FullInit.release());
+      }
+    } else {
+      // No initialization required.
+    }
 
-    Initializer = FullInit.take();
+    // Take the converted arguments and use them for the new expression.
+    NumConsArgs = ConvertedConstructorArgs.size();
+    ConsArgs = (Expr **)ConvertedConstructorArgs.take();
   }
 
   // Mark the new and delete operators as referenced.
@@ -1295,9 +1281,8 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
   // C++0x [expr.new]p17:
   //   If the new expression creates an array of objects of class type,
   //   access and ambiguity control are done for the destructor.
-  if (ArraySize && AllocType->isRecordType()) {
-    if (CXXDestructorDecl *dtor = LookupDestructor(
-            cast<CXXRecordDecl>(AllocType->getAs<RecordType>()->getDecl()))) {
+  if (ArraySize && Constructor) {
+    if (CXXDestructorDecl *dtor = LookupDestructor(Constructor->getParent())) {
       MarkFunctionReferenced(StartLoc, dtor);
       CheckDestructorAccess(StartLoc, dtor, 
                             PDiag(diag::err_access_dtor)
@@ -1306,18 +1291,25 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal,
   }
 
   PlacementArgs.release();
+  ConstructorArgs.release();
 
   return Owned(new (Context) CXXNewExpr(Context, UseGlobal, OperatorNew,
+                                        PlaceArgs, NumPlaceArgs, TypeIdParens,
+                                        ArraySize, Constructor, Init,
+                                        ConsArgs, NumConsArgs,
+                                        HadMultipleCandidates,
                                         OperatorDelete,
                                         UsualArrayDeleteWantsSize,
-                                        PlaceArgs, NumPlaceArgs, TypeIdParens,
-                                        ArraySize, initStyle, Initializer,
                                         ResultType, AllocTypeInfo,
-                                        StartLoc, DirectInitRange));
+                                        StartLoc,
+                                        Init ? ConstructorRParen :
+                                               TypeRange.getEnd(),
+                                        ConstructorLParen, ConstructorRParen));
 }
 
-/// \brief Checks that a type is suitable as the allocated type
+/// CheckAllocatedType - Checks that a type is suitable as the allocated type
 /// in a new-expression.
+/// dimension off and stores the size expression in ArraySize.
 bool Sema::CheckAllocatedType(QualType AllocType, SourceLocation Loc,
                               SourceRange R) {
   // C++ 5.3.4p1: "[The] type shall be a complete object type, but not an
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 88173555e92..3293f7468d8 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -1976,8 +1976,9 @@ public:
                                QualType AllocatedType,
                                TypeSourceInfo *AllocatedTypeInfo,
                                Expr *ArraySize,
-                               SourceRange DirectInitRange,
-                               Expr *Initializer) {
+                               SourceLocation ConstructorLParen,
+                               MultiExprArg ConstructorArgs,
+                               SourceLocation ConstructorRParen) {
     return getSema().BuildCXXNew(StartLoc, UseGlobal,
                                  PlacementLParen,
                                  move(PlacementArgs),
@@ -1986,8 +1987,9 @@ public:
                                  AllocatedType,
                                  AllocatedTypeInfo,
                                  ArraySize,
-                                 DirectInitRange,
-                                 Initializer);
+                                 ConstructorLParen,
+                                 move(ConstructorArgs),
+                                 ConstructorRParen);
   }
 
   /// \brief Build a new C++ "delete" expression.
@@ -7104,17 +7106,29 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
   if (getDerived().TransformExprs(E->getPlacementArgs(), 
                                   E->getNumPlacementArgs(), true,
                                   PlacementArgs, &ArgumentChanged))
-    return ExprError();
-
-  // Transform the initializer (if any).
-  Expr *OldInit = E->getInitializer();
-  ExprResult NewInit;
-  if (OldInit)
-    NewInit = getDerived().TransformExpr(OldInit);
-  if (NewInit.isInvalid())
-    return ExprError();
+    return ExprError();  
+
+  // Transform the constructor arguments (if any).
+  // As an annoying corner case, we may have introduced an implicit value-
+  // initialization expression when allocating a new array, which we implicitly
+  // drop. It will be re-created during type checking.
+  ASTOwningVector<Expr*> ConstructorArgs(SemaRef);
+  if (!(E->isArray() && E->getNumConstructorArgs() == 1 &&
+        isa<ImplicitValueInitExpr>(E->getConstructorArgs()[0])) &&
+      TransformExprs(E->getConstructorArgs(), E->getNumConstructorArgs(), true,
+                     ConstructorArgs, &ArgumentChanged))
+    return ExprError();  
+
+  // Transform constructor, new operator, and delete operator.
+  CXXConstructorDecl *Constructor = 0;
+  if (E->getConstructor()) {
+    Constructor = cast_or_null<CXXConstructorDecl>(
+                                   getDerived().TransformDecl(E->getLocStart(),
+                                                         E->getConstructor()));
+    if (!Constructor)
+      return ExprError();
+  }
 
-  // Transform new operator and delete operator.
   FunctionDecl *OperatorNew = 0;
   if (E->getOperatorNew()) {
     OperatorNew = cast_or_null<FunctionDecl>(
@@ -7136,18 +7150,21 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
   if (!getDerived().AlwaysRebuild() &&
       AllocTypeInfo == E->getAllocatedTypeSourceInfo() &&
       ArraySize.get() == E->getArraySize() &&
-      NewInit.get() == OldInit &&
+      Constructor == E->getConstructor() &&
       OperatorNew == E->getOperatorNew() &&
       OperatorDelete == E->getOperatorDelete() &&
       !ArgumentChanged) {
     // Mark any declarations we need as referenced.
     // FIXME: instantiation-specific.
+    if (Constructor)
+      SemaRef.MarkFunctionReferenced(E->getLocStart(), Constructor);
     if (OperatorNew)
       SemaRef.MarkFunctionReferenced(E->getLocStart(), OperatorNew);
     if (OperatorDelete)
       SemaRef.MarkFunctionReferenced(E->getLocStart(), OperatorDelete);
     
-    if (E->isArray() && !E->getAllocatedType()->isDependentType()) {
+    if (E->isArray() && Constructor && 
+        !E->getAllocatedType()->isDependentType()) {
       QualType ElementType
         = SemaRef.Context.getBaseElementType(E->getAllocatedType());
       if (const RecordType *RecordT = ElementType->getAs<RecordType>()) {
@@ -7157,7 +7174,7 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
         }
       }
     }
-
+    
     return SemaRef.Owned(E);
   }
 
@@ -7187,7 +7204,7 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
       }
     }
   }
-
+  
   return getDerived().RebuildCXXNewExpr(E->getLocStart(),
                                         E->isGlobalNew(),
                                         /*FIXME:*/E->getLocStart(),
@@ -7197,8 +7214,9 @@ TreeTransform<Derived>::TransformCXXNewExpr(CXXNewExpr *E) {
                                         AllocType,
                                         AllocTypeInfo,
                                         ArraySize.get(),
-                                        E->getDirectInitRange(),
-                                        NewInit.take());
+                                        E->getConstructorLParen(),
+                                        move_arg(ConstructorArgs),
+                                        E->getConstructorRParen());
 }
 
 template<typename Derived>