From 63ef464c3fad1e8b9f9360baa6c81f974b712e90 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 7 Jul 2010 22:35:13 +0000 Subject: [PATCH] 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 --- include/clang/AST/ExprCXX.h | 11 ++----- include/clang/Basic/DiagnosticSemaKinds.td | 3 ++ lib/CodeGen/CGExprCXX.cpp | 15 +++++++-- lib/Sema/SemaExprCXX.cpp | 36 +++++++++------------- lib/Sema/SemaInit.cpp | 3 +- lib/Sema/SemaStmt.cpp | 6 ---- test/CXX/class.access/p4.cpp | 5 +-- test/CodeGenCXX/default-arg-temps.cpp | 1 - test/CodeGenCXX/temporaries.cpp | 18 +++++++++++ test/SemaCXX/warn-unused-variables.cpp | 2 +- 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()) { - CXXRecordDecl *Record = cast(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(E)) E = Temps->getSubExpr(); - if (const CXXZeroInitValueExpr *Zero = dyn_cast(E)) { - if (const RecordType *RecordT = Zero->getType()->getAs()) - if (CXXRecordDecl *RecordD = dyn_cast(RecordT->getDecl())) - if (!RecordD->hasTrivialDestructor()) - return; - } if (const CallExpr *CE = dyn_cast(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(); } -- 2.40.0