[gcc r11-6918] c++: constexpr and empty fields [PR97566]

[gcc r11-6918] c++: constexpr and empty fields [PR97566]

[Jason Merrill]

https://gcc.gnu.org/g:a4dfd0f089af33f2af57bf422f9859405b9b4a16

commit r11-6918-ga4dfd0f089af33f2af57bf422f9859405b9b4a16
Author: Jason Merrill <jason@redhat.com>
Date:   Sun Jan 24 00:55:49 2021 -0500

    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:
---
 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 +-
 gcc/testsuite/g++.dg/cpp2a/no_unique_address10.C | 16 ++++++++
 gcc/testsuite/g++.dg/cpp2a/no_unique_address9.C  | 50 ++++++++++++++++++++++++
 7 files changed, 102 insertions(+), 20 deletions(-)

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);