c++: Handle array members in build_comparison_op [PR93480]

http://eel.is/c++draft/class.compare.default#6 says for the
expanded list of subobjects:
"In that list, any subobject of array type is recursively expanded
to the sequence of its elements, in the order of increasing subscript."
but build_comparison_op just tried to compare the whole arrays, which
failed and therefore the defaulted comparison was deleted.

The following patch instead compares the array elements, and
if info.defining, adds runtime loops around it so that it iterates
over increasing subscripts.

For flexible array members it punts, we don't know how large those will be,
for zero sized arrays it doesn't even try to compare the elements,
because if there are no elements, there is nothing to compare, and
for [1] arrays it will not emit a loop because it is enough to use
[0] array ref to cover everything.

2020-12-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/93480
	* method.c (common_comparison_type): If comps[i] is a TREE_LIST,
	use its TREE_VALUE instead.
	(build_comparison_op): Handle array members.

	* g++.dg/cpp2a/spaceship-synth10.C: New test.
	* g++.dg/cpp2a/spaceship-synth-neg5.C: New test.

diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index da580a868b8e6cd479d36cfb91f31004c4f3c8c4..dd351734b5e3fef1a61bbf3cc9ce5ba50dd64575 100644 (file)
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1230,6 +1230,8 @@ common_comparison_type (vec<tree> &comps)
   for (unsigned i = 0; i < comps.length(); ++i)
     {
       tree comp = comps[i];
+      if (TREE_CODE (comp) == TREE_LIST)
+       comp = TREE_VALUE (comp);
       tree ctype = TREE_TYPE (comp);
       comp_cat_tag tag = cat_tag_for (ctype);
       /* build_comparison_op already checked this.  */
@@ -1419,10 +1421,47 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
              continue;
            }
 
-         tree lhs_mem = build3 (COMPONENT_REF, expr_type, lhs, field,
-                                NULL_TREE);
-         tree rhs_mem = build3 (COMPONENT_REF, expr_type, rhs, field,
-                                NULL_TREE);
+         tree lhs_mem = build3_loc (field_loc, COMPONENT_REF, expr_type, lhs,
+                                    field, NULL_TREE);
+         tree rhs_mem = build3_loc (field_loc, COMPONENT_REF, expr_type, rhs,
+                                    field, NULL_TREE);
+         tree loop_indexes = NULL_TREE;
+         while (TREE_CODE (expr_type) == ARRAY_TYPE)
+           {
+             /* Flexible array member.  */
+             if (TYPE_DOMAIN (expr_type) == NULL_TREE
+                 || TYPE_MAX_VALUE (TYPE_DOMAIN (expr_type)) == NULL_TREE)
+               {
+                 if (complain & tf_error)
+                   inform (field_loc, "cannot default compare "
+                                      "flexible array member");
+                 bad = true;
+                 break;
+               }
+             tree maxval = TYPE_MAX_VALUE (TYPE_DOMAIN (expr_type));
+             /* [0] array.  No subobjects to compare, just skip it.  */
+             if (integer_all_onesp (maxval))
+               break;
+             tree idx;
+             /* [1] array, no loop needed, just add [0] ARRAY_REF.
+                Similarly if !info.defining.  */
+             if (integer_zerop (maxval) || !info.defining)
+               idx = size_zero_node;
+             /* Some other array, will need runtime loop.  */
+             else
+               {
+                 idx = force_target_expr (sizetype, maxval, complain);
+                 loop_indexes = tree_cons (idx, NULL_TREE, loop_indexes);
+               }
+             expr_type = TREE_TYPE (expr_type);
+             lhs_mem = build4_loc (field_loc, ARRAY_REF, expr_type, lhs_mem,
+                                   idx, NULL_TREE, NULL_TREE);
+             rhs_mem = build4_loc (field_loc, ARRAY_REF, expr_type, rhs_mem,
+                                   idx, NULL_TREE, NULL_TREE);
+           }
+         if (TREE_CODE (expr_type) == ARRAY_TYPE)
+           continue;
+
          tree overload = NULL_TREE;
          tree comp = build_new_op (field_loc, code, flags, lhs_mem, rhs_mem,
                                    NULL_TREE, &overload,
@@ -1486,6 +1525,18 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
              bad = true;
              continue;
            }
+         /* Most of the time, comp is the expression that should be evaluated
+            to compare the two members.  If the expression needs to be
+            evaluated more than once in a loop, it will be a TREE_LIST
+            instead, whose TREE_VALUE is the expression for one array element,
+            TREE_PURPOSE is innermost iterator temporary and if the array
+            is multidimensional, TREE_CHAIN will contain another TREE_LIST
+            with second innermost iterator in its TREE_PURPOSE and so on.  */
+         if (loop_indexes)
+           {
+             TREE_VALUE (loop_indexes) = comp;
+             comp = loop_indexes;
+           }
          comps.safe_push (comp);
        }
       if (code == SPACESHIP_EXPR && is_auto (rettype))
