From be6dd818fbf1305c895038904396d20761d3f98c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 19 Nov 2014 21:27:17 +0000 Subject: [PATCH] Fix bug where a trivial constexpr copy/move operation couldn't copy from an empty non-constexpr object. Such a copy doesn't break any of the constexpr rules. llvm-svn: 222387 --- clang/lib/AST/ExprConstant.cpp | 35 +++++++++++++++++++++--- clang/test/SemaCXX/constant-expression-cxx11.cpp | 13 +++++++++ clang/test/SemaCXX/constant-expression-cxx1y.cpp | 28 +++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index ea28d74..9a2c552 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -3657,6 +3657,22 @@ static bool CheckConstexprFunction(EvalInfo &Info, SourceLocation CallLoc, return false; } +/// Determine if a class has any fields that might need to be copied by a +/// trivial copy or move operation. +static bool hasFields(const CXXRecordDecl *RD) { + if (!RD || RD->isEmpty()) + return false; + for (auto *FD : RD->fields()) { + if (FD->isUnnamedBitfield()) + continue; + return true; + } + for (auto &Base : RD->bases()) + if (hasFields(Base.getType()->getAsCXXRecordDecl())) + return true; + return false; +} + namespace { typedef SmallVector ArgVector; } @@ -3695,8 +3711,12 @@ static bool HandleFunctionCall(SourceLocation CallLoc, // For a trivial copy or move assignment, perform an APValue copy. This is // essential for unions, where the operations performed by the assignment // operator cannot be represented as statements. + // + // 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(Callee); - if (MD && MD->isDefaulted() && MD->isTrivial()) { + if (MD && MD->isDefaulted() && MD->isTrivial() && + (MD->getParent()->isUnion() || hasFields(MD->getParent()))) { assert(This && (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator())); LValue RHS; @@ -3753,11 +3773,18 @@ static bool HandleConstructorCall(SourceLocation CallLoc, const LValue &This, } // For a trivial copy or move constructor, perform an APValue copy. This is - // essential for unions, where the operations performed by the constructor - // cannot be represented by ctor-initializers. + // essential for unions (or classes with anonymous union members), where the + // operations performed by the constructor cannot be represented by + // ctor-initializers. + // + // 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()))) { + (Definition->isMoveConstructor() && Definition->isTrivial())) && + (Definition->getParent()->isUnion() || + hasFields(Definition->getParent()))) { LValue RHS; RHS.setFrom(Info.Ctx, ArgValues[0]); return handleLValueToRValueConversion(Info, Args[0], Args[0]->getType(), diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp index f2ce282..d50dd0b 100644 --- a/clang/test/SemaCXX/constant-expression-cxx11.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp @@ -1942,3 +1942,16 @@ namespace PR19010 { void PR21327(int a, int b) { static_assert(&a + 1 != &b, ""); // expected-error {{constant expression}} } + +namespace EmptyClass { + struct E1 {} e1; + union E2 {} e2; // expected-note {{here}} + struct E3 : E1 {} e3; + + // The defaulted copy constructor for an empty class does not read any + // members. The defaulted copy constructor for an empty union reads the + // object representation. + constexpr E1 e1b(e1); + constexpr E2 e2b(e2); // expected-error {{constant expression}} expected-note{{read of non-const}} expected-note {{in call}} + constexpr E3 e3b(e3); +} diff --git a/clang/test/SemaCXX/constant-expression-cxx1y.cpp b/clang/test/SemaCXX/constant-expression-cxx1y.cpp index e1929f7..e8925f3 100644 --- a/clang/test/SemaCXX/constant-expression-cxx1y.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx1y.cpp @@ -910,3 +910,31 @@ namespace PR17331 { constexpr int ARR[] = { 1, 2, 3, 4, 5 }; static_assert(sum(ARR) == 15, ""); } + +namespace EmptyClass { + struct E1 {} e1; + union E2 {} e2; // expected-note 4{{here}} + struct E3 : E1 {} e3; + + template + constexpr int f(E &a, int kind) { + switch (kind) { + case 0: { E e(a); return 0; } // expected-note {{read}} expected-note {{in call}} + case 1: { E e(static_cast(a)); return 0; } // expected-note {{read}} expected-note {{in call}} + case 2: { E e; e = a; return 0; } // expected-note {{read}} expected-note {{in call}} + case 3: { E e; e = static_cast(a); return 0; } // expected-note {{read}} expected-note {{in call}} + } + } + constexpr int test1 = f(e1, 0); + constexpr int test2 = f(e2, 0); // expected-error {{constant expression}} expected-note {{in call}} + constexpr int test3 = f(e3, 0); + constexpr int test4 = f(e1, 1); + constexpr int test5 = f(e2, 1); // expected-error {{constant expression}} expected-note {{in call}} + constexpr int test6 = f(e3, 1); + constexpr int test7 = f(e1, 2); + constexpr int test8 = f(e2, 2); // expected-error {{constant expression}} expected-note {{in call}} + constexpr int test9 = f(e3, 2); + constexpr int testa = f(e1, 3); + constexpr int testb = f(e2, 3); // expected-error {{constant expression}} expected-note {{in call}} + constexpr int testc = f(e3, 3); +} -- 2.7.4