diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td index 48bafeeff2085d..544573edffdf8b 100644 --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -186,6 +186,9 @@ def note_constexpr_access_inactive_union_member : Note< "construction of subobject of|destruction of}0 " "member %1 of union with %select{active member %3|no active member}2 " "is not allowed in a constant expression">; +def note_constexpr_union_member_change_during_init : Note< + "assignment would change active union member during the initialization of " + "a different member of the same union">; def note_constexpr_access_static_temporary : Note< "%select{read of|read of|assignment to|increment of|decrement of|" "member call on|dynamic_cast of|typeid applied to|reconstruction of|" diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 5bb9fc319d4857..f5c37ad44ebd13 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -674,6 +674,7 @@ namespace { None, Bases, AfterBases, + AfterFields, Destroying, DestroyingBases }; @@ -821,6 +822,9 @@ namespace { void finishedConstructingBases() { EI.ObjectsUnderConstruction[Object] = ConstructionPhase::AfterBases; } + void finishedConstructingFields() { + EI.ObjectsUnderConstruction[Object] = ConstructionPhase::AfterFields; + } ~EvaluatingConstructorRAII() { if (DidInsert) EI.ObjectsUnderConstruction.erase(Object); } @@ -5121,6 +5125,7 @@ static Optional ComputeDynamicType(EvalInfo &Info, const Expr *E, case ConstructionPhase::None: case ConstructionPhase::AfterBases: + case ConstructionPhase::AfterFields: case ConstructionPhase::Destroying: // We've finished constructing the base classes and not yet started // destroying them again, so this is the dynamic type. @@ -5339,7 +5344,10 @@ static bool HandleDynamicCast(EvalInfo &Info, const ExplicitCastExpr *E, namespace { struct StartLifetimeOfUnionMemberHandler { + EvalInfo &Info; + const Expr *LHSExpr; const FieldDecl *Field; + bool DuringInit; static const AccessKinds AccessKind = AK_Assign; @@ -5355,9 +5363,21 @@ struct StartLifetimeOfUnionMemberHandler { // * No variant members' lifetimes begin // * All scalar subobjects whose lifetimes begin have indeterminate values assert(SubobjType->isUnionType()); - if (!declaresSameEntity(Subobj.getUnionField(), Field) || - !Subobj.getUnionValue().hasValue()) - Subobj.setUnion(Field, getDefaultInitValue(Field->getType())); + if (declaresSameEntity(Subobj.getUnionField(), Field)) { + // This union member is already active. If it's also in-lifetime, there's + // nothing to do. + if (Subobj.getUnionValue().hasValue()) + return true; + } else if (DuringInit) { + // We're currently in the process of initializing a different union + // member. If we carried on, that initialization would attempt to + // store to an inactive union member, resulting in undefined behavior. + Info.FFDiag(LHSExpr, + diag::note_constexpr_union_member_change_during_init); + return false; + } + + Subobj.setUnion(Field, getDefaultInitValue(Field->getType())); return true; } bool found(APSInt &Value, QualType SubobjType) { @@ -5460,7 +5480,10 @@ static bool HandleUnionActiveMemberChange(EvalInfo &Info, const Expr *LHSExpr, SubobjectDesignator D = LHS.Designator; D.truncate(Info.Ctx, LHS.Base, LengthAndField.first); - StartLifetimeOfUnionMemberHandler StartLifetime{LengthAndField.second}; + bool DuringInit = Info.isEvaluatingCtorDtor(LHS.Base, D.Entries) == + ConstructionPhase::AfterBases; + StartLifetimeOfUnionMemberHandler StartLifetime{ + Info, LHSExpr, LengthAndField.second, DuringInit}; if (!findSubobject(Info, LHSExpr, Obj, D, StartLifetime)) return false; } @@ -5778,6 +5801,8 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This, } } + EvalObj.finishedConstructingFields(); + return Success && EvaluateStmt(Ret, Info, Definition->getBody()) != ESR_Failed && LifetimeExtendedScope.destroy(); @@ -9180,6 +9205,8 @@ bool RecordExprEvaluator::VisitInitListExpr(const InitListExpr *E) { } } + EvalObj.finishedConstructingFields(); + return Success; } diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp index 01d2a7f58d4818..42d7300c5fdec8 100644 --- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp @@ -1346,3 +1346,38 @@ namespace mutable_subobjects { auto &zti = typeid(z.y); static_assert(&zti == &typeid(Y)); } + +namespace PR45133 { + struct A { long x; }; + + union U; + constexpr A foo(U *up); + + union U { + A a = foo(this); // expected-note {{in call to 'foo(&u)'}} + int y; + }; + + constexpr A foo(U *up) { + up->y = 11; // expected-note {{assignment would change active union member during the initialization of a different member}} + return {42}; + } + + constinit U u = {}; // expected-error {{constant init}} expected-note {{constinit}} + + template struct X {}; + + union V { + int a, b; + constexpr V(X<0>) : a(a = 1) {} // ok + constexpr V(X<1>) : a(b = 1) {} // expected-note {{assignment would change active union member during the initialization of a different member}} + constexpr V(X<2>) : a() { b = 1; } // ok + // This case (changing the active member then changing it back) is debatable, + // but it seems appropriate to reject. + constexpr V(X<3>) : a((b = 1, a = 1)) {} // expected-note {{assignment would change active union member during the initialization of a different member}} + }; + constinit V v0 = X<0>(); + constinit V v1 = X<1>(); // expected-error {{constant init}} expected-note {{constinit}} expected-note {{in call}} + constinit V v2 = X<2>(); + constinit V v3 = X<3>(); // expected-error {{constant init}} expected-note {{constinit}} expected-note {{in call}} +}