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.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 00c0dba0a554f9e5f370bacfa06fc3e73c88e8c5..40f5fef7baa0925f138fe0b5e9bb4acc1a7270e1 100644 (file)
--- 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 c1217535761eae02b5768faf9925af313caf33cf..baa97a0ef17769dfdb474902d55d7a40deacd0f6 100644 (file)
--- 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 bef452f592a45371a780ae4af6311b0aba3fb964..f31319904eb1b1d080188b1216a7beb821e2c12e 100644 (file)
--- 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 3b37c970dfa471ea8bc0382378c1b59edf715040..131da1a4ae40d780b7023bc0a64e6f6148fc21a9 100644 (file)
--- 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 f4be72fc514fb2db47c5a10973ff58f343af937d..9ba2897390a2ac6c132011b034cad80de3f9e155 100644 (file)
--- 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 (file)
index 0000000..cd9e8de
--- /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 (file)
index 0000000..7837acf
--- /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);