Do not use CXXZeroValueInitExpr for class types. Instead, use

CXXConstructExpr/CXXTemporaryObjectExpr/CXXNewExpr as
appropriate. Fixes PR7556, and provides a slide codegen improvement
when copy-initializing a POD class type from a value-initialized
temporary. Previously, we weren't eliding the copy.


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

10 files changed, 55 insertions, 45 deletions

diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h
index 6da32bacd3..7915ab142a 100644
--- a/include/clang/AST/ExprCXX.h
+++ b/include/clang/AST/ExprCXX.h
@@ -822,12 +822,8 @@ public:
 ///
 /// This expression type represents a C++ "functional" cast
 /// (C++[expr.type.conv]) with N != 1 arguments that invokes a
-/// constructor to build a temporary object. If N == 0 but no
-/// constructor will be called (because the functional cast is
-/// performing a value-initialized an object whose class type has no
-/// user-declared constructors), CXXZeroInitValueExpr will represent
-/// the functional cast. Finally, with N == 1 arguments the functional
-/// cast expression will be represented by CXXFunctionalCastExpr.
+/// constructor to build a temporary object. With N == 1 arguments the 
+/// functional cast expression will be represented by CXXFunctionalCastExpr.
 /// Example:
 /// @code
 /// struct X { X(int, float); }
@@ -863,8 +859,7 @@ public:
 
 /// CXXZeroInitValueExpr - [C++ 5.2.3p2]
 /// Expression "T()" which creates a value-initialized rvalue of type
-/// T, which is either a non-class type or a class type without any
-/// user-defined constructors.
+/// T, which is a non-class type.
 ///
 class CXXZeroInitValueExpr : public Expr {
   SourceLocation TyBeginLoc;
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index f88a0ae664..0883eab003 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -509,6 +509,9 @@ def err_access_dtor_vbase :
 def err_access_dtor_temp :
     Error<"temporary of type %0 has %select{private|protected}1 destructor">,
     NoSFINAE;
+def err_access_dtor_exception :
+    Error<"exception object of type %0 has %select{private|protected}1 "
+          "destructor">, NoSFINAE;
 def err_access_dtor_field :
     Error<"field of type %1 has %select{private|protected}2 destructor">,
     NoSFINAE;
diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index f2e6a11292..ea1753bbc1 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -572,9 +572,18 @@ static void EmitNewInitializer(CodeGenFunction &CGF, const CXXNewExpr *E,
   }
 
   if (CXXConstructorDecl *Ctor = E->getConstructor()) {
-    CGF.EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false, 
-                               NewPtr, E->constructor_arg_begin(),
-                               E->constructor_arg_end());
+    // Per C++ [expr.new]p15, if we have an initializer, then we're performing
+    // direct initialization. C++ [dcl.init]p5 requires that we 
+    // zero-initialize storage if there are no user-declared constructors.
+    if (E->hasInitializer() && 
+        !Ctor->getParent()->hasUserDeclaredConstructor() &&
+        !Ctor->getParent()->isEmpty())
+      CGF.EmitNullInitialization(NewPtr, E->getAllocatedType());
+      
+    if (!Ctor->isTrivial())
+      CGF.EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false, 
+                                 NewPtr, E->constructor_arg_begin(),
+                                 E->constructor_arg_end());
 
     return;
   }
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 2c631064ef..3619193434 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -480,7 +480,7 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *&E) {
 
   MarkDeclarationReferenced(E->getExprLoc(), Destructor);
   CheckDestructorAccess(E->getExprLoc(), Destructor,
-                        PDiag(diag::err_access_dtor_temp) << Ty);
+                        PDiag(diag::err_access_dtor_exception) << Ty);
   return false;
 }
 
@@ -566,27 +566,19 @@ Sema::ActOnCXXTypeConstructExpr(SourceRange TypeRange, TypeTy *TypeRep,
                                                      RParenLoc));
   }
 
