On Fri, 6 May 2022, Patrick Palka wrote:

> On Fri, 6 May 2022, Jason Merrill wrote:
> 
> > On 5/6/22 11:22, Patrick Palka wrote:
> > > Here ever since r10-7313-gb599bf9d6d1e18, reduced_constant_expression_p
> > > in C++11/14 is rejecting the marked sub-aggregate initializer (of type S)
> > > 
> > >    W w = {.D.2445={.s={.D.2387={.m=0}, .b=0}}}
> > >                       ^
> > > 
> > > ultimately because said initializer has CONSTRUCTOR_NO_CLEARING set, and
> > > so the function proceeds to verify that all fields of S are initialized.
> > > And before C++17 we don't expect to see base class fields (since
> > > next_initializable_field skips over the), so the base class initializer
> > > causes r_c_e_p to return false.
> > 
> > That seems like the primary bug.  I guess r_c_e_p shouldn't be using
> > next_initializable_field.  Really that function should only be used for
> > aggregates.
> 
> I see, I'll try replacing it in r_c_e_p.  Would that be in addition to
> or instead of the clear_no_implicit_zero approach?

I'm testing the following, which uses a custom predicate instead of
next_initializable_field in r_c_e_p.

-- >8 --

gcc/cp/ChangeLog:

        * constexpr.cc (reduced_constant_expression_p): Replace use of
        next_initializable_field with custom predicate.  Refactor to
        remove the use of goto.
        * decl.cc (next_initializable_field): Skip over vptr fields.
        Document that this function is to be used only for aggregate
        classes.
---
 gcc/cp/constexpr.cc | 65 ++++++++++++++++++++++++++-------------------
 gcc/cp/decl.cc      | 15 +++++------
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 9b1e71857fc..d1cd556591c 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3016,7 +3016,6 @@ reduced_constant_expression_p (tree t)
 
     case CONSTRUCTOR:
       /* And we need to handle PTRMEM_CST wrapped in a CONSTRUCTOR.  */
-      tree field;
       if (CONSTRUCTOR_NO_CLEARING (t))
        {
          if (TREE_CODE (TREE_TYPE (t)) == VECTOR_TYPE)
@@ -3041,42 +3040,54 @@ reduced_constant_expression_p (tree t)
                }
              if (find_array_ctor_elt (t, max) == -1)
                return false;
-             goto ok;
            }
-         else if (cxx_dialect >= cxx20
-                  && TREE_CODE (TREE_TYPE (t)) == UNION_TYPE)
+         else if (TREE_CODE (TREE_TYPE (t)) == UNION_TYPE)
            {
-             if (CONSTRUCTOR_NELTS (t) == 0)
+             if (CONSTRUCTOR_NELTS (t) != 1)
                /* An initialized union has a constructor element.  */
                return false;
-             /* And it only initializes one member.  */
-             field = NULL_TREE;
+             if (!reduced_constant_expression_p (CONSTRUCTOR_ELT (t, 
0)->value))
+               return false;
            }
          else
-           field = next_initializable_field (TYPE_FIELDS (TREE_TYPE (t)));
+           {
+             /* Similar to the predicate used in next_initializable_field,
+                except that we additionally skip over empty types (for which
+                we don't require an initializer), and we always consider base
+                class fields (not just in C++17 mode) and vptr fields.  */
+             auto ignorable_field_p = [] (tree field) {
+               if (!field)
+                 return false;
+               return (TREE_CODE (field) != FIELD_DECL
+                       || DECL_UNNAMED_BIT_FIELD (field)
+                       || (DECL_ARTIFICIAL (field)
+                           && !DECL_FIELD_IS_BASE (field)
+                           && !DECL_VIRTUAL_P (field))
+                       || is_really_empty_class (TREE_TYPE (field),
+                                                 /*ignore_vptr*/false));
+             };
+             tree field = TYPE_FIELDS (TREE_TYPE (t));
+             for (auto &e: CONSTRUCTOR_ELTS (t))
+               {
+                 if (!reduced_constant_expression_p (e.value))
+                   return false;
+                 while (e.index != field && ignorable_field_p (field))
+                   field = DECL_CHAIN (field);
+                 if (e.index == field)
+                   field = DECL_CHAIN (field);
+                 else
+                   return false;
+               }
+             while (ignorable_field_p (field))
+               field = DECL_CHAIN (field);
+             if (field)
+               return false;
+           }
        }
       else
-       field = NULL_TREE;
-      for (auto &e: CONSTRUCTOR_ELTS (t))
-       {
-         /* If VAL is null, we're in the middle of initializing this
-            element.  */
+       for (auto &e: CONSTRUCTOR_ELTS (t))
          if (!reduced_constant_expression_p (e.value))
            return false;
-         /* Empty class field may or may not have an initializer.  */
-         for (; field && e.index != field;
-              field = next_initializable_field (DECL_CHAIN (field)))
-           if (!is_really_empty_class (TREE_TYPE (field),
-                                       /*ignore_vptr*/false))
-             return false;
-         if (field)
-           field = next_initializable_field (DECL_CHAIN (field));
-       }
-      /* There could be a non-empty field at the end.  */
-      for (; field; field = next_initializable_field (DECL_CHAIN (field)))
-       if (!is_really_empty_class (TREE_TYPE (field), /*ignore_vptr*/false))
-         return false;
-ok:
       if (CONSTRUCTOR_NO_CLEARING (t))
        /* All the fields are initialized.  */
        CONSTRUCTOR_NO_CLEARING (t) = false;
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index c9110db796a..c564e5e596d 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -6382,10 +6382,10 @@ struct reshape_iter
 
 static tree reshape_init_r (tree, reshape_iter *, tree, tsubst_flags_t);
 
-/* FIELD is an element of TYPE_FIELDS or NULL.  In the former case, the value
-   returned is the next FIELD_DECL (possibly FIELD itself) that can be
-   initialized.  If there are no more such fields, the return value
-   will be NULL.  */
+/* FIELD is an element of TYPE_FIELDS of an aggregate class, or NULL.  In the
+   former case, the value returned is the next FIELD_DECL (possibly FIELD
+   itself) that can be initialized.  If there are no more such fields, the
+   return value will be NULL.  */
 
 tree
 next_initializable_field (tree field)
@@ -6394,11 +6394,8 @@ next_initializable_field (tree field)
         && (TREE_CODE (field) != FIELD_DECL
             || DECL_UNNAMED_BIT_FIELD (field)
             || (DECL_ARTIFICIAL (field)
-                /* In C++17, don't skip base class fields.  */
-                && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field))
-                /* Don't skip vptr fields.  We might see them when we're
-                   called from reduced_constant_expression_p.  */
-                && !DECL_VIRTUAL_P (field))))
+                /* In C++17, aggregates can have base classes.  */
+                && !(cxx_dialect >= cxx17 && DECL_FIELD_IS_BASE (field)))))
     field = DECL_CHAIN (field);
 
   return field;
-- 
2.36.0.63.gf5aaf72f1b

Reply via email to