From: Richard Smith Date: Tue, 21 May 2019 23:15:20 +0000 (+0000) Subject: [c++20] P1330R0: permit simple-assignments that change the active member X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=beec40f7fdb23833c51c2aa79009ce741b03d49a;p=clang [c++20] P1330R0: permit simple-assignments that change the active member of a union within constant expression evaluation. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@361329 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/APValue.h b/include/clang/AST/APValue.h index 02a6394bd5..7ef88d3996 100644 --- a/include/clang/AST/APValue.h +++ b/include/clang/AST/APValue.h @@ -283,6 +283,11 @@ public: : Kind(None) { MakeAddrLabelDiff(); setAddrLabelDiff(LHSExpr, RHSExpr); } + static APValue IndeterminateValue() { + APValue Result; + Result.Kind = Indeterminate; + return Result; + } ~APValue() { if (Kind != None && Kind != Indeterminate) diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 8a75468f66..143eaae37b 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -3115,6 +3115,13 @@ public: path_const_iterator path_begin() const { return path_buffer(); } path_const_iterator path_end() const { return path_buffer() + path_size(); } + llvm::iterator_range path() { + return llvm::make_range(path_begin(), path_end()); + } + llvm::iterator_range path() const { + return llvm::make_range(path_begin(), path_end()); + } + const FieldDecl *getTargetUnionField() const { assert(getCastKind() == CK_ToUnion); return getTargetFieldForToUnionCast(getType(), getSubExpr()->getType()); diff --git a/include/clang/Basic/DiagnosticASTKinds.td b/include/clang/Basic/DiagnosticASTKinds.td index f1172a91ea..363dc444db 100644 --- a/include/clang/Basic/DiagnosticASTKinds.td +++ b/include/clang/Basic/DiagnosticASTKinds.td @@ -55,6 +55,8 @@ def note_constexpr_non_global : Note< "%select{temporary|%3}2 is not a constant expression">; def note_constexpr_uninitialized : Note< "%select{|sub}0object of type %1 is not initialized">; +def note_constexpr_subobject_declared_here : Note< + "subobject declared here">; def note_constexpr_array_index : Note<"cannot refer to element %0 of " "%select{array of %2 element%plural{1:|:s}2|non-array object}1 " "in a constant expression">; diff --git a/include/clang/Basic/DiagnosticIDs.h b/include/clang/Basic/DiagnosticIDs.h index 0ea0cebed3..8ee9a970b1 100644 --- a/include/clang/Basic/DiagnosticIDs.h +++ b/include/clang/Basic/DiagnosticIDs.h @@ -33,7 +33,7 @@ namespace clang { DIAG_SIZE_SERIALIZATION = 120, DIAG_SIZE_LEX = 400, DIAG_SIZE_PARSE = 500, - DIAG_SIZE_AST = 150, + DIAG_SIZE_AST = 200, DIAG_SIZE_COMMENT = 100, DIAG_SIZE_CROSSTU = 100, DIAG_SIZE_SEMA = 4000, diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index 508456422b..5ec2883778 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -290,6 +290,27 @@ namespace { } } + void truncate(ASTContext &Ctx, APValue::LValueBase Base, + unsigned NewLength) { + if (Invalid) + return; + + assert(Base && "cannot truncate path for null pointer"); + assert(NewLength <= Entries.size() && "not a truncation"); + + if (NewLength == Entries.size()) + return; + Entries.resize(NewLength); + + bool IsArray = false; + bool FirstIsUnsizedArray = false; + MostDerivedPathLength = findMostDerivedSubobject( + Ctx, Base, Entries, MostDerivedArraySize, MostDerivedType, IsArray, + FirstIsUnsizedArray); + MostDerivedIsArrayElement = IsArray; + FirstEntryIsAnUnsizedArray = FirstIsUnsizedArray; + } + void setInvalid() { Invalid = true; Entries.clear(); @@ -2024,10 +2045,13 @@ static bool CheckLiteralType(EvalInfo &Info, const Expr *E, static bool CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc, QualType Type, const APValue &Value, - Expr::ConstExprUsage Usage = Expr::EvaluateForCodeGen) { + Expr::ConstExprUsage Usage = Expr::EvaluateForCodeGen, + SourceLocation SubobjectLoc = SourceLocation()) { if (!Value.hasValue()) { Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << true << Type; + if (SubobjectLoc.isValid()) + Info.Note(SubobjectLoc, diag::note_constexpr_subobject_declared_here); return false; } @@ -2043,18 +2067,20 @@ CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc, QualType Type, QualType EltTy = Type->castAsArrayTypeUnsafe()->getElementType(); for (unsigned I = 0, N = Value.getArrayInitializedElts(); I != N; ++I) { if (!CheckConstantExpression(Info, DiagLoc, EltTy, - Value.getArrayInitializedElt(I), Usage)) + Value.getArrayInitializedElt(I), Usage, + SubobjectLoc)) return false; } if (!Value.hasArrayFiller()) return true; return CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayFiller(), - Usage); + Usage, SubobjectLoc); } if (Value.isUnion() && Value.getUnionField()) { return CheckConstantExpression(Info, DiagLoc, Value.getUnionField()->getType(), - Value.getUnionValue(), Usage); + Value.getUnionValue(), Usage, + Value.getUnionField()->getLocation()); } if (Value.isStruct()) { RecordDecl *RD = Type->castAs()->getDecl(); @@ -2062,7 +2088,8 @@ CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc, QualType Type, unsigned BaseIndex = 0; for (const CXXBaseSpecifier &BS : CD->bases()) { if (!CheckConstantExpression(Info, DiagLoc, BS.getType(), - Value.getStructBase(BaseIndex), Usage)) + Value.getStructBase(BaseIndex), Usage, + BS.getBeginLoc())) return false; ++BaseIndex; } @@ -2073,7 +2100,7 @@ CheckConstantExpression(EvalInfo &Info, SourceLocation DiagLoc, QualType Type, if (!CheckConstantExpression(Info, DiagLoc, I->getType(), Value.getStructField(I->getFieldIndex()), - Usage)) + Usage, I->getLocation())) return false; } } @@ -2972,7 +2999,8 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj, // Walk the designator's path to find the subobject. for (unsigned I = 0, N = Sub.Entries.size(); /**/; ++I) { - if (!O->hasValue()) { + // Reading an indeterminate value is undefined, but assigning over one is OK. + if (O->isAbsent() || (O->isIndeterminate() && handler.AccessKind != AK_Assign)) { if (!Info.checkingPotentialConstantExpression()) Info.FFDiag(E, diag::note_constexpr_access_uninit) << handler.AccessKind << O->isIndeterminate(); @@ -4888,6 +4916,159 @@ static bool HandleDynamicCast(EvalInfo &Info, const ExplicitCastExpr *E, return RuntimeCheckFailed(&Paths); } +namespace { +struct StartLifetimeOfUnionMemberHandler { + const FieldDecl *Field; + + static const AccessKinds AccessKind = AK_Assign; + + APValue getDefaultInitValue(QualType SubobjType) { + if (auto *RD = SubobjType->getAsCXXRecordDecl()) { + if (RD->isUnion()) + return APValue((const FieldDecl*)nullptr); + + APValue Struct(APValue::UninitStruct(), RD->getNumBases(), + std::distance(RD->field_begin(), RD->field_end())); + + unsigned Index = 0; + for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(), + End = RD->bases_end(); I != End; ++I, ++Index) + Struct.getStructBase(Index) = getDefaultInitValue(I->getType()); + + for (const auto *I : RD->fields()) { + if (I->isUnnamedBitfield()) + continue; + Struct.getStructField(I->getFieldIndex()) = + getDefaultInitValue(I->getType()); + } + return Struct; + } + + if (auto *AT = dyn_cast_or_null( + SubobjType->getAsArrayTypeUnsafe())) { + APValue Array(APValue::UninitArray(), 0, AT->getSize().getZExtValue()); + if (Array.hasArrayFiller()) + Array.getArrayFiller() = getDefaultInitValue(AT->getElementType()); + return Array; + } + + return APValue::IndeterminateValue(); + } + + typedef bool result_type; + bool failed() { return false; } + bool found(APValue &Subobj, QualType SubobjType) { + // We are supposed to perform no initialization but begin the lifetime of + // the object. We interpret that as meaning to do what default + // initialization of the object would do if all constructors involved were + // trivial: + // * All base, non-variant member, and array element subobjects' lifetimes + // begin + // * No variant members' lifetimes begin + // * All scalar subobjects whose lifetimes begin have indeterminate values + assert(SubobjType->isUnionType()); + if (!declaresSameEntity(Subobj.getUnionField(), Field)) + Subobj.setUnion(Field, getDefaultInitValue(Field->getType())); + return true; + } + bool found(APSInt &Value, QualType SubobjType) { + llvm_unreachable("wrong value kind for union object"); + } + bool found(APFloat &Value, QualType SubobjType) { + llvm_unreachable("wrong value kind for union object"); + } +}; +} // end anonymous namespace + +const AccessKinds StartLifetimeOfUnionMemberHandler::AccessKind; + +/// Handle a builtin simple-assignment or a call to a trivial assignment +/// operator whose left-hand side might involve a union member access. If it +/// does, implicitly start the lifetime of any accessed union elements per +/// C++20 [class.union]5. +static bool HandleUnionActiveMemberChange(EvalInfo &Info, const Expr *LHSExpr, + const LValue &LHS) { + if (LHS.InvalidBase || LHS.Designator.Invalid) + return false; + + llvm::SmallVector, 4> UnionPathLengths; + // C++ [class.union]p5: + // define the set S(E) of subexpressions of E as follows: + const Expr *E = LHSExpr; + unsigned PathLength = LHS.Designator.Entries.size(); + while (E) { + // -- If E is of the form A.B, S(E) contains the elements of S(A)... + if (auto *ME = dyn_cast(E)) { + auto *FD = dyn_cast(ME->getMemberDecl()); + if (!FD) + break; + + // ... and also contains A.B if B names a union member + if (FD->getParent()->isUnion()) + UnionPathLengths.push_back({PathLength - 1, FD}); + + E = ME->getBase(); + --PathLength; + assert(declaresSameEntity(FD, + LHS.Designator.Entries[PathLength] + .getAsBaseOrMember().getPointer())); + + // -- If E is of the form A[B] and is interpreted as a built-in array + // subscripting operator, S(E) is [S(the array operand, if any)]. + } else if (auto *ASE = dyn_cast(E)) { + // Step over an ArrayToPointerDecay implicit cast. + auto *Base = ASE->getBase()->IgnoreImplicit(); + if (!Base->getType()->isArrayType()) + break; + + E = Base; + --PathLength; + + } else if (auto *ICE = dyn_cast(E)) { + // Step over a derived-to-base conversion. + if (ICE->getCastKind() == CK_NoOp) + continue; + if (ICE->getCastKind() != CK_DerivedToBase && + ICE->getCastKind() != CK_UncheckedDerivedToBase) + break; + for (const CXXBaseSpecifier *Elt : ICE->path()) { + --PathLength; + assert(declaresSameEntity(Elt->getType()->getAsCXXRecordDecl(), + LHS.Designator.Entries[PathLength] + .getAsBaseOrMember().getPointer())); + } + E = ICE->getSubExpr(); + + // -- Otherwise, S(E) is empty. + } else { + break; + } + } + + // Common case: no unions' lifetimes are started. + if (UnionPathLengths.empty()) + return true; + + // if modification of X [would access an inactive union member], an object + // of the type of X is implicitly created + CompleteObject Obj = + findCompleteObject(Info, LHSExpr, AK_Assign, LHS, LHSExpr->getType()); + if (!Obj) + return false; + for (std::pair LengthAndField : + llvm::reverse(UnionPathLengths)) { + // Form a designator for the union object. + SubobjectDesignator D = LHS.Designator; + D.truncate(Info.Ctx, LHS.Base, LengthAndField.first); + + StartLifetimeOfUnionMemberHandler StartLifetime{LengthAndField.second}; + if (!findSubobject(Info, LHSExpr, Obj, D, StartLifetime)) + return false; + } + + return true; +} + /// 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) { @@ -4958,6 +5139,9 @@ static bool HandleFunctionCall(SourceLocation CallLoc, if (!handleLValueToRValueConversion(Info, Args[0], Args[0]->getType(), RHS, RHSValue)) return false; + if (Info.getLangOpts().CPlusPlus2a && MD->isTrivial() && + !HandleUnionActiveMemberChange(Info, Args[0], *This)) + return false; if (!handleAssignment(Info, Args[0], *This, MD->getThisType(), RHSValue)) return false; @@ -6183,6 +6367,10 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) { if (!Evaluate(NewVal, this->Info, E->getRHS())) return false; + if (Info.getLangOpts().CPlusPlus2a && + !HandleUnionActiveMemberChange(Info, E->getLHS(), Result)) + return false; + return handleAssignment(this->Info, E, Result, E->getLHS()->getType(), NewVal); } diff --git a/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp b/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp index e24e63178e..17753604b8 100644 --- a/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp +++ b/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp @@ -328,7 +328,7 @@ namespace PR14503 { int n; struct { int x, - y; + y; // expected-note {{subobject declared here}} }; }; constexpr V() : x(0) {} diff --git a/test/SemaCXX/constant-expression-cxx2a.cpp b/test/SemaCXX/constant-expression-cxx2a.cpp index 90980c8d05..a0f92691c2 100644 --- a/test/SemaCXX/constant-expression-cxx2a.cpp +++ b/test/SemaCXX/constant-expression-cxx2a.cpp @@ -413,3 +413,104 @@ namespace TypeId { } static_assert(side_effects()); } + +namespace Union { + struct Base { + int y; // expected-note {{here}} + }; + struct A : Base { + int x; + int arr[3]; + union { int p, q; }; + }; + union B { + A a; + int b; + }; + constexpr int read_wrong_member() { // expected-error {{never produces a constant}} + B b = {.b = 1}; + return b.a.x; // expected-note {{read of member 'a' of union with active member 'b'}} + } + constexpr int change_member() { + B b = {.b = 1}; + b.a.x = 1; + return b.a.x; + } + static_assert(change_member() == 1); + constexpr int change_member_then_read_wrong_member() { // expected-error {{never produces a constant}} + B b = {.b = 1}; + b.a.x = 1; + return b.b; // expected-note {{read of member 'b' of union with active member 'a'}} + } + constexpr int read_wrong_member_indirect() { // expected-error {{never produces a constant}} + B b = {.b = 1}; + int *p = &b.a.y; + return *p; // expected-note {{read of member 'a' of union with active member 'b'}} + } + constexpr int read_uninitialized() { + B b = {.b = 1}; + int *p = &b.a.y; + b.a.x = 1; + return *p; // expected-note {{read of uninitialized object}} + } + static_assert(read_uninitialized() == 0); // expected-error {{constant}} expected-note {{in call}} + constexpr void write_wrong_member_indirect() { // expected-error {{never produces a constant}} + B b = {.b = 1}; + int *p = &b.a.y; + *p = 1; // expected-note {{assignment to member 'a' of union with active member 'b'}} + } + constexpr int write_uninitialized() { + B b = {.b = 1}; + int *p = &b.a.y; + b.a.x = 1; + *p = 1; + return *p; + } + static_assert(write_uninitialized() == 1); + constexpr int change_member_indirectly() { + B b = {.b = 1}; + b.a.arr[1] = 1; + int &r = b.a.y; + r = 123; + + b.b = 2; + b.a.y = 3; + b.a.arr[2] = 4; + return b.a.arr[2]; + } + static_assert(change_member_indirectly() == 4); + constexpr B return_uninit() { + B b = {.b = 1}; + b.a.x = 2; + return b; + } + constexpr B uninit = return_uninit(); // expected-error {{constant expression}} expected-note {{subobject of type 'int' is not initialized}} + static_assert(return_uninit().a.x == 2); + constexpr A return_uninit_struct() { + B b = {.b = 1}; + b.a.x = 2; + return b.a; + } + // FIXME: It's unclear that this should be valid. Copying a B involves + // copying the object representation of the union, but copying an A invokes a + // copy constructor that copies the object elementwise, and reading from + // b.a.y is undefined. + static_assert(return_uninit_struct().x == 2); + constexpr B return_init_all() { + B b = {.b = 1}; + b.a.x = 2; + b.a.y = 3; + b.a.arr[0] = 4; + b.a.arr[1] = 5; + b.a.arr[2] = 6; + return b; + } + static_assert(return_init_all().a.x == 2); + static_assert(return_init_all().a.y == 3); + static_assert(return_init_all().a.arr[0] == 4); + static_assert(return_init_all().a.arr[1] == 5); + static_assert(return_init_all().a.arr[2] == 6); + static_assert(return_init_all().a.p == 7); // expected-error {{}} expected-note {{read of member 'p' of union with no active member}} + static_assert(return_init_all().a.q == 8); // expected-error {{}} expected-note {{read of member 'q' of union with no active member}} + constexpr B init_all = return_init_all(); +} diff --git a/www/cxx_status.html b/www/cxx_status.html index 968d453a60..901cea861e 100755 --- a/www/cxx_status.html +++ b/www/cxx_status.html @@ -973,11 +973,10 @@ as the draft C++2a standard evolves. P1327R1 - SVN + SVN P1330R0 - No Prohibit aggregates with user-declared constructors