-  if (const RecordType *RT = Ty->getAs<RecordType>()) {
-    CXXRecordDecl *Record = cast<CXXRecordDecl>(RT->getDecl());
-
-    if (NumExprs > 1 || !Record->hasTrivialConstructor() ||
-        !Record->hasTrivialDestructor()) {
-      InitializedEntity Entity = InitializedEntity::InitializeTemporary(Ty);
-      InitializationKind Kind
-        = NumExprs ? InitializationKind::CreateDirect(TypeRange.getBegin(), 
-                                                      LParenLoc, RParenLoc)
-                   : InitializationKind::CreateValue(TypeRange.getBegin(), 
-                                                     LParenLoc, RParenLoc);
-      InitializationSequence InitSeq(*this, Entity, Kind, Exprs, NumExprs);
-      OwningExprResult Result = InitSeq.Perform(*this, Entity, Kind,
-                                                move(exprs));
-
-      // FIXME: Improve AST representation?
-      return move(Result);
-    }
-
-    // Fall through to value-initialize an object of class type that
-    // doesn't have a user-declared default constructor.
+  if (Ty->isRecordType()) {
+    InitializedEntity Entity = InitializedEntity::InitializeTemporary(Ty);
+    InitializationKind Kind
+      = NumExprs ? InitializationKind::CreateDirect(TypeRange.getBegin(), 
+                                                    LParenLoc, RParenLoc)
+                 : InitializationKind::CreateValue(TypeRange.getBegin(), 
+                                                   LParenLoc, RParenLoc);
+    InitializationSequence InitSeq(*this, Entity, Kind, Exprs, NumExprs);
+    OwningExprResult Result = InitSeq.Perform(*this, Entity, Kind,
+                                              move(exprs));
+
+    // FIXME: Improve AST representation?
+    return move(Result);
   }
 
   // C++ [expr.type.conv]p1:
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 536222c37f..6b7ff63372 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -2772,8 +2772,7 @@ static void TryValueInitialization(Sema &S,
       //    zero-initialized and, if T’s implicitly-declared default
       //    constructor is non-trivial, that constructor is called.
       if ((ClassDecl->getTagKind() == TTK_Class ||
-           ClassDecl->getTagKind() == TTK_Struct) &&
-          !ClassDecl->hasTrivialConstructor()) {
+           ClassDecl->getTagKind() == TTK_Struct)) {
         Sequence.AddZeroInitializationStep(Entity.getType());
         return TryConstructorInitialization(S, Entity, Kind, 0, 0, T, Sequence);        
       }
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 1abcf205dd..9c8f48bfea 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -92,12 +92,6 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) {
   
   if (const CXXExprWithTemporaries *Temps = dyn_cast<CXXExprWithTemporaries>(E))
     E = Temps->getSubExpr();
-  if (const CXXZeroInitValueExpr *Zero = dyn_cast<CXXZeroInitValueExpr>(E)) {
-    if (const RecordType *RecordT = Zero->getType()->getAs<RecordType>())
-      if (CXXRecordDecl *RecordD = dyn_cast<CXXRecordDecl>(RecordT->getDecl()))
-        if (!RecordD->hasTrivialDestructor())
-          return;
-  }
       
   if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
     if (E->getType()->isVoidType())
diff --git a/test/CXX/class.access/p4.cpp b/test/CXX/class.access/p4.cpp
index 552c52f977..90a1449610 100644
--- a/test/CXX/class.access/p4.cpp
+++ b/test/CXX/class.access/p4.cpp
@@ -423,6 +423,7 @@ namespace test15 {
 
 // PR7281
 namespace test16 {
-  class A { ~A(); }; // expected-note {{declared private here}}
-  void b() { throw A(); } // expected-error{{temporary of type 'test16::A' has private destructor}}
+  class A { ~A(); }; // expected-note 2{{declared private here}}
+  void b() { throw A(); } // expected-error{{temporary of type 'test16::A' has private destructor}} \
+  // expected-error{{exception object of type 'test16::A' has private destructor}}
 }
diff --git a/test/CodeGenCXX/default-arg-temps.cpp b/test/CodeGenCXX/default-arg-temps.cpp
index e4a06770cd..c4419850f1 100644
--- a/test/CodeGenCXX/default-arg-temps.cpp
+++ b/test/CodeGenCXX/default-arg-temps.cpp
@@ -44,7 +44,6 @@ class obj{ int a; float b; double d; };
 // CHECK: define void @_Z1hv()
 void h() {
   // CHECK: call void @llvm.memset.p0i8.i64(
-  // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(
   obj o = obj();
 }
 
diff --git a/test/CodeGenCXX/temporaries.cpp b/test/CodeGenCXX/temporaries.cpp
index eb543cb545..9a397abfc0 100644
--- a/test/CodeGenCXX/temporaries.cpp
+++ b/test/CodeGenCXX/temporaries.cpp
@@ -320,3 +320,21 @@ namespace UserConvertToValue {
     f(1);
   }
 }
+
+namespace PR7556 {
+  struct A { ~A(); }; 
+  struct B { int i; ~B(); }; 
+  struct C { int C::*pm; ~C(); }; 
+  // CHECK: define void @_ZN6PR75563fooEv()
+  void foo() { 
+    // CHECK: call void @_ZN6PR75561AD1Ev
+    A(); 
+    // CHECK: call void @llvm.memset.p0i8.i64
+    // CHECK: call void @_ZN6PR75561BD1Ev
+    B();
+    // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64
+    // CHECK: call void @_ZN6PR75561CD1Ev
+    C();
+    // CHECK-NEXT: ret void
+  }
+}
diff --git a/test/SemaCXX/warn-unused-variables.cpp b/test/SemaCXX/warn-unused-variables.cpp
index 5ef7e7002f..6992cdcd09 100644
--- a/test/SemaCXX/warn-unused-variables.cpp
+++ b/test/SemaCXX/warn-unused-variables.cpp
@@ -26,7 +26,7 @@ namespace PR5531 {
   };
 
   void test() {
-    A(); // expected-warning{{expression result unused}}
+    A();
     B(17);
     C();
   }