From a4dfd0f089af33f2af57bf422f9859405b9b4a16 Mon Sep 17 00:00:00 2001 From: Jason Merrill Date: Sun, 24 Jan 2021 00:55:49 -0500 Subject: [PATCH] c++: constexpr and empty fields [PR97566] In the discussion of PR98463, Jakub pointed out that in C++17 and up, cxx_fold_indirect_ref_1 could use the field we build for an empty base. I tried implementing that, but it broke one of the tuple tests, so I did some more digging. To start with, I generalized the PR98463 patch to handle the case where we do have a field, for an empty base or [[no_unique_address]] member. This is enough also for the no-field case because the member of the empty base must itself be an empty field; if it weren't, the base would not be empty. I looked for related PRs and found 97566, which was also fixed by the patch. After some poking around to figure out why, I noticed that the testcase had been breaking because E, though an empty class, has an ABI nvsize of one byte, and we were giving the [[no_unique_address]] FIELD_DECL that DECL_SIZE, whereas in build_base_field_1 empty base fields always get DECL_SIZE zero, and various places were relying on that to recognize empty fields. So I adjusted both the size and the checking. When I adjusted check_bases I wondered if we were correctly handling bases with only empty data members, but it appears we do. I'm deferring the cxx_fold_indirect_ref_1 change until stage 1, as I don't think it actually fixes anything. gcc/cp/ChangeLog: PR c++/97566 PR c++/98463 * class.c (layout_class_type): An empty field gets size 0. (is_empty_field): New. (check_bases): Check it. * cp-tree.h (is_empty_field): Declare it. * constexpr.c (cxx_eval_store_expression): Check it. (cx_check_missing_mem_inits): Likewise. * init.c (perform_member_init): Likewise. * typeck2.c (process_init_constructor_record): Likewise. gcc/testsuite/ChangeLog: PR c++/97566 * g++.dg/cpp2a/no_unique_address10.C: New test. * g++.dg/cpp2a/no_unique_address9.C: New test. --- gcc/cp/class.c | 31 +++++++++--- gcc/cp/constexpr.c | 20 +++----- gcc/cp/cp-tree.h | 1 + gcc/cp/init.c | 2 +- gcc/cp/typeck2.c | 2 +- .../g++.dg/cpp2a/no_unique_address10.C | 16 ++++++ .../g++.dg/cpp2a/no_unique_address9.C | 50 +++++++++++++++++++ 7 files changed, 102 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/no_unique_address10.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/no_unique_address9.C diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 00c0dba0a55..40f5fef7baa 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -1835,15 +1835,13 @@ check_bases (tree t, else if (CLASSTYPE_REPEATED_BASE_P (t)) CLASSTYPE_NON_STD_LAYOUT (t) = 1; else - /* ...either has no non-static data members in the most-derived - class and at most one base class with non-static data - members, or has no base classes with non-static data - members. FIXME This was reworded in DR 1813. */ + /* ...has all non-static data members and bit-fields in the class + and its base classes first declared in the same class. */ for (basefield = TYPE_FIELDS (basetype); basefield; basefield = DECL_CHAIN (basefield)) if (TREE_CODE (basefield) == FIELD_DECL && !(DECL_FIELD_IS_BASE (basefield) - && integer_zerop (DECL_SIZE (basefield)))) + && is_empty_field (basefield))) { if (field) CLASSTYPE_NON_STD_LAYOUT (t) = 1; @@ -4226,6 +4224,25 @@ field_poverlapping_p (tree decl) DECL_ATTRIBUTES (decl)); } +/* Return true iff DECL is an empty field, either for an empty base or a + [[no_unique_address]] data member. */ + +bool +is_empty_field (tree decl) +{ + if (TREE_CODE (decl) != FIELD_DECL) + return false; + + bool r = (is_empty_class (TREE_TYPE (decl)) + && (DECL_FIELD_IS_BASE (decl) + || field_poverlapping_p (decl))); + + /* Empty fields should have size zero. */ + gcc_checking_assert (!r || integer_zerop (DECL_SIZE (decl))); + + return r; +} + /* Record all of the empty subobjects of DECL_OR_BINFO. */ static void @@ -6612,7 +6629,9 @@ layout_class_type (tree t, tree *virtuals_p) /* end_of_class doesn't always give dsize, but it does in the case of a class with virtual bases, which is when dsize > nvsize. */ tree dsize = end_of_class (type, /*vbases*/true); - if (tree_int_cst_le (dsize, nvsize)) + if (CLASSTYPE_EMPTY_P (type)) + DECL_SIZE (field) = DECL_SIZE_UNIT (field) = size_zero_node; + else if (tree_int_cst_le (dsize, nvsize)) { DECL_SIZE_UNIT (field) = nvsize; DECL_SIZE (field) = CLASSTYPE_SIZE (type); diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index c1217535761..baa97a0ef17 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -821,7 +821,7 @@ cx_check_missing_mem_inits (tree ctype, tree body, bool complain) /* A flexible array can't be intialized here, so don't complain that it isn't. */ continue; - if (DECL_SIZE (field) && integer_zerop (DECL_SIZE (field))) + if (is_empty_field (field)) /* An empty field doesn't need an initializer. */ continue; ftype = strip_array_types (ftype); @@ -5291,17 +5291,13 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t, type = refs->pop(); tree index = refs->pop(); - if (TREE_CODE (index) == FIELD_DECL - && !(same_type_ignoring_top_level_qualifiers_p - (DECL_CONTEXT (index), TREE_TYPE (*valp)))) - { - /* INDEX isn't a member of *valp. This can happen if it's a member - of an empty base which isn't represented with a FIELD_DECL. Stop - trying to build a CONSTRUCTOR for the inner target; we'll notice - this disconnect again below and just return init. */ - gcc_assert (is_empty_class (DECL_CONTEXT (index))); - break; - } + if (is_empty_field (index)) + /* Don't build a sub-CONSTRUCTOR for an empty base or field, as they + have no data and might have an offset lower than previously declared + fields, which confuses the middle-end. The code below will notice + that we don't have a CONSTRUCTOR for our inner target and just + return init. */ + break; if (code == UNION_TYPE && CONSTRUCTOR_NELTS (*valp) && CONSTRUCTOR_ELT (*valp, 0)->index != index) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index bef452f592a..f31319904eb 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6515,6 +6515,7 @@ extern int same_signature_p (const_tree, const_tree); extern tree lookup_vfn_in_binfo (tree, tree); extern void maybe_add_class_template_decl_list (tree, tree, int); extern void unreverse_member_declarations (tree); +extern bool is_empty_field (tree); extern void invalidate_class_lookup_cache (void); extern void maybe_note_name_used_in_class (tree, tree); extern void note_name_declared_in_class (tree, tree); diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 3b37c970dfa..131da1a4ae4 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -877,7 +877,7 @@ perform_member_init (tree member, tree init) } if (init == error_mark_node) return; - if (DECL_SIZE (member) && integer_zerop (DECL_SIZE (member)) + if (is_empty_field (member) && !TREE_SIDE_EFFECTS (init)) /* Don't add trivial initialization of an empty base/field, as they might not be ordered the way the back-end expects. */ diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c index f4be72fc514..9ba2897390a 100644 --- a/gcc/cp/typeck2.c +++ b/gcc/cp/typeck2.c @@ -1626,7 +1626,7 @@ process_init_constructor_record (tree type, tree init, int nested, int flags, } } - if (DECL_SIZE (field) && integer_zerop (DECL_SIZE (field)) + if (is_empty_field (field) && !TREE_SIDE_EFFECTS (next)) /* Don't add trivial initialization of an empty base/field to the constructor, as they might not be ordered the way the back-end diff --git a/gcc/testsuite/g++.dg/cpp2a/no_unique_address10.C b/gcc/testsuite/g++.dg/cpp2a/no_unique_address10.C new file mode 100644 index 00000000000..cd9e8de4ad1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/no_unique_address10.C @@ -0,0 +1,16 @@ +// Make sure [[no_unique_address]] doesn't affect is_standard_layout. +// { dg-do compile { target c++11 } } + +struct E1 { }; struct E2 { }; +struct A +{ + [[no_unique_address]] E1 e; +}; + +struct B: A +{ + [[no_unique_address]] E2 e; +}; + +static_assert(__is_standard_layout (A), ""); +static_assert(!__is_standard_layout (B), ""); diff --git a/gcc/testsuite/g++.dg/cpp2a/no_unique_address9.C b/gcc/testsuite/g++.dg/cpp2a/no_unique_address9.C new file mode 100644 index 00000000000..7837acf0ea5 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/no_unique_address9.C @@ -0,0 +1,50 @@ +// PR c++/97566 +// { dg-do compile { target c++14 } } + +// error disappears if E doesn't inherit from B +struct B {}; +struct E : B {}; + +struct counter { + constexpr void inc() { size++; } + + // error disappears if you remove or reorder this value + int unused = 0; + int size = 0; + [[no_unique_address]] E empty = {}; +}; + +#define SA(X) static_assert((X),#X) + +constexpr int test1() { + counter x; + x.inc(); + return x.size; +} +SA(test1() == 1); + +constexpr int test2() { + counter x = { 0, 1, {} }; + x.inc(); + return x.size; +} +SA(test2() == 2); + +counter y; + +struct counter2 { + constexpr counter2() { inc(); } + constexpr void inc() { size++; } + + // error disappears if you remove or reorder this value + int unused = 0; + int size = 0; + [[no_unique_address]] E empty = {}; +}; + +constexpr int test3() { + counter2 x; + x.inc(); + return x.size; +} +SA(test3() == 2); -- 2.30.2