From ffd454b92ba6ff5499cf57f82a2b0f4cee59978c Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 22 Dec 2020 20:18:10 +0100 Subject: [PATCH] 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 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. --- gcc/cp/method.c | 94 ++++++++++++++++++- .../g++.dg/cpp2a/spaceship-synth-neg5.C | 16 ++++ .../g++.dg/cpp2a/spaceship-synth10.C | 57 +++++++++++ 3 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg5.C create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth10.C diff --git a/gcc/cp/method.c b/gcc/cp/method.c index da580a868b8..dd351734b5e 100644 --- a/gcc/cp/method.c +++ b/gcc/cp/method.c @@ -1230,6 +1230,8 @@ common_comparison_type (vec &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 index 00000000000..a01d3d3ead3 --- /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 + +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 index 00000000000..235e0020caf --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth10.C @@ -0,0 +1,57 @@ +// { dg-do run { target c++20 } } +// { dg-options "" } + +#include + +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 (); +} -- 2.30.2