r236142 - PR23373: A defaulted union copy constructor that is not trivial must still be
Richard Smith
richard-llvm at metafoo.co.uk
Wed Apr 29 12:26:57 PDT 2015
Author: rsmith
Date: Wed Apr 29 14:26:57 2015
New Revision: 236142
URL: http://llvm.org/viewvc/llvm-project?rev=236142&view=rev
Log:
PR23373: A defaulted union copy constructor that is not trivial must still be
emitted as a memcpy.
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=236142&r1=236141&r2=236142&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Apr 29 14:26:57 2015
@@ -3727,8 +3727,9 @@ static bool HandleFunctionCall(SourceLoc
// Skip this for non-union classes with no fields; in that case, the defaulted
// copy/move does not actually read the object.
const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Callee);
- if (MD && MD->isDefaulted() && MD->isTrivial() &&
- (MD->getParent()->isUnion() || hasFields(MD->getParent()))) {
+ if (MD && MD->isDefaulted() &&
+ (MD->getParent()->isUnion() ||
+ (MD->isTrivial() && hasFields(MD->getParent())))) {
assert(This &&
(MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()));
LValue RHS;
@@ -3792,11 +3793,9 @@ static bool HandleConstructorCall(Source
// Skip this for empty non-union classes; we should not perform an
// lvalue-to-rvalue conversion on them because their copy constructor does not
// actually read them.
- if (Definition->isDefaulted() &&
- ((Definition->isCopyConstructor() && Definition->isTrivial()) ||
- (Definition->isMoveConstructor() && Definition->isTrivial())) &&
+ if (Definition->isDefaulted() && Definition->isCopyOrMoveConstructor() &&
(Definition->getParent()->isUnion() ||
- hasFields(Definition->getParent()))) {
+ (Definition->isTrivial() && hasFields(Definition->getParent())))) {
LValue RHS;
RHS.setFrom(Info.Ctx, ArgValues[0]);
return handleLValueToRValueConversion(Info, Args[0], Args[0]->getType(),
Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=236142&r1=236141&r2=236142&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Apr 29 14:26:57 2015
@@ -540,6 +540,23 @@ static void EmitAggMemberInitializer(Cod
CGF.EmitBlock(AfterFor, true);
}
+static bool isMemcpyEquivalentSpecialMember(const CXXMethodDecl *D) {
+ auto *CD = dyn_cast<CXXConstructorDecl>(D);
+ if (!(CD && CD->isCopyOrMoveConstructor()) &&
+ !D->isCopyAssignmentOperator() && !D->isMoveAssignmentOperator())
+ return false;
+
+ // We can emit a memcpy for a trivial copy or move constructor/assignment.
+ if (D->isTrivial() && !D->getParent()->mayInsertExtraPadding())
+ return true;
+
+ // We *must* emit a memcpy for a defaulted union copy or move op.
+ if (D->getParent()->isUnion() && D->isDefaulted())
+ return true;
+
+ return false;
+}
+
static void EmitMemberInitializer(CodeGenFunction &CGF,
const CXXRecordDecl *ClassDecl,
CXXCtorInitializer *MemberInit,
@@ -581,7 +598,7 @@ static void EmitMemberInitializer(CodeGe
QualType BaseElementTy = CGF.getContext().getBaseElementType(Array);
CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(MemberInit->getInit());
if (BaseElementTy.isPODType(CGF.getContext()) ||
- (CE && CE->getConstructor()->isTrivial())) {
+ (CE && isMemcpyEquivalentSpecialMember(CE->getConstructor()))) {
unsigned SrcArgIndex =
CGF.CGM.getCXXABI().getSrcArgforCopyCtor(Constructor, Args);
llvm::Value *SrcPtr
@@ -1021,8 +1038,8 @@ namespace {
QualType FieldType = Field->getType();
CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(MemberInit->getInit());
- // Bail out on non-POD, not-trivially-constructable members.
- if (!(CE && CE->getConstructor()->isTrivial()) &&
+ // Bail out on non-memcpyable, not-trivially-copyable members.
+ if (!(CE && isMemcpyEquivalentSpecialMember(CE->getConstructor())) &&
!(FieldType.isTriviallyCopyableType(CGF.getContext()) ||
FieldType->isReferenceType()))
return false;
@@ -1127,9 +1144,7 @@ namespace {
return Field;
} else if (CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(S)) {
CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(MCE->getCalleeDecl());
- if (!(MD && (MD->isCopyAssignmentOperator() ||
- MD->isMoveAssignmentOperator()) &&
- MD->isTrivial()))
+ if (!(MD && isMemcpyEquivalentSpecialMember(MD)))
return nullptr;
MemberExpr *IOA = dyn_cast<MemberExpr>(MCE->getImplicitObjectArgument());
if (!IOA)
@@ -1732,18 +1747,23 @@ void CodeGenFunction::EmitCXXConstructor
bool ForVirtualBase,
bool Delegating, llvm::Value *This,
const CXXConstructExpr *E) {
- // If this is a trivial constructor, just emit what's needed.
- if (D->isTrivial() && !D->getParent()->mayInsertExtraPadding()) {
- if (E->getNumArgs() == 0) {
- // Trivial default constructor, no codegen required.
- assert(D->isDefaultConstructor() &&
- "trivial 0-arg ctor not a default ctor");
- return;
- }
+ // C++11 [class.mfct.non-static]p2:
+ // If a non-static member function of a class X is called for an object that
+ // is not of type X, or of a type derived from X, the behavior is undefined.
+ // FIXME: Provide a source location here.
+ EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, SourceLocation(), This,
+ getContext().getRecordType(D->getParent()));
+ if (D->isTrivial() && D->isDefaultConstructor()) {
+ assert(E->getNumArgs() == 0 && "trivial default ctor with args");
+ return;
+ }
+
+ // If this is a trivial constructor, just emit what's needed. If this is a
+ // union copy constructor, we must emit a memcpy, because the AST does not
+ // model that copy.
+ if (isMemcpyEquivalentSpecialMember(D)) {
assert(E->getNumArgs() == 1 && "unexpected argcount for trivial ctor");
- assert(D->isCopyOrMoveConstructor() &&
- "trivial 1-arg ctor not a copy/move ctor");
const Expr *Arg = E->getArg(0);
QualType SrcTy = Arg->getType();
@@ -1753,13 +1773,6 @@ void CodeGenFunction::EmitCXXConstructor
return;
}
- // C++11 [class.mfct.non-static]p2:
- // If a non-static member function of a class X is called for an object that
- // is not of type X, or of a type derived from X, the behavior is undefined.
- // FIXME: Provide a source location here.
- EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, SourceLocation(), This,
- getContext().getRecordType(D->getParent()));
-
CallArgList Args;
// Push the this ptr.
@@ -1784,8 +1797,7 @@ void
CodeGenFunction::EmitSynthesizedCXXCopyCtorCall(const CXXConstructorDecl *D,
llvm::Value *This, llvm::Value *Src,
const CXXConstructExpr *E) {
- if (D->isTrivial() &&
- !D->getParent()->mayInsertExtraPadding()) {
+ if (isMemcpyEquivalentSpecialMember(D)) {
assert(E->getNumArgs() == 1 && "unexpected argcount for trivial ctor");
assert(D->isCopyOrMoveConstructor() &&
"trivial 1-arg ctor not a copy/move ctor");
Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=236142&r1=236141&r2=236142&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Wed Apr 29 14:26:57 2015
@@ -1415,7 +1415,8 @@ void CodeGenFunction::EmitAggregateCopy(
assert((Record->hasTrivialCopyConstructor() ||
Record->hasTrivialCopyAssignment() ||
Record->hasTrivialMoveConstructor() ||
- Record->hasTrivialMoveAssignment()) &&
+ Record->hasTrivialMoveAssignment() ||
+ Record->isUnion()) &&
"Trying to aggregate-copy a type without a trivial copy/move "
"constructor or assignment operator");
// Ignore empty classes in C++.
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=236142&r1=236141&r2=236142&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Wed Apr 29 14:26:57 2015
@@ -173,7 +173,7 @@ RValue CodeGenFunction::EmitCXXMemberOrO
This = EmitLValue(Base).getAddress();
- if (MD->isTrivial()) {
+ if (MD->isTrivial() || (MD->isDefaulted() && MD->getParent()->isUnion())) {
if (isa<CXXDestructorDecl>(MD)) return RValue::get(nullptr);
if (isa<CXXConstructorDecl>(MD) &&
cast<CXXConstructorDecl>(MD)->isDefaultConstructor())
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=236142&r1=236141&r2=236142&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Apr 29 14:26:57 2015
@@ -10214,7 +10214,9 @@ void Sema::DefineImplicitCopyAssignment(
// Assign non-static members.
for (auto *Field : ClassDecl->fields()) {
- if (Field->isUnnamedBitfield())
+ // FIXME: We should form some kind of AST representation for the implied
+ // memcpy in a union copy operation.
+ if (Field->isUnnamedBitfield() || Field->getParent()->isUnion())
continue;
if (Field->isInvalidDecl()) {
@@ -10644,7 +10646,9 @@ void Sema::DefineImplicitMoveAssignment(
// Assign non-static members.
for (auto *Field : ClassDecl->fields()) {
- if (Field->isUnnamedBitfield())
+ // FIXME: We should form some kind of AST representation for the implied
+ // memcpy in a union copy operation.
+ if (Field->isUnnamedBitfield() || Field->getParent()->isUnion())
continue;
if (Field->isInvalidDecl()) {
Modified: cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp?rev=236142&r1=236141&r2=236142&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp Wed Apr 29 14:26:57 2015
@@ -1,4 +1,24 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+union PR23373 {
+ PR23373(PR23373&) = default;
+ PR23373 &operator=(PR23373&) = default;
+ int n;
+ float f;
+};
+extern PR23373 pr23373_a;
+
+PR23373 pr23373_b(pr23373_a);
+// CHECK-LABEL: define {{.*}} @__cxx_global_var_init(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} @pr23373_b{{.*}}, {{.*}} @pr23373_a{{.*}}, i64 4, i32 4, i1 false)
+
+PR23373 pr23373_f() { return pr23373_a; }
+// CHECK-LABEL: define {{.*}} @_Z9pr23373_fv(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i64 4, i32 4, i1 false)
+
+void pr23373_g(PR23373 &a, PR23373 &b) { a = b; }
+// CHECK-LABEL: define {{.*}} @_Z9pr23373_g
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i64 4, i32 4, i1 false)
struct A { virtual void a(); };
A x(A& y) { return y; }
More information about the cfe-commits
mailing list