[pushed] c++: Fix defaulted <=> fallback to < and == [PR96299]
Jason Merrill
jason@redhat.com
Tue Dec 8 20:05:09 GMT 2020
I thought I had implemented P1186R3, but apparently I didn't read it closely
enough to understand the point of the paper, namely that for a defaulted
operator<=>, if a member type doesn't have a viable operator<=>, we will use
its operator< and operator== if the defaulted operator has an specific
comparison category as its return type; the compiler can't guess if it
should be strong_ordering or something else, but the user can make that
choice explicit.
The libstdc++ test change was necessary because of the change in
genericize_spaceship from op0 > op1 to op1 < op0; this should be equivalent,
but isn't because of PR88173.
Tested x86_64-pc-linux-gnu, applying to trunk.
gcc/cp/ChangeLog:
PR c++/96299
* cp-tree.h (build_new_op): Add overload that omits some parms.
(genericize_spaceship): Add location_t parm.
* constexpr.c (cxx_eval_binary_expression): Pass it.
* cp-gimplify.c (genericize_spaceship): Pass it.
* method.c (genericize_spaceship): Handle class-type arguments.
(build_comparison_op): Fall back to op</== when appropriate.
gcc/testsuite/ChangeLog:
PR c++/96299
* g++.dg/cpp2a/spaceship-synth-neg2.C: Move error.
* g++.dg/cpp2a/spaceship-p1186.C: New test.
---
gcc/cp/cp-tree.h | 9 +-
gcc/cp/constexpr.c | 2 +-
gcc/cp/cp-gimplify.c | 2 +-
gcc/cp/method.c | 115 +++++++++++++----
gcc/testsuite/g++.dg/cpp2a/spaceship-p1186.C | 117 ++++++++++++++++++
.../g++.dg/cpp2a/spaceship-synth-neg2.C | 4 +-
.../comparisons/algorithms/partial_order.cc | 4 +-
7 files changed, 222 insertions(+), 31 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-p1186.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 66ad114567d..6270fadfe2b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6335,6 +6335,13 @@ extern tree build_new_op (const op_location_t &,
enum tree_code,
int, tree, tree, tree, tree *,
tsubst_flags_t);
+/* Wrapper that leaves out the usually-null op3 and overload parms. */
+inline tree build_new_op (const op_location_t &loc, enum tree_code code,
+ int flags, tree arg1, tree arg2,
+ tsubst_flags_t complain)
+{
+ return build_new_op (loc, code, flags, arg1, arg2, NULL_TREE, NULL, complain);
+}
extern tree build_op_call (tree, vec<tree, va_gc> **,
tsubst_flags_t);
extern bool aligned_allocation_fn_p (tree);
@@ -7807,7 +7814,7 @@ extern tree merge_types (tree, tree);
extern tree strip_array_domain (tree);
extern tree check_return_expr (tree, bool *);
extern tree spaceship_type (tree, tsubst_flags_t = tf_warning_or_error);
-extern tree genericize_spaceship (tree, tree, tree);
+extern tree genericize_spaceship (location_t, tree, tree, tree);
extern tree cp_build_binary_op (const op_location_t &,
enum tree_code, tree, tree,
tsubst_flags_t);
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index cb477c848d1..2ef6de83830 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3159,7 +3159,7 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t,
overflow_p);
else if (code == SPACESHIP_EXPR)
{
- r = genericize_spaceship (type, lhs, rhs);
+ r = genericize_spaceship (loc, type, lhs, rhs);
return cxx_eval_constant_expression (ctx, r, lval, non_constant_p,
overflow_p);
}
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 8bbcf017369..4f62398dfb0 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -882,7 +882,7 @@ static tree genericize_spaceship (tree expr)
tree type = TREE_TYPE (expr);
tree op0 = TREE_OPERAND (expr, 0);
tree op1 = TREE_OPERAND (expr, 1);
- return genericize_spaceship (type, op0, op1);
+ return genericize_spaceship (input_location, type, op0, op1);
}
/* If EXPR involves an anonymous VLA type, prepend a DECL_EXPR for that type
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 4de192fac00..da580a868b8 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1063,43 +1063,60 @@ spaceship_type (tree optype, tsubst_flags_t complain)
return lookup_comparison_category (tag, complain);
}
-/* Turn <=> with type TYPE and operands OP0 and OP1 into GENERIC. */
+/* Turn <=> with type TYPE and operands OP0 and OP1 into GENERIC.
+ This is also used by build_comparison_op for fallback to op< and op==
+ in a defaulted op<=>. */
tree
-genericize_spaceship (tree type, tree op0, tree op1)
+genericize_spaceship (location_t loc, tree type, tree op0, tree op1)
{
/* ??? maybe optimize based on knowledge of representation? */
comp_cat_tag tag = cat_tag_for (type);
+
+ if (tag == cc_last && is_auto (type))
+ {
+ /* build_comparison_op is checking to see if we want to suggest changing
+ the op<=> return type from auto to a specific comparison category; any
+ category will do for now. */
+ tag = cc_strong_ordering;
+ type = lookup_comparison_category (tag, tf_none);
+ if (type == error_mark_node)
+ return error_mark_node;
+ }
+
gcc_checking_assert (tag < cc_last);
tree r;
- op0 = save_expr (op0);
- op1 = save_expr (op1);
+ if (SCALAR_TYPE_P (TREE_TYPE (op0)))
+ {
+ op0 = save_expr (op0);
+ op1 = save_expr (op1);
+ }
tree gt = lookup_comparison_result (tag, type, 1);
+ int flags = LOOKUP_NORMAL;
+ tsubst_flags_t complain = tf_none;
+
if (tag == cc_partial_ordering)
{
/* op0 == op1 ? equivalent : op0 < op1 ? less :
- op0 > op1 ? greater : unordered */
+ op1 < op0 ? greater : unordered */
tree uo = lookup_comparison_result (tag, type, 3);
- tree comp = fold_build2 (GT_EXPR, boolean_type_node, op0, op1);
- r = fold_build3 (COND_EXPR, type, comp, gt, uo);
+ tree comp = build_new_op (loc, LT_EXPR, flags, op1, op0, complain);
+ r = build_conditional_expr (loc, comp, gt, uo, complain);
}
else
/* op0 == op1 ? equal : op0 < op1 ? less : greater */
r = gt;
tree lt = lookup_comparison_result (tag, type, 2);
- tree comp = fold_build2 (LT_EXPR, boolean_type_node, op0, op1);
- r = fold_build3 (COND_EXPR, type, comp, lt, r);
+ tree comp = build_new_op (loc, LT_EXPR, flags, op0, op1, complain);
+ r = build_conditional_expr (loc, comp, lt, r, complain);
tree eq = lookup_comparison_result (tag, type, 0);
- comp = fold_build2 (EQ_EXPR, boolean_type_node, op0, op1);
- r = fold_build3 (COND_EXPR, type, comp, eq, r);
-
- /* Wrap the whole thing in a TARGET_EXPR like build_conditional_expr_1. */
- r = get_target_expr (r);
+ comp = build_new_op (loc, EQ_EXPR, flags, op0, op1, complain);
+ r = build_conditional_expr (loc, comp, eq, r, complain);
return r;
}
@@ -1323,7 +1340,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
if (!info.defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
return;
- int flags = LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED;
+ int flags = LOOKUP_NORMAL;
const ovl_op_info_t *op = IDENTIFIER_OVL_OP_INFO (DECL_NAME (fndecl));
tree_code code = op->tree_code;
@@ -1364,6 +1381,10 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
if (code == EQ_EXPR || code == SPACESHIP_EXPR)
{
+ comp_cat_tag retcat = cc_last;
+ if (code == SPACESHIP_EXPR && !FNDECL_USED_AUTO (fndecl))
+ retcat = cat_tag_for (rettype);
+
bool bad = false;
auto_vec<tree> comps;
@@ -1375,13 +1396,15 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
{
tree expr_type = TREE_TYPE (field);
+ location_t field_loc = DECL_SOURCE_LOCATION (field);
+
/* A defaulted comparison operator function for class C is defined as
deleted if any non-static data member of C is of reference type or
C has variant members. */
if (TREE_CODE (expr_type) == REFERENCE_TYPE)
{
if (complain & tf_error)
- inform (DECL_SOURCE_LOCATION (field), "cannot default compare "
+ inform (field_loc, "cannot default compare "
"reference member %qD", field);
bad = true;
continue;
@@ -1390,7 +1413,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
&& next_initializable_field (TYPE_FIELDS (expr_type)))
{
if (complain & tf_error)
- inform (DECL_SOURCE_LOCATION (field), "cannot default compare "
+ inform (field_loc, "cannot default compare "
"anonymous union member");
bad = true;
continue;
@@ -1400,25 +1423,69 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
NULL_TREE);
tree rhs_mem = build3 (COMPONENT_REF, expr_type, rhs, field,
NULL_TREE);
- tree comp = build_new_op (info.loc, code, flags, lhs_mem, rhs_mem,
- NULL_TREE, NULL, complain);
+ tree overload = NULL_TREE;
+ tree comp = build_new_op (field_loc, code, flags, lhs_mem, rhs_mem,
+ NULL_TREE, &overload,
+ retcat != cc_last ? tf_none : complain);
if (comp == error_mark_node)
{
- bad = true;
- continue;
+ if (overload == NULL_TREE && code == SPACESHIP_EXPR
+ && (retcat != cc_last || complain))
+ {
+ tree comptype = (retcat != cc_last ? rettype
+ : DECL_SAVED_AUTO_RETURN_TYPE (fndecl));
+ /* No viable <=>, try using op< and op==. */
+ tree lteq = genericize_spaceship (field_loc, comptype,
+ lhs_mem, rhs_mem);
+ if (lteq != error_mark_node)
+ {
+ /* We found usable < and ==. */
+ if (retcat != cc_last)
+ /* Return type is a comparison category, use them. */
+ comp = lteq;
+ else if (complain & tf_error)
+ /* Return type is auto, suggest changing it. */
+ inform (info.loc, "changing the return type from %qs "
+ "to a comparison category type will allow the "
+ "comparison to use %qs and %qs", "auto",
+ "operator<", "operator==");
+ }
+ else if (retcat != cc_last && complain != tf_none)
+ /* No usable < and ==, give an error for op<=>. */
+ build_new_op (field_loc, code, flags, lhs_mem, rhs_mem,
+ complain);
+ }
+ if (comp == error_mark_node)
+ {
+ bad = true;
+ continue;
+ }
}
- if (code == SPACESHIP_EXPR
- && cat_tag_for (TREE_TYPE (comp)) == cc_last)
+ if (code != SPACESHIP_EXPR)
+ ;
+ else if (FNDECL_USED_AUTO (fndecl)
+ && cat_tag_for (TREE_TYPE (comp)) == cc_last)
{
/* The operator function is defined as deleted if ... Ri is not a
comparison category type. */
if (complain & tf_error)
- inform (DECL_SOURCE_LOCATION (field),
+ inform (field_loc,
"three-way comparison of %qD has type %qT, not a "
"comparison category type", field, TREE_TYPE (comp));
bad = true;
continue;
}
+ else if (!FNDECL_USED_AUTO (fndecl)
+ && !can_convert (rettype, TREE_TYPE (comp), complain))
+ {
+ if (complain & tf_error)
+ error_at (field_loc,
+ "three-way comparison of %qD has type %qT, which "
+ "does not convert to %qT",
+ field, TREE_TYPE (comp), rettype);
+ bad = true;
+ continue;
+ }
comps.safe_push (comp);
}
if (code == SPACESHIP_EXPR && is_auto (rettype))
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-p1186.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-p1186.C
new file mode 100644
index 00000000000..a979fcb65df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-p1186.C
@@ -0,0 +1,117 @@
+// PR c++/96299
+// P1186R3
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+
+struct H {
+ bool operator==(H const &) const;
+ bool operator<(H const &) const;
+};
+
+// Can't deduce category from op< and op==.
+struct A {
+ int i;
+ char c;
+ H h; // { dg-error "operator<=>" }
+ auto operator<=>(A const &) const = default;
+};
+
+// OK, explicit return type tells us how to interpret op</==.
+struct B {
+ int i;
+ char c;
+ H h;
+ std::strong_ordering operator<=>(B const &) const = default;
+};
+
+struct C {
+ bool operator<(C const &) const;
+};
+
+// C's op< isn't enough, need op== as well.
+struct D {
+ C c; // { dg-error "operator<=>" }
+ std::strong_ordering operator<=>(D const &) const = default;
+};
+
+struct E {
+ std::partial_ordering operator<=>(E const &) const;
+};
+
+// Can't use a weak op<=> to build a strong op<=>.
+struct F {
+ E e; // { dg-error "strong_ordering" }
+ H h;
+ std::strong_ordering operator<=>(F const &) const = default;
+};
+
+struct G {
+ std::strong_ordering operator<=>(G const &) const;
+};
+
+// OK, uses op<=> for g and op</op== for h.
+struct I {
+ G g;
+ H h;
+ std::partial_ordering operator<=>(I const &) const = default;
+};
+
+template <class T>
+struct J {
+ T t; // { dg-error "no match|ambiguous" }
+ auto operator<=>(J const &) const = default;
+};
+
+template <class T>
+struct K {
+ T t; // { dg-error "no match" }
+ std::partial_ordering operator<=>(K const &) const = default;
+};
+
+template <class T>
+struct M {
+ T t; // { dg-error "no match|strong_ordering" }
+ std::strong_ordering operator<=>(M const &) const = default;
+};
+
+// Test that we don't fall back to </== if <=> is ambiguous.
+struct N {
+ bool operator==(N const &) const;
+ bool operator<(N const &) const;
+};
+template <class T>
+std::partial_ordering operator<=>(N const &, T&&);
+template <class T>
+std::partial_ordering operator<=>(T&&, N const &);
+
+void
+foo (A &a1, A &a2, B &b1, B &b2, D& d1, D& d2, F& f1, F& f2, I& i1, I& i2)
+{
+ auto a = a1 < a2; // { dg-error "deleted" }
+ auto b = b1 < b2;
+ auto d = d1 < d2; // { dg-error "deleted" }
+ auto f = f1 < f2; // { dg-error "deleted" }
+ auto i = i1 < i2;
+
+ auto j1 = J<int>() < J<int>();
+ auto j2 = J<H>() < J<H>(); // { dg-error "deleted" }
+ auto j3 = J<C>() < J<C>(); // { dg-error "deleted" }
+ auto j4 = J<E>() < J<E>();
+ auto j5 = J<G>() < J<G>();
+ auto j6 = J<N>() < J<N>(); // { dg-error "deleted" }
+
+ auto k1 = K<int>() < K<int>();
+ auto k2 = K<H>() < K<H>();
+ auto k3 = K<C>() < K<C>(); // { dg-error "deleted" }
+ auto k4 = K<E>() < K<E>();
+ auto k5 = K<G>() < K<G>();
+ auto k6 = K<N>() < K<N>(); // { dg-error "deleted" }
+
+ auto m1 = M<int>() < M<int>();
+ auto m2 = M<H>() < M<H>();
+ auto m3 = M<C>() < M<C>(); // { dg-error "deleted" }
+ auto m4 = M<E>() < M<E>(); // { dg-error "deleted" }
+ auto m5 = M<G>() < M<G>();
+ auto m6 = M<N>() < M<N>(); // { dg-error "deleted" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg2.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg2.C
index 486db6cbbf9..c6acb7b5a80 100644
--- a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg2.C
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg2.C
@@ -14,9 +14,9 @@ bool operator<(const X<T>&, const X<T>&) { return true; }
struct Y
{
int a;
- X<int> c;
+ X<int> c; // { dg-error "no match" }
- auto operator <=>(Y const&) const = default; // { dg-error "no match" }
+ auto operator <=>(Y const&) const = default;
};
void f()
diff --git a/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc b/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc
index 62b379a98cb..094ac702874 100644
--- a/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc
+++ b/libstdc++-v3/testsuite/18_support/comparisons/algorithms/partial_order.cc
@@ -70,7 +70,7 @@ test03()
constexpr double epsilon = std::numeric_limits<double>::epsilon();
static_assert( partial_order(denorm, smallest) == partial_ordering::less );
static_assert( partial_order(denorm, 0.0) == partial_ordering::greater );
- // FIXME: these should all use static_assert
+ // FIXME: these should all use static_assert. See PR88173.
VERIFY( partial_order(0.0, nan) == partial_ordering::unordered );
VERIFY( partial_order(nan, nan) == partial_ordering::unordered );
VERIFY( partial_order(nan, 0.0) == partial_ordering::unordered );
@@ -81,7 +81,7 @@ test03()
VERIFY( partial_order(-inf, -nan) == partial_ordering::unordered );
static_assert( partial_order(max, inf) == partial_ordering::less );
static_assert( partial_order(inf, max) == partial_ordering::greater );
- static_assert( partial_order(inf, nan) == partial_ordering::unordered );
+ VERIFY( partial_order(inf, nan) == partial_ordering::unordered );
static_assert( partial_order(1.0, 1.0+epsilon) == partial_ordering::less );
}
base-commit: f7251a2c103bc48775cb9726a4bebeaebde96684
prerequisite-patch-id: e8b4ee7b9dc040dc0c4622f159e37ceb20607386
--
2.27.0
More information about the Gcc-patches
mailing list