| Message ID | ZQ2sQNoVz0rblzDN@Thaum.localdomain |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] c++: Check for indirect change of active union member in constexpr [PR101631,PR102286] | expand |
Now that bootstrap has finished, I have gotten regressions in the following libstdc++ tests: Running libstdc++:libstdc++-dg/conformance.exp ... FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++23 (test for excess errors) FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++26 (test for excess errors) FAIL: 20_util/variant/constexpr.cc -std=gnu++20 (test for excess errors) FAIL: 20_util/variant/constexpr.cc -std=gnu++26 (test for excess errors) FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++20 (test for excess errors) FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++26 (test for excess errors) FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++20 (test for excess errors) FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++26 (test for excess errors) FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc -std=gnu++20 (test for excess errors) FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc -std=gnu++26 (test for excess errors) FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++20 (test for excess errors) FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++26 (test for excess errors) FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++23 (test for excess errors) UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++23 compilation failed to produce executable FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++26 (test for excess errors) UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++26 compilation failed to produce executable On investigation though it looks like the issue might be with libstdc++ rather than the patch itself; running the failing tests using clang with libstdc++ also produces similar errors, and my reading of the code suggests that this is correct. What's the way forward here? Should I look at creating a patch to fix the libstdc++ issues before resubmitting this patch for the C++ frontend? Or should I submit a version of this patch without the `std::construct_at` changes and wait till libstdc++ gets fixed for that? On Sat, Sep 23, 2023 at 01:01:20AM +1000, Nathaniel Shead wrote: > On Fri, Sep 22, 2023 at 02:21:15PM +0100, Jason Merrill wrote: > > On 9/21/23 09:41, Nathaniel Shead wrote: > > > I've updated the error messages, and also fixed another bug I found > > > while retesting (value-initialised unions weren't considered to have any > > > active member yet). > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu. > > > > > > -- >8 -- > > > > > > This patch adds checks for attempting to change the active member of a > > > union by methods other than a member access expression. > > > > > > To be able to properly distinguish `*(&u.a) = ` from `u.a = `, this > > > patch redoes the solution for c++/59950 to avoid extranneous *&; it > > > seems that the only case that needed the workaround was when copying > > > empty classes. > > > > > > This patch also ensures that constructors for a union field mark that > > > field as the active member before entering the call itself; this ensures > > > that modifications of the field within the constructor's body don't > > > cause false positives (as these will not appear to be member access > > > expressions). This means that we no longer need to start the lifetime of > > > empty union members after the constructor body completes. > > > > > > As a drive-by fix, this patch also ensures that value-initialised unions > > > are considered to have activated their initial member for the purpose of > > > checking stores, which catches some additional mistakes pre-C++20. > > > > > > PR c++/101631 > > > > > > gcc/cp/ChangeLog: > > > > > > * call.cc (build_over_call): Fold more indirect refs for trivial > > > assignment op. > > > * class.cc (type_has_non_deleted_trivial_default_ctor): Create. > > > * constexpr.cc (cxx_eval_call_expression): Start lifetime of > > > union member before entering constructor. > > > (cxx_eval_store_expression): Activate member for > > > value-initialised union. Check for accessing inactive union > > > member indirectly. > > > * cp-tree.h (type_has_non_deleted_trivial_default_ctor): > > > Forward declare. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * g++.dg/cpp1y/constexpr-89336-3.C: Fix union initialisation. > > > * g++.dg/cpp1y/constexpr-union6.C: New test. > > > * g++.dg/cpp2a/constexpr-union2.C: New test. > > > * g++.dg/cpp2a/constexpr-union3.C: New test. > > > * g++.dg/cpp2a/constexpr-union4.C: New test. > > > * g++.dg/cpp2a/constexpr-union5.C: New test. > > > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > > --- > > > gcc/cp/call.cc | 11 +- > > > gcc/cp/class.cc | 8 ++ > > > gcc/cp/constexpr.cc | 129 +++++++++++++----- > > > gcc/cp/cp-tree.h | 1 + > > > .../g++.dg/cpp1y/constexpr-89336-3.C | 2 +- > > > gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C | 13 ++ > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C | 30 ++++ > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C | 45 ++++++ > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C | 29 ++++ > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C | 71 ++++++++++ > > > 10 files changed, 296 insertions(+), 43 deletions(-) > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > > > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > > > index e8dafbd8ba6..c1fb8807d3f 100644 > > > --- a/gcc/cp/call.cc > > > +++ b/gcc/cp/call.cc > > > @@ -10330,10 +10330,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) > > > && DECL_OVERLOADED_OPERATOR_IS (fn, NOP_EXPR) > > > && trivial_fn_p (fn)) > > > { > > > - /* Don't use cp_build_fold_indirect_ref, op= returns an lvalue even if > > > - the object argument isn't one. */ > > > - tree to = cp_build_indirect_ref (input_location, argarray[0], > > > - RO_ARROW, complain); > > > + tree to = cp_build_fold_indirect_ref (argarray[0]); > > > tree type = TREE_TYPE (to); > > > tree as_base = CLASSTYPE_AS_BASE (type); > > > tree arg = argarray[1]; > > > @@ -10341,7 +10338,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) > > > if (is_really_empty_class (type, /*ignore_vptr*/true)) > > > { > > > - /* Avoid copying empty classes. */ > > > + /* Avoid copying empty classes, but ensure op= returns an lvalue even > > > + if the object argument isn't one. This isn't needed in other cases > > > + since MODIFY_EXPR is always considered an lvalue. */ > > > + to = cp_build_addr_expr (to, tf_none); > > > + to = cp_build_indirect_ref (input_location, to, RO_ARROW, complain); > > > val = build2 (COMPOUND_EXPR, type, arg, to); > > > suppress_warning (val, OPT_Wunused); > > > } > > > diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc > > > index b71333af1f8..e31aeb8e68b 100644 > > > --- a/gcc/cp/class.cc > > > +++ b/gcc/cp/class.cc > > > @@ -5688,6 +5688,14 @@ type_has_virtual_destructor (tree type) > > > return (dtor && DECL_VIRTUAL_P (dtor)); > > > } > > > +/* True iff class TYPE has a non-deleted trivial default > > > + constructor. */ > > > + > > > +bool type_has_non_deleted_trivial_default_ctor (tree type) > > > +{ > > > + return TYPE_HAS_TRIVIAL_DFLT (type) && locate_ctor (type); > > > +} > > > + > > > /* Returns true iff T, a class, has a move-assignment or > > > move-constructor. Does not lazily declare either. > > > If USER_P is false, any move function will do. If it is true, the > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > > > index a673a6022f1..5e50fc2c792 100644 > > > --- a/gcc/cp/constexpr.cc > > > +++ b/gcc/cp/constexpr.cc > > > @@ -3146,40 +3146,34 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > > > cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/false, > > > non_constant_p, overflow_p); > > > + /* If this is a constructor, we are beginning the lifetime of the > > > + object we are initializing. */ > > > + if (new_obj > > > + && DECL_CONSTRUCTOR_P (fun) > > > + && TREE_CODE (new_obj) == COMPONENT_REF > > > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (new_obj, 0))) == UNION_TYPE) > > > + { > > > + tree activate = build2 (INIT_EXPR, TREE_TYPE (new_obj), > > > + new_obj, > > > + build_constructor (TREE_TYPE (new_obj), > > > + NULL)); > > > + cxx_eval_constant_expression (ctx, activate, > > > + lval, non_constant_p, overflow_p); > > > + ggc_free (activate); > > > + } > > > + > > > tree jump_target = NULL_TREE; > > > cxx_eval_constant_expression (&call_ctx, body, > > > vc_discard, non_constant_p, overflow_p, > > > &jump_target); > > > if (DECL_CONSTRUCTOR_P (fun)) > > > - { > > > - /* This can be null for a subobject constructor call, in > > > - which case what we care about is the initialization > > > - side-effects rather than the value. We could get at the > > > - value by evaluating *this, but we don't bother; there's > > > - no need to put such a call in the hash table. */ > > > - result = lval ? ctx->object : ctx->ctor; > > > - > > > - /* If we've just evaluated a subobject constructor call for an > > > - empty union member, it might not have produced a side effect > > > - that actually activated the union member. So produce such a > > > - side effect now to ensure the union appears initialized. */ > > > - if (!result && new_obj > > > - && TREE_CODE (new_obj) == COMPONENT_REF > > > - && TREE_CODE (TREE_TYPE > > > - (TREE_OPERAND (new_obj, 0))) == UNION_TYPE > > > - && is_really_empty_class (TREE_TYPE (new_obj), > > > - /*ignore_vptr*/false)) > > > - { > > > - tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj), > > > - new_obj, > > > - build_constructor (TREE_TYPE (new_obj), > > > - NULL)); > > > - cxx_eval_constant_expression (ctx, activate, lval, > > > - non_constant_p, overflow_p); > > > - ggc_free (activate); > > > - } > > > - } > > > + /* This can be null for a subobject constructor call, in > > > + which case what we care about is the initialization > > > + side-effects rather than the value. We could get at the > > > + value by evaluating *this, but we don't bother; there's > > > + no need to put such a call in the hash table. */ > > > + result = lval ? ctx->object : ctx->ctor; > > > else if (VOID_TYPE_P (TREE_TYPE (res))) > > > result = void_node; > > > else > > > @@ -6127,6 +6121,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > mutable_p) > > > && const_object_being_modified == NULL_TREE) > > > const_object_being_modified = probe; > > > + > > > + /* Track named member accesses for unions to validate modifications > > > + that change active member. */ > > > + if (!evaluated && TREE_CODE (probe) == COMPONENT_REF) > > > + vec_safe_push (refs, probe); > > > + else > > > + vec_safe_push (refs, NULL_TREE); > > > + > > > vec_safe_push (refs, elt); > > > vec_safe_push (refs, TREE_TYPE (probe)); > > > probe = ob; > > > @@ -6135,6 +6137,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > case REALPART_EXPR: > > > gcc_assert (probe == target); > > > + vec_safe_push (refs, NULL_TREE); > > > vec_safe_push (refs, probe); > > > vec_safe_push (refs, TREE_TYPE (probe)); > > > probe = TREE_OPERAND (probe, 0); > > > @@ -6142,6 +6145,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > case IMAGPART_EXPR: > > > gcc_assert (probe == target); > > > + vec_safe_push (refs, NULL_TREE); > > > vec_safe_push (refs, probe); > > > vec_safe_push (refs, TREE_TYPE (probe)); > > > probe = TREE_OPERAND (probe, 0); > > > @@ -6230,6 +6234,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > enum tree_code code = TREE_CODE (type); > > > tree reftype = refs->pop(); > > > tree index = refs->pop(); > > > + bool is_access_expr = refs->pop() != NULL_TREE; > > > if (code == COMPLEX_TYPE) > > > { > > > @@ -6270,21 +6275,72 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > > type = reftype; > > > - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp) > > > - && CONSTRUCTOR_ELT (*valp, 0)->index != index) > > > + if (code == UNION_TYPE > > > + && TREE_CODE (t) == MODIFY_EXPR > > > + && (CONSTRUCTOR_NELTS (*valp) == 0 > > > + || CONSTRUCTOR_ELT (*valp, 0)->index != index)) > > > { > > > - if (cxx_dialect < cxx20) > > > + /* Probably a change of active member for a union. Ensure that > > > + this is valid. */ > > > + no_zero_init = true; > > > + > > > + tree init = *valp; > > > + if (!CONSTRUCTOR_NO_CLEARING (*valp) && CONSTRUCTOR_NELTS (*valp) == 0) > > > + /* This is a value-initialized union, process the initialization > > > + to ensure that the active member is properly set. */ > > > + init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error); > > > + > > > + bool has_active_member = CONSTRUCTOR_NELTS (init) != 0; > > > + tree inner = strip_array_types (reftype); > > > + > > > + if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index) > > > + /* After handling value-initialization, this wasn't actually a > > > + change of active member like we initially thought. */ > > > + no_zero_init = false; > > > + else if (has_active_member && cxx_dialect < cxx20) > > > { > > > if (!ctx->quiet) > > > error_at (cp_expr_loc_or_input_loc (t), > > > "change of the active member of a union " > > > - "from %qD to %qD", > > > - CONSTRUCTOR_ELT (*valp, 0)->index, > > > + "from %qD to %qD is not a constant expression " > > > + "before C++20", > > > + CONSTRUCTOR_ELT (init, 0)->index, > > > index); > > > *non_constant_p = true; > > > } > > > - else if (TREE_CODE (t) == MODIFY_EXPR > > > - && CONSTRUCTOR_NO_CLEARING (*valp)) > > > + else if (!is_access_expr > > > + || (CLASS_TYPE_P (inner) > > > + && !type_has_non_deleted_trivial_default_ctor (inner))) > > > + { > > > + /* Diagnose changing active union member after initialization > > > + without a valid member access expression, as described in > > > + [class.union.general] p5. */ > > > + if (!ctx->quiet) > > > + { > > > + if (has_active_member) > > > + error_at (cp_expr_loc_or_input_loc (t), > > > + "accessing %qD member instead of initialized " > > > + "%qD member in constant expression", > > > + index, CONSTRUCTOR_ELT (init, 0)->index); > > > + else > > > + error_at (cp_expr_loc_or_input_loc (t), > > > + "accessing uninitialized member %qD", > > > + index); > > > + if (is_access_expr) > > > + inform (DECL_SOURCE_LOCATION (index), > > > + "%qD does not implicitly begin its lifetime " > > > + "because %qT does not have a non-deleted " > > > + "trivial default constructor", > > > + index, inner); > > > > It now occurs to me that this should suggest using placement new instead. > > And that we should test using placement new to begin the lifetime of such > > members. > > > > Jason > > > > Good point. On testing placement new I found I wasn't checking for > indirect change of active member during initialisation either, but this > was pretty trivial to add. How does this look? > > Bootstrap + regtest ongoing, but dg.exp completed with no errors. I'm > away from my computer next week so I thought I'd send this through now > anyway. > > -- >8 -- > > This patch adds checks for attempting to change the active member of a > union by methods other than a member access expression. > > To be able to properly distinguish `*(&u.a) = ` from `u.a = `, this > patch redoes the solution for c++/59950 to avoid extranneous *&; it > seems that the only case that needed the workaround was when copying > empty classes. > > This patch also ensures that constructors for a union field mark that > field as the active member before entering the call itself; this ensures > that modifications of the field within the constructor's body don't > cause false positives (as these will not appear to be member access > expressions). This means that we no longer need to start the lifetime of > empty union members after the constructor body completes. > > As a drive-by fix, this patch also ensures that value-initialised unions > are considered to have activated their initial member for the purpose of > checking stores, which catches some additional mistakes pre-C++20. > > PR c++/101631 > PR c++/102286 > > gcc/cp/ChangeLog: > > * call.cc (build_over_call): Fold more indirect refs for trivial > assignment op. > * class.cc (type_has_non_deleted_trivial_default_ctor): Create. > * constexpr.cc (cxx_eval_call_expression): Start lifetime of > union member before entering constructor. > (cxx_eval_store_expression): Activate member for > value-initialised union. Check for accessing inactive union > member indirectly. > * cp-tree.h (type_has_non_deleted_trivial_default_ctor): > Forward declare. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp1y/constexpr-89336-3.C: Fix union initialisation. > * g++.dg/cpp1y/constexpr-union6.C: New test. > * g++.dg/cpp2a/constexpr-union2.C: New test. > * g++.dg/cpp2a/constexpr-union3.C: New test. > * g++.dg/cpp2a/constexpr-union4.C: New test. > * g++.dg/cpp2a/constexpr-union5.C: New test. > * g++.dg/cpp2a/constexpr-union6.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/call.cc | 11 +- > gcc/cp/class.cc | 8 ++ > gcc/cp/constexpr.cc | 135 +++++++++++++----- > gcc/cp/cp-tree.h | 1 + > .../g++.dg/cpp1y/constexpr-89336-3.C | 2 +- > gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C | 13 ++ > gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C | 30 ++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C | 45 ++++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C | 29 ++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C | 71 +++++++++ > gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C | 43 ++++++ > 11 files changed, 345 insertions(+), 43 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > index e8dafbd8ba6..c1fb8807d3f 100644 > --- a/gcc/cp/call.cc > +++ b/gcc/cp/call.cc > @@ -10330,10 +10330,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) > && DECL_OVERLOADED_OPERATOR_IS (fn, NOP_EXPR) > && trivial_fn_p (fn)) > { > - /* Don't use cp_build_fold_indirect_ref, op= returns an lvalue even if > - the object argument isn't one. */ > - tree to = cp_build_indirect_ref (input_location, argarray[0], > - RO_ARROW, complain); > + tree to = cp_build_fold_indirect_ref (argarray[0]); > tree type = TREE_TYPE (to); > tree as_base = CLASSTYPE_AS_BASE (type); > tree arg = argarray[1]; > @@ -10341,7 +10338,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) > > if (is_really_empty_class (type, /*ignore_vptr*/true)) > { > - /* Avoid copying empty classes. */ > + /* Avoid copying empty classes, but ensure op= returns an lvalue even > + if the object argument isn't one. This isn't needed in other cases > + since MODIFY_EXPR is always considered an lvalue. */ > + to = cp_build_addr_expr (to, tf_none); > + to = cp_build_indirect_ref (input_location, to, RO_ARROW, complain); > val = build2 (COMPOUND_EXPR, type, arg, to); > suppress_warning (val, OPT_Wunused); > } > diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc > index b71333af1f8..e31aeb8e68b 100644 > --- a/gcc/cp/class.cc > +++ b/gcc/cp/class.cc > @@ -5688,6 +5688,14 @@ type_has_virtual_destructor (tree type) > return (dtor && DECL_VIRTUAL_P (dtor)); > } > > +/* True iff class TYPE has a non-deleted trivial default > + constructor. */ > + > +bool type_has_non_deleted_trivial_default_ctor (tree type) > +{ > + return TYPE_HAS_TRIVIAL_DFLT (type) && locate_ctor (type); > +} > + > /* Returns true iff T, a class, has a move-assignment or > move-constructor. Does not lazily declare either. > If USER_P is false, any move function will do. If it is true, the > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index a673a6022f1..8e6f60d03d2 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -3146,40 +3146,34 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/false, > non_constant_p, overflow_p); > > + /* If this is a constructor, we are beginning the lifetime of the > + object we are initializing. */ > + if (new_obj > + && DECL_CONSTRUCTOR_P (fun) > + && TREE_CODE (new_obj) == COMPONENT_REF > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (new_obj, 0))) == UNION_TYPE) > + { > + tree activate = build2 (INIT_EXPR, TREE_TYPE (new_obj), > + new_obj, > + build_constructor (TREE_TYPE (new_obj), > + NULL)); > + cxx_eval_constant_expression (ctx, activate, > + lval, non_constant_p, overflow_p); > + ggc_free (activate); > + } > + > tree jump_target = NULL_TREE; > cxx_eval_constant_expression (&call_ctx, body, > vc_discard, non_constant_p, overflow_p, > &jump_target); > > if (DECL_CONSTRUCTOR_P (fun)) > - { > - /* This can be null for a subobject constructor call, in > - which case what we care about is the initialization > - side-effects rather than the value. We could get at the > - value by evaluating *this, but we don't bother; there's > - no need to put such a call in the hash table. */ > - result = lval ? ctx->object : ctx->ctor; > - > - /* If we've just evaluated a subobject constructor call for an > - empty union member, it might not have produced a side effect > - that actually activated the union member. So produce such a > - side effect now to ensure the union appears initialized. */ > - if (!result && new_obj > - && TREE_CODE (new_obj) == COMPONENT_REF > - && TREE_CODE (TREE_TYPE > - (TREE_OPERAND (new_obj, 0))) == UNION_TYPE > - && is_really_empty_class (TREE_TYPE (new_obj), > - /*ignore_vptr*/false)) > - { > - tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj), > - new_obj, > - build_constructor (TREE_TYPE (new_obj), > - NULL)); > - cxx_eval_constant_expression (ctx, activate, lval, > - non_constant_p, overflow_p); > - ggc_free (activate); > - } > - } > + /* This can be null for a subobject constructor call, in > + which case what we care about is the initialization > + side-effects rather than the value. We could get at the > + value by evaluating *this, but we don't bother; there's > + no need to put such a call in the hash table. */ > + result = lval ? ctx->object : ctx->ctor; > else if (VOID_TYPE_P (TREE_TYPE (res))) > result = void_node; > else > @@ -6127,6 +6121,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > mutable_p) > && const_object_being_modified == NULL_TREE) > const_object_being_modified = probe; > + > + /* Track named member accesses for unions to validate modifications > + that change active member. */ > + if (!evaluated && TREE_CODE (probe) == COMPONENT_REF) > + vec_safe_push (refs, probe); > + else > + vec_safe_push (refs, NULL_TREE); > + > vec_safe_push (refs, elt); > vec_safe_push (refs, TREE_TYPE (probe)); > probe = ob; > @@ -6135,6 +6137,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > case REALPART_EXPR: > gcc_assert (probe == target); > + vec_safe_push (refs, NULL_TREE); > vec_safe_push (refs, probe); > vec_safe_push (refs, TREE_TYPE (probe)); > probe = TREE_OPERAND (probe, 0); > @@ -6142,6 +6145,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > case IMAGPART_EXPR: > gcc_assert (probe == target); > + vec_safe_push (refs, NULL_TREE); > vec_safe_push (refs, probe); > vec_safe_push (refs, TREE_TYPE (probe)); > probe = TREE_OPERAND (probe, 0); > @@ -6230,6 +6234,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > enum tree_code code = TREE_CODE (type); > tree reftype = refs->pop(); > tree index = refs->pop(); > + bool is_access_expr = refs->pop() != NULL_TREE; > > if (code == COMPLEX_TYPE) > { > @@ -6270,21 +6275,78 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > > type = reftype; > > - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp) > - && CONSTRUCTOR_ELT (*valp, 0)->index != index) > + /* Check for implicit change of active member for a union. */ > + if (code == UNION_TYPE > + && (TREE_CODE (t) == MODIFY_EXPR > + /* Also check if initializations have implicit change of active > + member earlier up the access chain. */ > + || !refs->is_empty()) > + && (CONSTRUCTOR_NELTS (*valp) == 0 > + || CONSTRUCTOR_ELT (*valp, 0)->index != index)) > { > - if (cxx_dialect < cxx20) > + no_zero_init = true; > + > + tree init = *valp; > + if (TREE_CODE (t) == MODIFY_EXPR > + && !CONSTRUCTOR_NO_CLEARING (*valp) > + && CONSTRUCTOR_NELTS (*valp) == 0) > + /* Modifying a value-initialized union, process the initialization > + to ensure that the active member is properly set. */ > + init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error); > + > + bool has_active_member = CONSTRUCTOR_NELTS (init) != 0; > + tree inner = strip_array_types (reftype); > + > + if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index) > + /* After handling value-initialization, this wasn't actually a > + change of active member like we initially thought. */ > + no_zero_init = false; > + else if (has_active_member && cxx_dialect < cxx20) > { > if (!ctx->quiet) > error_at (cp_expr_loc_or_input_loc (t), > "change of the active member of a union " > - "from %qD to %qD", > - CONSTRUCTOR_ELT (*valp, 0)->index, > + "from %qD to %qD is not a constant expression " > + "before C++20", > + CONSTRUCTOR_ELT (init, 0)->index, > index); > *non_constant_p = true; > } > - else if (TREE_CODE (t) == MODIFY_EXPR > - && CONSTRUCTOR_NO_CLEARING (*valp)) > + else if (!is_access_expr > + || (TREE_CODE (t) == MODIFY_EXPR > + && CLASS_TYPE_P (inner) > + && !type_has_non_deleted_trivial_default_ctor (inner))) > + { > + /* Diagnose changing active union member after initialization > + without a valid member access expression, as described in > + [class.union.general] p5. */ > + if (!ctx->quiet) > + { > + if (has_active_member) > + error_at (cp_expr_loc_or_input_loc (t), > + "accessing %qD member instead of initialized " > + "%qD member in constant expression", > + index, CONSTRUCTOR_ELT (init, 0)->index); > + else > + error_at (cp_expr_loc_or_input_loc (t), > + "accessing uninitialized member %qD", > + index); > + if (is_access_expr) > + inform (DECL_SOURCE_LOCATION (index), > + "%qD does not implicitly begin its lifetime " > + "because %qT does not have a non-deleted " > + "trivial default constructor, use " > + "%<std::construct_at%> instead", > + index, inner); > + else > + inform (DECL_SOURCE_LOCATION (index), > + "initializing %qD requires a member access " > + "expression as the left operand of the assignment", > + index); > + } > + *non_constant_p = true; > + } > + else if (has_active_member && CONSTRUCTOR_NO_CLEARING (init)) > { > /* Diagnose changing the active union member while the union > is in the process of being initialized. */ > @@ -6292,11 +6354,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, > error_at (cp_expr_loc_or_input_loc (t), > "change of the active member of a union " > "from %qD to %qD during initialization", > - CONSTRUCTOR_ELT (*valp, 0)->index, > + CONSTRUCTOR_ELT (init, 0)->index, > index); > *non_constant_p = true; > } > - no_zero_init = true; > } > > ctors.safe_push (valp); > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index fd6bf9fd4a0..40aa9f0be2b 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -6815,6 +6815,7 @@ extern bool trivial_default_constructor_is_constexpr (tree); > extern bool type_has_constexpr_default_constructor (tree); > extern bool type_has_constexpr_destructor (tree); > extern bool type_has_virtual_destructor (tree); > +extern bool type_has_non_deleted_trivial_default_ctor (tree); > extern bool classtype_has_move_assign_or_move_ctor_p (tree, bool user_declared); > extern bool classtype_has_non_deleted_move_ctor (tree); > extern tree classtype_has_depr_implicit_copy (tree); > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > index 9d370ddefcf..6a60966f6f8 100644 > --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > @@ -18,7 +18,7 @@ constexpr int > bar () > { > union U { int a[5]; long b; }; > - union V { union U u; short v; }; > + union V { short v; union U u; }; > V w {}; > w.v = 5; > w.u.a[3] = w.u.a[1] = w.v; // { dg-error "change of the active member of a union from" "" { target c++17_down } } > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > new file mode 100644 > index 00000000000..ff7ebf19f89 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > @@ -0,0 +1,13 @@ > +// { dg-do compile { target c++14 } } > + > +union U { > + int a; > + float b; > +}; > + > +constexpr bool foo() { > + U u {}; > + u.b = 1.0f; // { dg-error "change of the active member" "" { target c++17_down } } > + return u.b == 1.0f; > +} > +constexpr bool x = foo(); > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > new file mode 100644 > index 00000000000..1712395d7e7 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > @@ -0,0 +1,30 @@ > +// PR c++/101631 > +// { dg-do compile { target c++20 } } > + > +struct sso { > + union { > + int buf[10]; > + int* alloc; > + }; > +}; > + > +constexpr bool direct() { > + sso val; > + val.alloc = nullptr; > + val.buf[5] = 42; > + return true; > +} > +constexpr bool ok = direct(); > + > + > +constexpr void perform_assignment(int& left, int right) noexcept { > + left = right; // { dg-error "accessing .+ member instead of initialized" } > +} > + > +constexpr bool indirect() { > + sso val; > + val.alloc = nullptr; > + perform_assignment(val.buf[5], 42); // { dg-message "in .constexpr. expansion" } > + return true; > +} > +constexpr bool err = indirect(); // { dg-message "in .constexpr. expansion" } > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > new file mode 100644 > index 00000000000..6d30bb2498f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > @@ -0,0 +1,45 @@ > +// { dg-do compile { target c++20 } } > + > +struct S > +{ > + union { > + char buf[8]; > + char* ptr; > + }; > + unsigned len; > + > + constexpr S(const char* s, unsigned n) > + { > + char* p; > + if (n > 7) > + p = ptr = new char[n+1]; > + else > + p = buf; > + for (len = 0; len < n; ++len) > + p[len] = s[len]; // { dg-error "accessing uninitialized member" } > + p[len] = '\0'; > + } > + > + constexpr ~S() > + { > + if (len > 7) > + delete[] ptr; > + } > +}; > + > +constexpr bool test1() > +{ > + S s("test", 4); // { dg-message "in .constexpr. expansion" } > + return true; > +} > + > +constexpr bool a = test1(); // { dg-message "in .constexpr. expansion" } > + > + > +constexpr bool test2() > +{ > + S s("hello world", 11); > + return true; > +} > + > +constexpr bool b = test2(); > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > new file mode 100644 > index 00000000000..429ab203ac2 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > @@ -0,0 +1,29 @@ > +// { dg-do compile { target c++20 } } > + > +// from [class.union.general] p5 > + > +union A { int x; int y[4]; }; > +struct B { A a; }; > +union C { B b; int k; }; > +constexpr int f() { > + C c; // does not start lifetime of any union member > + c.b.a.y[3] = 4; // OK, S(c.b.a.y[3]) contains c.b and c.b.a.y; > + // creates objects to hold union members c.b and c.b.a.y > + return c.b.a.y[3]; // OK, c.b.a.y refers to newly created object (see [basic.life]) > +} > +constexpr int a = f(); > + > +struct X { const int a; int b; }; > +union Y { X x; int k; };// { dg-message "does not implicitly begin its lifetime" } > +constexpr int g() { > + Y y = { { 1, 2 } }; // OK, y.x is active union member ([class.mem]) > + int n = y.x.a; > + y.k = 4; // OK, ends lifetime of y.x, y.k is active member of union > + > + y.x.b = n; // { dg-error "accessing .* member instead of initialized .* member" } > + // undefined behavior: y.x.b modified outside its lifetime, > + // S(y.x.b) is empty because X's default constructor is deleted, > + // so union member y.x's lifetime does not implicitly start > + return 0; > +} > +constexpr int b = g(); // { dg-message "in .constexpr. expansion" } > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > new file mode 100644 > index 00000000000..c94d7cc0fc9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > @@ -0,0 +1,71 @@ > +// { dg-do compile { target c++20 } } > + > +union U { int a; int b; int c[2]; }; > + > +constexpr int test1() { > + U u; > + u.a = 10; > + *&u.b = 20; // { dg-error "accessing" } > + return u.b; > +} > +constexpr int x1 = test1(); // { dg-message "in .constexpr. expansion" } > + > +constexpr int test2() { > + U u; > + u.a = 10; > + (0, u.b) = 20; // { dg-error "accessing" } > + return u.b; > +} > +constexpr int x2 = test2(); // { dg-message "in .constexpr. expansion" } > + > +constexpr int test3() { > + U u; > + u.a = 0; > + int* p = &u.b; > + p[u.a] = 10; // { dg-error "accessing" } > + return u.b; > +} > +constexpr int x3 = test3(); // { dg-message "in .constexpr. expansion" } > + > +constexpr int test4() { > + U u; > + u.a = 0; > + int* p = &u.b; > + u.a[p] = 10; // { dg-error "accessing" } > + return u.b; > +} > +constexpr int x4 = test4(); // { dg-message "in .constexpr. expansion" } > + > +struct S { U u[10]; }; > +constexpr int test5() { > + S s; > + s.u[4].a = 10; > + 6[s.u].b = 15; > + return 4[s.u].a + s.u[6].b; > +} > +static_assert(test5() == 25); > + > +constexpr int test6() { > + U u; > + u.a = 5; > + u.c[0] = 3; > + 1[u.c] = 8; > + return 1[u.c] + u.c[0]; > +} > +static_assert(test6() == 11); > + > +constexpr int test7() { > + U u {}; // value initialisation initialises first member > + int* p = &u.a; > + *p = 10; > + return *p; > +} > +constexpr int x7 = test7(); > + > +constexpr int test8() { > + U u; // default initialisation leaves no member initialised > + int* p = &u.a; > + *p = 10; // { dg-error "accessing" } > + return *p; > +} > +constexpr int x8 = test8(); // { dg-message "in .constexpr. expansion" } > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > new file mode 100644 > index 00000000000..7c37716892c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > @@ -0,0 +1,43 @@ > +// { dg-do compile { target c++20 } } > +// PR c++/102286 > + > +#include "construct_at.h" > + > +struct S { const int a; int b; }; > +union U { int k; S s; }; > + > +constexpr int test1() { > + U u {}; > + std::construct_at(&u.s, S{ 1, 2 }); > + return u.s.b; > +} > +static_assert(test1() == 2); > + > +constexpr int test2() { > + U u {}; > + int* p = &u.s.b; > + std::construct_at(p, 5); // { dg-message "in .constexpr. expansion" } > + return u.s.b; > +} > +constexpr int x2 = test2(); // { dg-message "in .constexpr. expansion" } > + > +struct S2 { int a; int b; }; > +union U2 { int k; S2 s; }; > +constexpr int test3() { > + U2 u; > + int* p = &u.s.b; > + std::construct_at(p, 8); // { dg-message "in .constexpr. expansion" } > + return u.s.b; > +}; > +constexpr int x3 = test3(); // { dg-message "in .constexpr. expansion" } > + > +constexpr int test4() { > + union { > + int data[1]; > + } u; > + std::construct_at(u.data, 0); // { dg-message "in .constexpr. expansion" } > + return 0; > +} > +constexpr int x4 = test4(); // { dg-message "in .constexpr. expansion" } > + > +// { dg-error "accessing uninitialized member" "" { target *-*-* } 0 } > -- > 2.41.0 >
On Sat, 23 Sept 2023, 01:39 Nathaniel Shead via Libstdc++, < libstdc++@gcc.gnu.org> wrote: > Now that bootstrap has finished, I have gotten regressions in the > following libstdc++ tests: > > Running libstdc++:libstdc++-dg/conformance.exp ... > FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++23 (test for excess > errors) > FAIL: 20_util/bitset/access/constexpr.cc -std=gnu++26 (test for excess > errors) > FAIL: 20_util/variant/constexpr.cc -std=gnu++20 (test for excess errors) > FAIL: 20_util/variant/constexpr.cc -std=gnu++26 (test for excess errors) > FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++20 (test > for excess errors) > FAIL: 21_strings/basic_string/cons/char/constexpr.cc -std=gnu++26 (test > for excess errors) > FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++20 (test > for excess errors) > FAIL: 21_strings/basic_string/cons/wchar_t/constexpr.cc -std=gnu++26 (test > for excess errors) > FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc > -std=gnu++20 (test for excess errors) > FAIL: 21_strings/basic_string/modifiers/swap/constexpr-wchar_t.cc > -std=gnu++26 (test for excess errors) > FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++20 > (test for excess errors) > FAIL: 21_strings/basic_string/modifiers/swap/constexpr.cc -std=gnu++26 > (test for excess errors) > FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++23 (test for excess > errors) > UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++23 compilation > failed to produce executable > FAIL: std/ranges/adaptors/join_with/1.cc -std=gnu++26 (test for excess > errors) > UNRESOLVED: std/ranges/adaptors/join_with/1.cc -std=gnu++26 compilation > failed to produce executable > > On investigation though it looks like the issue might be with libstdc++ > rather than the patch itself; running the failing tests using clang with > libstdc++ also produces similar errors, and my reading of the code > suggests that this is correct. > > What's the way forward here? Should I look at creating a patch to fix > the libstdc++ issues before resubmitting this patch for the C++ > frontend? Or should I submit a version of this patch without the > `std::construct_at` changes and wait till libstdc++ gets fixed for that? > I think we should fix libstdc++. There are probably only a few places that need a fix, which cause all those failures. I can help with those fixes. I'll look into it after the weekend. > On Sat, Sep 23, 2023 at 01:01:20AM +1000, Nathaniel Shead wrote: > > On Fri, Sep 22, 2023 at 02:21:15PM +0100, Jason Merrill wrote: > > > On 9/21/23 09:41, Nathaniel Shead wrote: > > > > I've updated the error messages, and also fixed another bug I found > > > > while retesting (value-initialised unions weren't considered to have > any > > > > active member yet). > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu. > > > > > > > > -- >8 -- > > > > > > > > This patch adds checks for attempting to change the active member of > a > > > > union by methods other than a member access expression. > > > > > > > > To be able to properly distinguish `*(&u.a) = ` from `u.a = `, this > > > > patch redoes the solution for c++/59950 to avoid extranneous *&; it > > > > seems that the only case that needed the workaround was when copying > > > > empty classes. > > > > > > > > This patch also ensures that constructors for a union field mark that > > > > field as the active member before entering the call itself; this > ensures > > > > that modifications of the field within the constructor's body don't > > > > cause false positives (as these will not appear to be member access > > > > expressions). This means that we no longer need to start the > lifetime of > > > > empty union members after the constructor body completes. > > > > > > > > As a drive-by fix, this patch also ensures that value-initialised > unions > > > > are considered to have activated their initial member for the > purpose of > > > > checking stores, which catches some additional mistakes pre-C++20. > > > > > > > > PR c++/101631 > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > * call.cc (build_over_call): Fold more indirect refs for trivial > > > > assignment op. > > > > * class.cc (type_has_non_deleted_trivial_default_ctor): Create. > > > > * constexpr.cc (cxx_eval_call_expression): Start lifetime of > > > > union member before entering constructor. > > > > (cxx_eval_store_expression): Activate member for > > > > value-initialised union. Check for accessing inactive union > > > > member indirectly. > > > > * cp-tree.h (type_has_non_deleted_trivial_default_ctor): > > > > Forward declare. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > * g++.dg/cpp1y/constexpr-89336-3.C: Fix union initialisation. > > > > * g++.dg/cpp1y/constexpr-union6.C: New test. > > > > * g++.dg/cpp2a/constexpr-union2.C: New test. > > > > * g++.dg/cpp2a/constexpr-union3.C: New test. > > > > * g++.dg/cpp2a/constexpr-union4.C: New test. > > > > * g++.dg/cpp2a/constexpr-union5.C: New test. > > > > > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > > > --- > > > > gcc/cp/call.cc | 11 +- > > > > gcc/cp/class.cc | 8 ++ > > > > gcc/cp/constexpr.cc | 129 > +++++++++++++----- > > > > gcc/cp/cp-tree.h | 1 + > > > > .../g++.dg/cpp1y/constexpr-89336-3.C | 2 +- > > > > gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C | 13 ++ > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C | 30 ++++ > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C | 45 ++++++ > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C | 29 ++++ > > > > gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C | 71 ++++++++++ > > > > 10 files changed, 296 insertions(+), 43 deletions(-) > > > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > > > > > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > > > > index e8dafbd8ba6..c1fb8807d3f 100644 > > > > --- a/gcc/cp/call.cc > > > > +++ b/gcc/cp/call.cc > > > > @@ -10330,10 +10330,7 @@ build_over_call (struct z_candidate *cand, > int flags, tsubst_flags_t complain) > > > > && DECL_OVERLOADED_OPERATOR_IS (fn, NOP_EXPR) > > > > && trivial_fn_p (fn)) > > > > { > > > > - /* Don't use cp_build_fold_indirect_ref, op= returns an > lvalue even if > > > > - the object argument isn't one. */ > > > > - tree to = cp_build_indirect_ref (input_location, argarray[0], > > > > - RO_ARROW, complain); > > > > + tree to = cp_build_fold_indirect_ref (argarray[0]); > > > > tree type = TREE_TYPE (to); > > > > tree as_base = CLASSTYPE_AS_BASE (type); > > > > tree arg = argarray[1]; > > > > @@ -10341,7 +10338,11 @@ build_over_call (struct z_candidate *cand, > int flags, tsubst_flags_t complain) > > > > if (is_really_empty_class (type, /*ignore_vptr*/true)) > > > > { > > > > - /* Avoid copying empty classes. */ > > > > + /* Avoid copying empty classes, but ensure op= returns an lvalue > even > > > > + if the object argument isn't one. This isn't needed in other > cases > > > > + since MODIFY_EXPR is always considered an lvalue. */ > > > > + to = cp_build_addr_expr (to, tf_none); > > > > + to = cp_build_indirect_ref (input_location, to, RO_ARROW, > complain); > > > > val = build2 (COMPOUND_EXPR, type, arg, to); > > > > suppress_warning (val, OPT_Wunused); > > > > } > > > > diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc > > > > index b71333af1f8..e31aeb8e68b 100644 > > > > --- a/gcc/cp/class.cc > > > > +++ b/gcc/cp/class.cc > > > > @@ -5688,6 +5688,14 @@ type_has_virtual_destructor (tree type) > > > > return (dtor && DECL_VIRTUAL_P (dtor)); > > > > } > > > > +/* True iff class TYPE has a non-deleted trivial default > > > > + constructor. */ > > > > + > > > > +bool type_has_non_deleted_trivial_default_ctor (tree type) > > > > +{ > > > > + return TYPE_HAS_TRIVIAL_DFLT (type) && locate_ctor (type); > > > > +} > > > > + > > > > /* Returns true iff T, a class, has a move-assignment or > > > > move-constructor. Does not lazily declare either. > > > > If USER_P is false, any move function will do. If it is true, > the > > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > > > > index a673a6022f1..5e50fc2c792 100644 > > > > --- a/gcc/cp/constexpr.cc > > > > +++ b/gcc/cp/constexpr.cc > > > > @@ -3146,40 +3146,34 @@ cxx_eval_call_expression (const > constexpr_ctx *ctx, tree t, > > > > cxx_set_object_constness (ctx, new_obj, > /*readonly_p=*/false, > > > > non_constant_p, overflow_p); > > > > + /* If this is a constructor, we are beginning the lifetime of the > > > > + object we are initializing. */ > > > > + if (new_obj > > > > + && DECL_CONSTRUCTOR_P (fun) > > > > + && TREE_CODE (new_obj) == COMPONENT_REF > > > > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (new_obj, 0))) == > UNION_TYPE) > > > > + { > > > > + tree activate = build2 (INIT_EXPR, TREE_TYPE (new_obj), > > > > + new_obj, > > > > + build_constructor (TREE_TYPE > (new_obj), > > > > + NULL)); > > > > + cxx_eval_constant_expression (ctx, activate, > > > > + lval, non_constant_p, > overflow_p); > > > > + ggc_free (activate); > > > > + } > > > > + > > > > tree jump_target = NULL_TREE; > > > > cxx_eval_constant_expression (&call_ctx, body, > > > > vc_discard, > non_constant_p, overflow_p, > > > > &jump_target); > > > > if (DECL_CONSTRUCTOR_P (fun)) > > > > - { > > > > - /* This can be null for a subobject constructor call, in > > > > - which case what we care about is the initialization > > > > - side-effects rather than the value. We could get at the > > > > - value by evaluating *this, but we don't bother; there's > > > > - no need to put such a call in the hash table. */ > > > > - result = lval ? ctx->object : ctx->ctor; > > > > - > > > > - /* If we've just evaluated a subobject constructor call for > an > > > > - empty union member, it might not have produced a side > effect > > > > - that actually activated the union member. So produce > such a > > > > - side effect now to ensure the union appears initialized. > */ > > > > - if (!result && new_obj > > > > - && TREE_CODE (new_obj) == COMPONENT_REF > > > > - && TREE_CODE (TREE_TYPE > > > > - (TREE_OPERAND (new_obj, 0))) == UNION_TYPE > > > > - && is_really_empty_class (TREE_TYPE (new_obj), > > > > - /*ignore_vptr*/false)) > > > > - { > > > > - tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj), > > > > - new_obj, > > > > - build_constructor (TREE_TYPE > (new_obj), > > > > - NULL)); > > > > - cxx_eval_constant_expression (ctx, activate, lval, > > > > - non_constant_p, > overflow_p); > > > > - ggc_free (activate); > > > > - } > > > > - } > > > > + /* This can be null for a subobject constructor call, in > > > > + which case what we care about is the initialization > > > > + side-effects rather than the value. We could get at the > > > > + value by evaluating *this, but we don't bother; there's > > > > + no need to put such a call in the hash table. */ > > > > + result = lval ? ctx->object : ctx->ctor; > > > > else if (VOID_TYPE_P (TREE_TYPE (res))) > > > > result = void_node; > > > > else > > > > @@ -6127,6 +6121,14 @@ cxx_eval_store_expression (const > constexpr_ctx *ctx, tree t, > > > > mutable_p) > > > > && const_object_being_modified == NULL_TREE) > > > > const_object_being_modified = probe; > > > > + > > > > + /* Track named member accesses for unions to validate > modifications > > > > + that change active member. */ > > > > + if (!evaluated && TREE_CODE (probe) == COMPONENT_REF) > > > > + vec_safe_push (refs, probe); > > > > + else > > > > + vec_safe_push (refs, NULL_TREE); > > > > + > > > > vec_safe_push (refs, elt); > > > > vec_safe_push (refs, TREE_TYPE (probe)); > > > > probe = ob; > > > > @@ -6135,6 +6137,7 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > > > case REALPART_EXPR: > > > > gcc_assert (probe == target); > > > > + vec_safe_push (refs, NULL_TREE); > > > > vec_safe_push (refs, probe); > > > > vec_safe_push (refs, TREE_TYPE (probe)); > > > > probe = TREE_OPERAND (probe, 0); > > > > @@ -6142,6 +6145,7 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > > > case IMAGPART_EXPR: > > > > gcc_assert (probe == target); > > > > + vec_safe_push (refs, NULL_TREE); > > > > vec_safe_push (refs, probe); > > > > vec_safe_push (refs, TREE_TYPE (probe)); > > > > probe = TREE_OPERAND (probe, 0); > > > > @@ -6230,6 +6234,7 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > > > enum tree_code code = TREE_CODE (type); > > > > tree reftype = refs->pop(); > > > > tree index = refs->pop(); > > > > + bool is_access_expr = refs->pop() != NULL_TREE; > > > > if (code == COMPLEX_TYPE) > > > > { > > > > @@ -6270,21 +6275,72 @@ cxx_eval_store_expression (const > constexpr_ctx *ctx, tree t, > > > > type = reftype; > > > > - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp) > > > > - && CONSTRUCTOR_ELT (*valp, 0)->index != index) > > > > + if (code == UNION_TYPE > > > > + && TREE_CODE (t) == MODIFY_EXPR > > > > + && (CONSTRUCTOR_NELTS (*valp) == 0 > > > > + || CONSTRUCTOR_ELT (*valp, 0)->index != index)) > > > > { > > > > - if (cxx_dialect < cxx20) > > > > + /* Probably a change of active member for a union. Ensure that > > > > + this is valid. */ > > > > + no_zero_init = true; > > > > + > > > > + tree init = *valp; > > > > + if (!CONSTRUCTOR_NO_CLEARING (*valp) && CONSTRUCTOR_NELTS > (*valp) == 0) > > > > + /* This is a value-initialized union, process the > initialization > > > > + to ensure that the active member is properly set. */ > > > > + init = build_value_init (TREE_TYPE (*valp), > tf_warning_or_error); > > > > + > > > > + bool has_active_member = CONSTRUCTOR_NELTS (init) != 0; > > > > + tree inner = strip_array_types (reftype); > > > > + > > > > + if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == > index) > > > > + /* After handling value-initialization, this wasn't actually a > > > > + change of active member like we initially thought. */ > > > > + no_zero_init = false; > > > > + else if (has_active_member && cxx_dialect < cxx20) > > > > { > > > > if (!ctx->quiet) > > > > error_at (cp_expr_loc_or_input_loc (t), > > > > "change of the active member of a union " > > > > - "from %qD to %qD", > > > > - CONSTRUCTOR_ELT (*valp, 0)->index, > > > > + "from %qD to %qD is not a constant expression " > > > > + "before C++20", > > > > + CONSTRUCTOR_ELT (init, 0)->index, > > > > index); > > > > *non_constant_p = true; > > > > } > > > > - else if (TREE_CODE (t) == MODIFY_EXPR > > > > - && CONSTRUCTOR_NO_CLEARING (*valp)) > > > > + else if (!is_access_expr > > > > + || (CLASS_TYPE_P (inner) > > > > + && !type_has_non_deleted_trivial_default_ctor > (inner))) > > > > + { > > > > + /* Diagnose changing active union member after initialization > > > > + without a valid member access expression, as described in > > > > + [class.union.general] p5. */ > > > > + if (!ctx->quiet) > > > > + { > > > > + if (has_active_member) > > > > + error_at (cp_expr_loc_or_input_loc (t), > > > > + "accessing %qD member instead of initialized > " > > > > + "%qD member in constant expression", > > > > + index, CONSTRUCTOR_ELT (init, 0)->index); > > > > + else > > > > + error_at (cp_expr_loc_or_input_loc (t), > > > > + "accessing uninitialized member %qD", > > > > + index); > > > > + if (is_access_expr) > > > > + inform (DECL_SOURCE_LOCATION (index), > > > > + "%qD does not implicitly begin its lifetime " > > > > + "because %qT does not have a non-deleted " > > > > + "trivial default constructor", > > > > + index, inner); > > > > > > It now occurs to me that this should suggest using placement new > instead. > > > And that we should test using placement new to begin the lifetime of > such > > > members. > > > > > > Jason > > > > > > > Good point. On testing placement new I found I wasn't checking for > > indirect change of active member during initialisation either, but this > > was pretty trivial to add. How does this look? > > > > Bootstrap + regtest ongoing, but dg.exp completed with no errors. I'm > > away from my computer next week so I thought I'd send this through now > > anyway. > > > > -- >8 -- > > > > This patch adds checks for attempting to change the active member of a > > union by methods other than a member access expression. > > > > To be able to properly distinguish `*(&u.a) = ` from `u.a = `, this > > patch redoes the solution for c++/59950 to avoid extranneous *&; it > > seems that the only case that needed the workaround was when copying > > empty classes. > > > > This patch also ensures that constructors for a union field mark that > > field as the active member before entering the call itself; this ensures > > that modifications of the field within the constructor's body don't > > cause false positives (as these will not appear to be member access > > expressions). This means that we no longer need to start the lifetime of > > empty union members after the constructor body completes. > > > > As a drive-by fix, this patch also ensures that value-initialised unions > > are considered to have activated their initial member for the purpose of > > checking stores, which catches some additional mistakes pre-C++20. > > > > PR c++/101631 > > PR c++/102286 > > > > gcc/cp/ChangeLog: > > > > * call.cc (build_over_call): Fold more indirect refs for trivial > > assignment op. > > * class.cc (type_has_non_deleted_trivial_default_ctor): Create. > > * constexpr.cc (cxx_eval_call_expression): Start lifetime of > > union member before entering constructor. > > (cxx_eval_store_expression): Activate member for > > value-initialised union. Check for accessing inactive union > > member indirectly. > > * cp-tree.h (type_has_non_deleted_trivial_default_ctor): > > Forward declare. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp1y/constexpr-89336-3.C: Fix union initialisation. > > * g++.dg/cpp1y/constexpr-union6.C: New test. > > * g++.dg/cpp2a/constexpr-union2.C: New test. > > * g++.dg/cpp2a/constexpr-union3.C: New test. > > * g++.dg/cpp2a/constexpr-union4.C: New test. > > * g++.dg/cpp2a/constexpr-union5.C: New test. > > * g++.dg/cpp2a/constexpr-union6.C: New test. > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > --- > > gcc/cp/call.cc | 11 +- > > gcc/cp/class.cc | 8 ++ > > gcc/cp/constexpr.cc | 135 +++++++++++++----- > > gcc/cp/cp-tree.h | 1 + > > .../g++.dg/cpp1y/constexpr-89336-3.C | 2 +- > > gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C | 13 ++ > > gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C | 30 ++++ > > gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C | 45 ++++++ > > gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C | 29 ++++ > > gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C | 71 +++++++++ > > gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C | 43 ++++++ > > 11 files changed, 345 insertions(+), 43 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > > index e8dafbd8ba6..c1fb8807d3f 100644 > > --- a/gcc/cp/call.cc > > +++ b/gcc/cp/call.cc > > @@ -10330,10 +10330,7 @@ build_over_call (struct z_candidate *cand, int > flags, tsubst_flags_t complain) > > && DECL_OVERLOADED_OPERATOR_IS (fn, NOP_EXPR) > > && trivial_fn_p (fn)) > > { > > - /* Don't use cp_build_fold_indirect_ref, op= returns an lvalue > even if > > - the object argument isn't one. */ > > - tree to = cp_build_indirect_ref (input_location, argarray[0], > > - RO_ARROW, complain); > > + tree to = cp_build_fold_indirect_ref (argarray[0]); > > tree type = TREE_TYPE (to); > > tree as_base = CLASSTYPE_AS_BASE (type); > > tree arg = argarray[1]; > > @@ -10341,7 +10338,11 @@ build_over_call (struct z_candidate *cand, int > flags, tsubst_flags_t complain) > > > > if (is_really_empty_class (type, /*ignore_vptr*/true)) > > { > > - /* Avoid copying empty classes. */ > > + /* Avoid copying empty classes, but ensure op= returns an lvalue > even > > + if the object argument isn't one. This isn't needed in other > cases > > + since MODIFY_EXPR is always considered an lvalue. */ > > + to = cp_build_addr_expr (to, tf_none); > > + to = cp_build_indirect_ref (input_location, to, RO_ARROW, > complain); > > val = build2 (COMPOUND_EXPR, type, arg, to); > > suppress_warning (val, OPT_Wunused); > > } > > diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc > > index b71333af1f8..e31aeb8e68b 100644 > > --- a/gcc/cp/class.cc > > +++ b/gcc/cp/class.cc > > @@ -5688,6 +5688,14 @@ type_has_virtual_destructor (tree type) > > return (dtor && DECL_VIRTUAL_P (dtor)); > > } > > > > +/* True iff class TYPE has a non-deleted trivial default > > + constructor. */ > > + > > +bool type_has_non_deleted_trivial_default_ctor (tree type) > > +{ > > + return TYPE_HAS_TRIVIAL_DFLT (type) && locate_ctor (type); > > +} > > + > > /* Returns true iff T, a class, has a move-assignment or > > move-constructor. Does not lazily declare either. > > If USER_P is false, any move function will do. If it is true, the > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > > index a673a6022f1..8e6f60d03d2 100644 > > --- a/gcc/cp/constexpr.cc > > +++ b/gcc/cp/constexpr.cc > > @@ -3146,40 +3146,34 @@ cxx_eval_call_expression (const constexpr_ctx > *ctx, tree t, > > cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/false, > > non_constant_p, overflow_p); > > > > + /* If this is a constructor, we are beginning the lifetime of the > > + object we are initializing. */ > > + if (new_obj > > + && DECL_CONSTRUCTOR_P (fun) > > + && TREE_CODE (new_obj) == COMPONENT_REF > > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (new_obj, 0))) == > UNION_TYPE) > > + { > > + tree activate = build2 (INIT_EXPR, TREE_TYPE (new_obj), > > + new_obj, > > + build_constructor (TREE_TYPE > (new_obj), > > + NULL)); > > + cxx_eval_constant_expression (ctx, activate, > > + lval, non_constant_p, > overflow_p); > > + ggc_free (activate); > > + } > > + > > tree jump_target = NULL_TREE; > > cxx_eval_constant_expression (&call_ctx, body, > > vc_discard, non_constant_p, > overflow_p, > > &jump_target); > > > > if (DECL_CONSTRUCTOR_P (fun)) > > - { > > - /* This can be null for a subobject constructor call, in > > - which case what we care about is the initialization > > - side-effects rather than the value. We could get at the > > - value by evaluating *this, but we don't bother; there's > > - no need to put such a call in the hash table. */ > > - result = lval ? ctx->object : ctx->ctor; > > - > > - /* If we've just evaluated a subobject constructor call for > an > > - empty union member, it might not have produced a side > effect > > - that actually activated the union member. So produce > such a > > - side effect now to ensure the union appears initialized. > */ > > - if (!result && new_obj > > - && TREE_CODE (new_obj) == COMPONENT_REF > > - && TREE_CODE (TREE_TYPE > > - (TREE_OPERAND (new_obj, 0))) == UNION_TYPE > > - && is_really_empty_class (TREE_TYPE (new_obj), > > - /*ignore_vptr*/false)) > > - { > > - tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj), > > - new_obj, > > - build_constructor (TREE_TYPE > (new_obj), > > - NULL)); > > - cxx_eval_constant_expression (ctx, activate, lval, > > - non_constant_p, > overflow_p); > > - ggc_free (activate); > > - } > > - } > > + /* This can be null for a subobject constructor call, in > > + which case what we care about is the initialization > > + side-effects rather than the value. We could get at the > > + value by evaluating *this, but we don't bother; there's > > + no need to put such a call in the hash table. */ > > + result = lval ? ctx->object : ctx->ctor; > > else if (VOID_TYPE_P (TREE_TYPE (res))) > > result = void_node; > > else > > @@ -6127,6 +6121,14 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > mutable_p) > > && const_object_being_modified == NULL_TREE) > > const_object_being_modified = probe; > > + > > + /* Track named member accesses for unions to validate > modifications > > + that change active member. */ > > + if (!evaluated && TREE_CODE (probe) == COMPONENT_REF) > > + vec_safe_push (refs, probe); > > + else > > + vec_safe_push (refs, NULL_TREE); > > + > > vec_safe_push (refs, elt); > > vec_safe_push (refs, TREE_TYPE (probe)); > > probe = ob; > > @@ -6135,6 +6137,7 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > > > case REALPART_EXPR: > > gcc_assert (probe == target); > > + vec_safe_push (refs, NULL_TREE); > > vec_safe_push (refs, probe); > > vec_safe_push (refs, TREE_TYPE (probe)); > > probe = TREE_OPERAND (probe, 0); > > @@ -6142,6 +6145,7 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > > > case IMAGPART_EXPR: > > gcc_assert (probe == target); > > + vec_safe_push (refs, NULL_TREE); > > vec_safe_push (refs, probe); > > vec_safe_push (refs, TREE_TYPE (probe)); > > probe = TREE_OPERAND (probe, 0); > > @@ -6230,6 +6234,7 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > enum tree_code code = TREE_CODE (type); > > tree reftype = refs->pop(); > > tree index = refs->pop(); > > + bool is_access_expr = refs->pop() != NULL_TREE; > > > > if (code == COMPLEX_TYPE) > > { > > @@ -6270,21 +6275,78 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > > > type = reftype; > > > > - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp) > > - && CONSTRUCTOR_ELT (*valp, 0)->index != index) > > + /* Check for implicit change of active member for a union. */ > > + if (code == UNION_TYPE > > + && (TREE_CODE (t) == MODIFY_EXPR > > + /* Also check if initializations have implicit change of > active > > + member earlier up the access chain. */ > > + || !refs->is_empty()) > > + && (CONSTRUCTOR_NELTS (*valp) == 0 > > + || CONSTRUCTOR_ELT (*valp, 0)->index != index)) > > { > > - if (cxx_dialect < cxx20) > > + no_zero_init = true; > > + > > + tree init = *valp; > > + if (TREE_CODE (t) == MODIFY_EXPR > > + && !CONSTRUCTOR_NO_CLEARING (*valp) > > + && CONSTRUCTOR_NELTS (*valp) == 0) > > + /* Modifying a value-initialized union, process the > initialization > > + to ensure that the active member is properly set. */ > > + init = build_value_init (TREE_TYPE (*valp), > tf_warning_or_error); > > + > > + bool has_active_member = CONSTRUCTOR_NELTS (init) != 0; > > + tree inner = strip_array_types (reftype); > > + > > + if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == > index) > > + /* After handling value-initialization, this wasn't actually a > > + change of active member like we initially thought. */ > > + no_zero_init = false; > > + else if (has_active_member && cxx_dialect < cxx20) > > { > > if (!ctx->quiet) > > error_at (cp_expr_loc_or_input_loc (t), > > "change of the active member of a union " > > - "from %qD to %qD", > > - CONSTRUCTOR_ELT (*valp, 0)->index, > > + "from %qD to %qD is not a constant expression " > > + "before C++20", > > + CONSTRUCTOR_ELT (init, 0)->index, > > index); > > *non_constant_p = true; > > } > > - else if (TREE_CODE (t) == MODIFY_EXPR > > - && CONSTRUCTOR_NO_CLEARING (*valp)) > > + else if (!is_access_expr > > + || (TREE_CODE (t) == MODIFY_EXPR > > + && CLASS_TYPE_P (inner) > > + && !type_has_non_deleted_trivial_default_ctor > (inner))) > > + { > > + /* Diagnose changing active union member after initialization > > + without a valid member access expression, as described in > > + [class.union.general] p5. */ > > + if (!ctx->quiet) > > + { > > + if (has_active_member) > > + error_at (cp_expr_loc_or_input_loc (t), > > + "accessing %qD member instead of initialized > " > > + "%qD member in constant expression", > > + index, CONSTRUCTOR_ELT (init, 0)->index); > > + else > > + error_at (cp_expr_loc_or_input_loc (t), > > + "accessing uninitialized member %qD", > > + index); > > + if (is_access_expr) > > + inform (DECL_SOURCE_LOCATION (index), > > + "%qD does not implicitly begin its lifetime " > > + "because %qT does not have a non-deleted " > > + "trivial default constructor, use " > > + "%<std::construct_at%> instead", > > + index, inner); > > + else > > + inform (DECL_SOURCE_LOCATION (index), > > + "initializing %qD requires a member access " > > + "expression as the left operand of the > assignment", > > + index); > > + } > > + *non_constant_p = true; > > + } > > + else if (has_active_member && CONSTRUCTOR_NO_CLEARING (init)) > > { > > /* Diagnose changing the active union member while the union > > is in the process of being initialized. */ > > @@ -6292,11 +6354,10 @@ cxx_eval_store_expression (const constexpr_ctx > *ctx, tree t, > > error_at (cp_expr_loc_or_input_loc (t), > > "change of the active member of a union " > > "from %qD to %qD during initialization", > > - CONSTRUCTOR_ELT (*valp, 0)->index, > > + CONSTRUCTOR_ELT (init, 0)->index, > > index); > > *non_constant_p = true; > > } > > - no_zero_init = true; > > } > > > > ctors.safe_push (valp); > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index fd6bf9fd4a0..40aa9f0be2b 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -6815,6 +6815,7 @@ extern bool > trivial_default_constructor_is_constexpr (tree); > > extern bool type_has_constexpr_default_constructor (tree); > > extern bool type_has_constexpr_destructor (tree); > > extern bool type_has_virtual_destructor (tree); > > +extern bool type_has_non_deleted_trivial_default_ctor (tree); > > extern bool classtype_has_move_assign_or_move_ctor_p (tree, bool > user_declared); > > extern bool classtype_has_non_deleted_move_ctor (tree); > > extern tree classtype_has_depr_implicit_copy (tree); > > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > > index 9d370ddefcf..6a60966f6f8 100644 > > --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C > > @@ -18,7 +18,7 @@ constexpr int > > bar () > > { > > union U { int a[5]; long b; }; > > - union V { union U u; short v; }; > > + union V { short v; union U u; }; > > V w {}; > > w.v = 5; > > w.u.a[3] = w.u.a[1] = w.v; // { dg-error "change of the > active member of a union from" "" { target c++17_down } } > > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > > new file mode 100644 > > index 00000000000..ff7ebf19f89 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C > > @@ -0,0 +1,13 @@ > > +// { dg-do compile { target c++14 } } > > + > > +union U { > > + int a; > > + float b; > > +}; > > + > > +constexpr bool foo() { > > + U u {}; > > + u.b = 1.0f; // { dg-error "change of the active member" "" { target > c++17_down } } > > + return u.b == 1.0f; > > +} > > +constexpr bool x = foo(); > > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > > new file mode 100644 > > index 00000000000..1712395d7e7 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C > > @@ -0,0 +1,30 @@ > > +// PR c++/101631 > > +// { dg-do compile { target c++20 } } > > + > > +struct sso { > > + union { > > + int buf[10]; > > + int* alloc; > > + }; > > +}; > > + > > +constexpr bool direct() { > > + sso val; > > + val.alloc = nullptr; > > + val.buf[5] = 42; > > + return true; > > +} > > +constexpr bool ok = direct(); > > + > > + > > +constexpr void perform_assignment(int& left, int right) noexcept { > > + left = right; // { dg-error "accessing .+ member instead of > initialized" } > > +} > > + > > +constexpr bool indirect() { > > + sso val; > > + val.alloc = nullptr; > > + perform_assignment(val.buf[5], 42); // { dg-message "in .constexpr. > expansion" } > > + return true; > > +} > > +constexpr bool err = indirect(); // { dg-message "in .constexpr. > expansion" } > > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > > new file mode 100644 > > index 00000000000..6d30bb2498f > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C > > @@ -0,0 +1,45 @@ > > +// { dg-do compile { target c++20 } } > > + > > +struct S > > +{ > > + union { > > + char buf[8]; > > + char* ptr; > > + }; > > + unsigned len; > > + > > + constexpr S(const char* s, unsigned n) > > + { > > + char* p; > > + if (n > 7) > > + p = ptr = new char[n+1]; > > + else > > + p = buf; > > + for (len = 0; len < n; ++len) > > + p[len] = s[len]; // { dg-error "accessing uninitialized member" } > > + p[len] = '\0'; > > + } > > + > > + constexpr ~S() > > + { > > + if (len > 7) > > + delete[] ptr; > > + } > > +}; > > + > > +constexpr bool test1() > > +{ > > + S s("test", 4); // { dg-message "in .constexpr. expansion" } > > + return true; > > +} > > + > > +constexpr bool a = test1(); // { dg-message "in .constexpr. expansion" > } > > + > > + > > +constexpr bool test2() > > +{ > > + S s("hello world", 11); > > + return true; > > +} > > + > > +constexpr bool b = test2(); > > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > > new file mode 100644 > > index 00000000000..429ab203ac2 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C > > @@ -0,0 +1,29 @@ > > +// { dg-do compile { target c++20 } } > > + > > +// from [class.union.general] p5 > > + > > +union A { int x; int y[4]; }; > > +struct B { A a; }; > > +union C { B b; int k; }; > > +constexpr int f() { > > + C c; // does not start lifetime of any union member > > + c.b.a.y[3] = 4; // OK, S(c.b.a.y[3]) contains c.b and c.b.a.y; > > + // creates objects to hold union members c.b > and c.b.a.y > > + return c.b.a.y[3]; // OK, c.b.a.y refers to newly created object > (see [basic.life]) > > +} > > +constexpr int a = f(); > > + > > +struct X { const int a; int b; }; > > +union Y { X x; int k; };// { dg-message "does not implicitly begin its > lifetime" } > > +constexpr int g() { > > + Y y = { { 1, 2 } }; // OK, y.x is active union member ([class.mem]) > > + int n = y.x.a; > > + y.k = 4; // OK, ends lifetime of y.x, y.k is active > member of union > > + > > + y.x.b = n; // { dg-error "accessing .* member instead of > initialized .* member" } > > + // undefined behavior: y.x.b modified outside > its lifetime, > > + // S(y.x.b) is empty because X's default > constructor is deleted, > > + // so union member y.x's lifetime does not > implicitly start > > + return 0; > > +} > > +constexpr int b = g(); // { dg-message "in .constexpr. expansion" } > > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > > new file mode 100644 > > index 00000000000..c94d7cc0fc9 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C > > @@ -0,0 +1,71 @@ > > +// { dg-do compile { target c++20 } } > > + > > +union U { int a; int b; int c[2]; }; > > + > > +constexpr int test1() { > > + U u; > > + u.a = 10; > > + *&u.b = 20; // { dg-error "accessing" } > > + return u.b; > > +} > > +constexpr int x1 = test1(); // { dg-message "in .constexpr. expansion" > } > > + > > +constexpr int test2() { > > + U u; > > + u.a = 10; > > + (0, u.b) = 20; // { dg-error "accessing" } > > + return u.b; > > +} > > +constexpr int x2 = test2(); // { dg-message "in .constexpr. expansion" > } > > + > > +constexpr int test3() { > > + U u; > > + u.a = 0; > > + int* p = &u.b; > > + p[u.a] = 10; // { dg-error "accessing" } > > + return u.b; > > +} > > +constexpr int x3 = test3(); // { dg-message "in .constexpr. expansion" > } > > + > > +constexpr int test4() { > > + U u; > > + u.a = 0; > > + int* p = &u.b; > > + u.a[p] = 10; // { dg-error "accessing" } > > + return u.b; > > +} > > +constexpr int x4 = test4(); // { dg-message "in .constexpr. expansion" > } > > + > > +struct S { U u[10]; }; > > +constexpr int test5() { > > + S s; > > + s.u[4].a = 10; > > + 6[s.u].b = 15; > > + return 4[s.u].a + s.u[6].b; > > +} > > +static_assert(test5() == 25); > > + > > +constexpr int test6() { > > + U u; > > + u.a = 5; > > + u.c[0] = 3; > > + 1[u.c] = 8; > > + return 1[u.c] + u.c[0]; > > +} > > +static_assert(test6() == 11); > > + > > +constexpr int test7() { > > + U u {}; // value initialisation initialises first member > > + int* p = &u.a; > > + *p = 10; > > + return *p; > > +} > > +constexpr int x7 = test7(); > > + > > +constexpr int test8() { > > + U u; // default initialisation leaves no member initialised > > + int* p = &u.a; > > + *p = 10; // { dg-error "accessing" } > > + return *p; > > +} > > +constexpr int x8 = test8(); // { dg-message "in .constexpr. expansion" > } > > diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > > new file mode 100644 > > index 00000000000..7c37716892c > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C > > @@ -0,0 +1,43 @@ > > +// { dg-do compile { target c++20 } } > > +// PR c++/102286 > > + > > +#include "construct_at.h" > > + > > +struct S { const int a; int b; }; > > +union U { int k; S s; }; > > + > > +constexpr int test1() { > > + U u {}; > > + std::construct_at(&u.s, S{ 1, 2 }); > > + return u.s.b; > > +} > > +static_assert(test1() == 2); > > + > > +constexpr int test2() { > > + U u {}; > > + int* p = &u.s.b; > > + std::construct_at(p, 5); // { dg-message "in .constexpr. expansion" } > > + return u.s.b; > > +} > > +constexpr int x2 = test2(); // { dg-message "in .constexpr. expansion" > } > > + > > +struct S2 { int a; int b; }; > > +union U2 { int k; S2 s; }; > > +constexpr int test3() { > > + U2 u; > > + int* p = &u.s.b; > > + std::construct_at(p, 8); // { dg-message "in .constexpr. expansion" } > > + return u.s.b; > > +}; > > +constexpr int x3 = test3(); // { dg-message "in .constexpr. expansion" > } > > + > > +constexpr int test4() { > > + union { > > + int data[1]; > > + } u; > > + std::construct_at(u.data, 0); // { dg-message "in .constexpr. > expansion" } > > + return 0; > > +} > > +constexpr int x4 = test4(); // { dg-message "in .constexpr. expansion" > } > > + > > +// { dg-error "accessing uninitialized member" "" { target *-*-* } 0 } > > -- > > 2.41.0 > > >
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index e8dafbd8ba6..c1fb8807d3f 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -10330,10 +10330,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) && DECL_OVERLOADED_OPERATOR_IS (fn, NOP_EXPR) && trivial_fn_p (fn)) { - /* Don't use cp_build_fold_indirect_ref, op= returns an lvalue even if - the object argument isn't one. */ - tree to = cp_build_indirect_ref (input_location, argarray[0], - RO_ARROW, complain); + tree to = cp_build_fold_indirect_ref (argarray[0]); tree type = TREE_TYPE (to); tree as_base = CLASSTYPE_AS_BASE (type); tree arg = argarray[1]; @@ -10341,7 +10338,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) if (is_really_empty_class (type, /*ignore_vptr*/true)) { - /* Avoid copying empty classes. */ + /* Avoid copying empty classes, but ensure op= returns an lvalue even + if the object argument isn't one. This isn't needed in other cases + since MODIFY_EXPR is always considered an lvalue. */ + to = cp_build_addr_expr (to, tf_none); + to = cp_build_indirect_ref (input_location, to, RO_ARROW, complain); val = build2 (COMPOUND_EXPR, type, arg, to); suppress_warning (val, OPT_Wunused); } diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index b71333af1f8..e31aeb8e68b 100644 --- a/gcc/cp/class.cc +++ b/gcc/cp/class.cc @@ -5688,6 +5688,14 @@ type_has_virtual_destructor (tree type) return (dtor && DECL_VIRTUAL_P (dtor)); } +/* True iff class TYPE has a non-deleted trivial default + constructor. */ + +bool type_has_non_deleted_trivial_default_ctor (tree type) +{ + return TYPE_HAS_TRIVIAL_DFLT (type) && locate_ctor (type); +} + /* Returns true iff T, a class, has a move-assignment or move-constructor. Does not lazily declare either. If USER_P is false, any move function will do. If it is true, the diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index a673a6022f1..8e6f60d03d2 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -3146,40 +3146,34 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, cxx_set_object_constness (ctx, new_obj, /*readonly_p=*/false, non_constant_p, overflow_p); + /* If this is a constructor, we are beginning the lifetime of the + object we are initializing. */ + if (new_obj + && DECL_CONSTRUCTOR_P (fun) + && TREE_CODE (new_obj) == COMPONENT_REF + && TREE_CODE (TREE_TYPE (TREE_OPERAND (new_obj, 0))) == UNION_TYPE) + { + tree activate = build2 (INIT_EXPR, TREE_TYPE (new_obj), + new_obj, + build_constructor (TREE_TYPE (new_obj), + NULL)); + cxx_eval_constant_expression (ctx, activate, + lval, non_constant_p, overflow_p); + ggc_free (activate); + } + tree jump_target = NULL_TREE; cxx_eval_constant_expression (&call_ctx, body, vc_discard, non_constant_p, overflow_p, &jump_target); if (DECL_CONSTRUCTOR_P (fun)) - { - /* This can be null for a subobject constructor call, in - which case what we care about is the initialization - side-effects rather than the value. We could get at the - value by evaluating *this, but we don't bother; there's - no need to put such a call in the hash table. */ - result = lval ? ctx->object : ctx->ctor; - - /* If we've just evaluated a subobject constructor call for an - empty union member, it might not have produced a side effect - that actually activated the union member. So produce such a - side effect now to ensure the union appears initialized. */ - if (!result && new_obj - && TREE_CODE (new_obj) == COMPONENT_REF - && TREE_CODE (TREE_TYPE - (TREE_OPERAND (new_obj, 0))) == UNION_TYPE - && is_really_empty_class (TREE_TYPE (new_obj), - /*ignore_vptr*/false)) - { - tree activate = build2 (MODIFY_EXPR, TREE_TYPE (new_obj), - new_obj, - build_constructor (TREE_TYPE (new_obj), - NULL)); - cxx_eval_constant_expression (ctx, activate, lval, - non_constant_p, overflow_p); - ggc_free (activate); - } - } + /* This can be null for a subobject constructor call, in + which case what we care about is the initialization + side-effects rather than the value. We could get at the + value by evaluating *this, but we don't bother; there's + no need to put such a call in the hash table. */ + result = lval ? ctx->object : ctx->ctor; else if (VOID_TYPE_P (TREE_TYPE (res))) result = void_node; else @@ -6127,6 +6121,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, mutable_p) && const_object_being_modified == NULL_TREE) const_object_being_modified = probe; + + /* Track named member accesses for unions to validate modifications + that change active member. */ + if (!evaluated && TREE_CODE (probe) == COMPONENT_REF) + vec_safe_push (refs, probe); + else + vec_safe_push (refs, NULL_TREE); + vec_safe_push (refs, elt); vec_safe_push (refs, TREE_TYPE (probe)); probe = ob; @@ -6135,6 +6137,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, case REALPART_EXPR: gcc_assert (probe == target); + vec_safe_push (refs, NULL_TREE); vec_safe_push (refs, probe); vec_safe_push (refs, TREE_TYPE (probe)); probe = TREE_OPERAND (probe, 0); @@ -6142,6 +6145,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, case IMAGPART_EXPR: gcc_assert (probe == target); + vec_safe_push (refs, NULL_TREE); vec_safe_push (refs, probe); vec_safe_push (refs, TREE_TYPE (probe)); probe = TREE_OPERAND (probe, 0); @@ -6230,6 +6234,7 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, enum tree_code code = TREE_CODE (type); tree reftype = refs->pop(); tree index = refs->pop(); + bool is_access_expr = refs->pop() != NULL_TREE; if (code == COMPLEX_TYPE) { @@ -6270,21 +6275,78 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, type = reftype; - if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp) - && CONSTRUCTOR_ELT (*valp, 0)->index != index) + /* Check for implicit change of active member for a union. */ + if (code == UNION_TYPE + && (TREE_CODE (t) == MODIFY_EXPR + /* Also check if initializations have implicit change of active + member earlier up the access chain. */ + || !refs->is_empty()) + && (CONSTRUCTOR_NELTS (*valp) == 0 + || CONSTRUCTOR_ELT (*valp, 0)->index != index)) { - if (cxx_dialect < cxx20) + no_zero_init = true; + + tree init = *valp; + if (TREE_CODE (t) == MODIFY_EXPR + && !CONSTRUCTOR_NO_CLEARING (*valp) + && CONSTRUCTOR_NELTS (*valp) == 0) + /* Modifying a value-initialized union, process the initialization + to ensure that the active member is properly set. */ + init = build_value_init (TREE_TYPE (*valp), tf_warning_or_error); + + bool has_active_member = CONSTRUCTOR_NELTS (init) != 0; + tree inner = strip_array_types (reftype); + + if (has_active_member && CONSTRUCTOR_ELT (init, 0)->index == index) + /* After handling value-initialization, this wasn't actually a + change of active member like we initially thought. */ + no_zero_init = false; + else if (has_active_member && cxx_dialect < cxx20) { if (!ctx->quiet) error_at (cp_expr_loc_or_input_loc (t), "change of the active member of a union " - "from %qD to %qD", - CONSTRUCTOR_ELT (*valp, 0)->index, + "from %qD to %qD is not a constant expression " + "before C++20", + CONSTRUCTOR_ELT (init, 0)->index, index); *non_constant_p = true; } - else if (TREE_CODE (t) == MODIFY_EXPR - && CONSTRUCTOR_NO_CLEARING (*valp)) + else if (!is_access_expr + || (TREE_CODE (t) == MODIFY_EXPR + && CLASS_TYPE_P (inner) + && !type_has_non_deleted_trivial_default_ctor (inner))) + { + /* Diagnose changing active union member after initialization + without a valid member access expression, as described in + [class.union.general] p5. */ + if (!ctx->quiet) + { + if (has_active_member) + error_at (cp_expr_loc_or_input_loc (t), + "accessing %qD member instead of initialized " + "%qD member in constant expression", + index, CONSTRUCTOR_ELT (init, 0)->index); + else + error_at (cp_expr_loc_or_input_loc (t), + "accessing uninitialized member %qD", + index); + if (is_access_expr) + inform (DECL_SOURCE_LOCATION (index), + "%qD does not implicitly begin its lifetime " + "because %qT does not have a non-deleted " + "trivial default constructor, use " + "%<std::construct_at%> instead", + index, inner); + else + inform (DECL_SOURCE_LOCATION (index), + "initializing %qD requires a member access " + "expression as the left operand of the assignment", + index); + } + *non_constant_p = true; + } + else if (has_active_member && CONSTRUCTOR_NO_CLEARING (init)) { /* Diagnose changing the active union member while the union is in the process of being initialized. */ @@ -6292,11 +6354,10 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, error_at (cp_expr_loc_or_input_loc (t), "change of the active member of a union " "from %qD to %qD during initialization", - CONSTRUCTOR_ELT (*valp, 0)->index, + CONSTRUCTOR_ELT (init, 0)->index, index); *non_constant_p = true; } - no_zero_init = true; } ctors.safe_push (valp); diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index fd6bf9fd4a0..40aa9f0be2b 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6815,6 +6815,7 @@ extern bool trivial_default_constructor_is_constexpr (tree); extern bool type_has_constexpr_default_constructor (tree); extern bool type_has_constexpr_destructor (tree); extern bool type_has_virtual_destructor (tree); +extern bool type_has_non_deleted_trivial_default_ctor (tree); extern bool classtype_has_move_assign_or_move_ctor_p (tree, bool user_declared); extern bool classtype_has_non_deleted_move_ctor (tree); extern tree classtype_has_depr_implicit_copy (tree); diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C index 9d370ddefcf..6a60966f6f8 100644 --- a/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-89336-3.C @@ -18,7 +18,7 @@ constexpr int bar () { union U { int a[5]; long b; }; - union V { union U u; short v; }; + union V { short v; union U u; }; V w {}; w.v = 5; w.u.a[3] = w.u.a[1] = w.v; // { dg-error "change of the active member of a union from" "" { target c++17_down } } diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C new file mode 100644 index 00000000000..ff7ebf19f89 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-union6.C @@ -0,0 +1,13 @@ +// { dg-do compile { target c++14 } } + +union U { + int a; + float b; +}; + +constexpr bool foo() { + U u {}; + u.b = 1.0f; // { dg-error "change of the active member" "" { target c++17_down } } + return u.b == 1.0f; +} +constexpr bool x = foo(); diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C new file mode 100644 index 00000000000..1712395d7e7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union2.C @@ -0,0 +1,30 @@ +// PR c++/101631 +// { dg-do compile { target c++20 } } + +struct sso { + union { + int buf[10]; + int* alloc; + }; +}; + +constexpr bool direct() { + sso val; + val.alloc = nullptr; + val.buf[5] = 42; + return true; +} +constexpr bool ok = direct(); + + +constexpr void perform_assignment(int& left, int right) noexcept { + left = right; // { dg-error "accessing .+ member instead of initialized" } +} + +constexpr bool indirect() { + sso val; + val.alloc = nullptr; + perform_assignment(val.buf[5], 42); // { dg-message "in .constexpr. expansion" } + return true; +} +constexpr bool err = indirect(); // { dg-message "in .constexpr. expansion" } diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C new file mode 100644 index 00000000000..6d30bb2498f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union3.C @@ -0,0 +1,45 @@ +// { dg-do compile { target c++20 } } + +struct S +{ + union { + char buf[8]; + char* ptr; + }; + unsigned len; + + constexpr S(const char* s, unsigned n) + { + char* p; + if (n > 7) + p = ptr = new char[n+1]; + else + p = buf; + for (len = 0; len < n; ++len) + p[len] = s[len]; // { dg-error "accessing uninitialized member" } + p[len] = '\0'; + } + + constexpr ~S() + { + if (len > 7) + delete[] ptr; + } +}; + +constexpr bool test1() +{ + S s("test", 4); // { dg-message "in .constexpr. expansion" } + return true; +} + +constexpr bool a = test1(); // { dg-message "in .constexpr. expansion" } + + +constexpr bool test2() +{ + S s("hello world", 11); + return true; +} + +constexpr bool b = test2(); diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C new file mode 100644 index 00000000000..429ab203ac2 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union4.C @@ -0,0 +1,29 @@ +// { dg-do compile { target c++20 } } + +// from [class.union.general] p5 + +union A { int x; int y[4]; }; +struct B { A a; }; +union C { B b; int k; }; +constexpr int f() { + C c; // does not start lifetime of any union member + c.b.a.y[3] = 4; // OK, S(c.b.a.y[3]) contains c.b and c.b.a.y; + // creates objects to hold union members c.b and c.b.a.y + return c.b.a.y[3]; // OK, c.b.a.y refers to newly created object (see [basic.life]) +} +constexpr int a = f(); + +struct X { const int a; int b; }; +union Y { X x; int k; };// { dg-message "does not implicitly begin its lifetime" } +constexpr int g() { + Y y = { { 1, 2 } }; // OK, y.x is active union member ([class.mem]) + int n = y.x.a; + y.k = 4; // OK, ends lifetime of y.x, y.k is active member of union + + y.x.b = n; // { dg-error "accessing .* member instead of initialized .* member" } + // undefined behavior: y.x.b modified outside its lifetime, + // S(y.x.b) is empty because X's default constructor is deleted, + // so union member y.x's lifetime does not implicitly start + return 0; +} +constexpr int b = g(); // { dg-message "in .constexpr. expansion" } diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C new file mode 100644 index 00000000000..c94d7cc0fc9 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union5.C @@ -0,0 +1,71 @@ +// { dg-do compile { target c++20 } } + +union U { int a; int b; int c[2]; }; + +constexpr int test1() { + U u; + u.a = 10; + *&u.b = 20; // { dg-error "accessing" } + return u.b; +} +constexpr int x1 = test1(); // { dg-message "in .constexpr. expansion" } + +constexpr int test2() { + U u; + u.a = 10; + (0, u.b) = 20; // { dg-error "accessing" } + return u.b; +} +constexpr int x2 = test2(); // { dg-message "in .constexpr. expansion" } + +constexpr int test3() { + U u; + u.a = 0; + int* p = &u.b; + p[u.a] = 10; // { dg-error "accessing" } + return u.b; +} +constexpr int x3 = test3(); // { dg-message "in .constexpr. expansion" } + +constexpr int test4() { + U u; + u.a = 0; + int* p = &u.b; + u.a[p] = 10; // { dg-error "accessing" } + return u.b; +} +constexpr int x4 = test4(); // { dg-message "in .constexpr. expansion" } + +struct S { U u[10]; }; +constexpr int test5() { + S s; + s.u[4].a = 10; + 6[s.u].b = 15; + return 4[s.u].a + s.u[6].b; +} +static_assert(test5() == 25); + +constexpr int test6() { + U u; + u.a = 5; + u.c[0] = 3; + 1[u.c] = 8; + return 1[u.c] + u.c[0]; +} +static_assert(test6() == 11); + +constexpr int test7() { + U u {}; // value initialisation initialises first member + int* p = &u.a; + *p = 10; + return *p; +} +constexpr int x7 = test7(); + +constexpr int test8() { + U u; // default initialisation leaves no member initialised + int* p = &u.a; + *p = 10; // { dg-error "accessing" } + return *p; +} +constexpr int x8 = test8(); // { dg-message "in .constexpr. expansion" } diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C new file mode 100644 index 00000000000..7c37716892c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-union6.C @@ -0,0 +1,43 @@ +// { dg-do compile { target c++20 } } +// PR c++/102286 + +#include "construct_at.h" + +struct S { const int a; int b; }; +union U { int k; S s; }; + +constexpr int test1() { + U u {}; + std::construct_at(&u.s, S{ 1, 2 }); + return u.s.b; +} +static_assert(test1() == 2); + +constexpr int test2() { + U u {}; + int* p = &u.s.b; + std::construct_at(p, 5); // { dg-message "in .constexpr. expansion" } + return u.s.b; +} +constexpr int x2 = test2(); // { dg-message "in .constexpr. expansion" } + +struct S2 { int a; int b; }; +union U2 { int k; S2 s; }; +constexpr int test3() { + U2 u; + int* p = &u.s.b; + std::construct_at(p, 8); // { dg-message "in .constexpr. expansion" } + return u.s.b; +}; +constexpr int x3 = test3(); // { dg-message "in .constexpr. expansion" } + +constexpr int test4() { + union { + int data[1]; + } u; + std::construct_at(u.data, 0); // { dg-message "in .constexpr. expansion" } + return 0; +} +constexpr int x4 = test4(); // { dg-message "in .constexpr. expansion" } + +// { dg-error "accessing uninitialized member" "" { target *-*-* } 0 }