@@ -1502,8 +1553,38 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
        {
          tree comp = comps[i];
          tree eq, retval = NULL_TREE, if_ = NULL_TREE;
+         tree loop_indexes = NULL_TREE;
          if (info.defining)
-           if_ = begin_if_stmt ();
+           {
+             if (TREE_CODE (comp) == TREE_LIST)
+               {
+                 loop_indexes = comp;
+                 comp = TREE_VALUE (comp);
+                 loop_indexes = nreverse (loop_indexes);
+                 for (tree loop_index = loop_indexes; loop_index;
+                      loop_index = TREE_CHAIN (loop_index))
+                   {
+                     tree for_stmt = begin_for_stmt (NULL_TREE, NULL_TREE);
+                     tree idx = TREE_PURPOSE (loop_index);
+                     tree maxval = TARGET_EXPR_INITIAL (idx);
+                     TARGET_EXPR_INITIAL (idx) = size_zero_node;
+                     add_stmt (idx);
+                     finish_init_stmt (for_stmt);
+                     finish_for_cond (build2 (LE_EXPR, boolean_type_node, idx,
+                                              maxval), for_stmt, false, 0);
+                     finish_for_expr (cp_build_unary_op (PREINCREMENT_EXPR,
+                                                         TARGET_EXPR_SLOT (idx),
+                                                         false, complain),
+                                                         for_stmt);
+                     /* Store in TREE_VALUE the for_stmt tree, so that we can
+                        later on call finish_for_stmt on it (in the reverse
+                        order).  */
+                     TREE_VALUE (loop_index) = for_stmt;
+                   }
+                 loop_indexes = nreverse (loop_indexes);
+               }
+             if_ = begin_if_stmt ();
+           }
          /* Spaceship is specified to use !=, but for the comparison category
             types, != is equivalent to !(==), so let's use == directly.  */
          if (code == EQ_EXPR)
@@ -1542,6 +1623,9 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
              finish_return_stmt (retval);
              finish_else_clause (if_);
              finish_if_stmt (if_);
+             for (tree loop_index = loop_indexes; loop_index;
+                  loop_index = TREE_CHAIN (loop_index))
+               finish_for_stmt (TREE_VALUE (loop_index));
            }
        }
       if (info.defining)

diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg5.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg5.C
new file mode 100644 (file)
index 0000000..a01d3d3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg5.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++20 } }
+// { dg-options "" }
+
+#include <compare>
+
+struct C {
+  int y;
+  int x[];                                     // { dg-message "cannot default compare flexible array member" }
+  auto operator<=>(C const&) const = default;  // { dg-message "is implicitly deleted because the default definition would be ill-formed" }
+};
+
+bool
+foo (C &c1, C &c2)
+{
+  return c1 > c2;      // { dg-error "use of deleted function" }
+}

diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth10.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth10.C
new file mode 100644 (file)
index 0000000..235e002
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth10.C
@@ -0,0 +1,57 @@
+// { dg-do run { target c++20 } }
+// { dg-options "" }
+
+#include <compare>
+
+struct C {
+  int y;
+  int x[4];
+  auto operator<=>(C const&) const = default;
+};
+
+struct D {
+  int y;
+  int x[1];
+  auto operator<=>(D const&) const = default;
+};
+
+struct E {
+  int y;
+  int x[0];
+  auto operator<=>(E const&) const = default;
+};
+
+int
+main ()
+{
+  constexpr C c1 = { 1, { 2, 3, 4, 5 } };
+  constexpr C c2 = { 1, { 2, 3, 5, 4 } };
+  constexpr C c3 = { 1, { 2, 2, 6, 7 } };
+  static_assert (c1 < c2);
+  static_assert (c3 < c1);
+  constexpr D d1 = { 1, { 2 } };
+  constexpr D d2 = { 1, { 3 } };
+  constexpr D d3 = { 1, { 1 } };
+  static_assert (d1 < d2);
+  static_assert (d3 < d1);
+  constexpr E e1 = { 1, {} };
+  constexpr E e2 = { 2, {} };
+  constexpr E e3 = { 1, {} };
+  static_assert (e1 < e2);
+  static_assert (e1 == e3);
+  C c4 = { 1, { 2, 3, 4, 5 } };
+  C c5 = { 1, { 2, 3, 5, 4 } };
+  C c6 = { 1, { 2, 2, 6, 7 } };
+  if (c4 >= c5 || c6 >= c4)
+    __builtin_abort ();
+  D d4 = { 1, { 2 } };
+  D d5 = { 1, { 3 } };
+  D d6 = { 1, { 1 } };
+  if (d4 >= d5 || d6 >= d4)
+    __builtin_abort ();
+  E e4 = { 1, {} };
+  E e5 = { 2, {} };
+  E e6 = { 1, {} };
+  if (e4 >= e5 || e4 != e6)
+    __builtin_abort ();
+}