| Message ID | 20221107205752.2735464-1-jason@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC(libstdc++)] c++: implement P2468R2, the equality operator you are looking for | expand |
On Mon, 7 Nov 2022 at 20:58, Jason Merrill <jason@redhat.com> wrote: > > Tested x86_64-pc-linux-gnu. Jonathan, what do you want to do about the library > test failure? > > -- >8 -- > > This paper is resolving the problem of well-formed C++17 code becoming > ambiguous in C++20 due to asymmetrical operator== being compared with itself > in reverse. I had previously implemented a tiebreaker such that if the two > candidates were functions with the same parameter types, we would prefer the > non-reversed candidate. But the committee went with a different approach: > if there's an operator!= with the same parameter types as the operator==, > don't consider the reversed form of the ==. > > So this patch implements that, and changes my old tiebreaker to give a > pedwarn if it is used. I also noticed that we were giving duplicate errors > for some testcases, and fixed the tourney logic to avoid that. > > As a result, a lot of tests of the form > > struct A { bool operator==(const A&); }; > > need to be fixed to add a const function-cv-qualifier, e.g. > > struct A { bool operator==(const A&) const; }; > > The committee thought such code ought to be fixed, so breaking it was fine. > > 18_support/comparisons/algorithms/fallback.cc also breaks with this patch, > because of the similarly asymmetrical > > bool operator==(const S&, S&) { return true; } > > I assume this was written this way deliberately, so I'm not sure what to do > about it. Yes, that was deliberate. The compare_strong_order_fallback function has these constraints: template<typename _Tp, __decayed_same_as<_Tp> _Up> requires __strongly_ordered<_Tp, _Up> || __op_eq_lt<_Tp, _Up> constexpr strong_ordering operator() [[nodiscard]] (_Tp&& __e, _Up&& __f) const And similarly for the other fallbacks. So I wanted to check that two types that decay to the same type (like S and const S) can be compared with == and <, and therefore can be used with this function. But if such asymmetry is no longer allowed, maybe the library function is no longer usable with pathological cases like the test, and so the test should be changed. We can't just replace the decayed_same_as constraint with same_as because the std::strong_order customization point still supports similar types, but we could do: --- a/libstdc++-v3/libsupc++/compare +++ b/libstdc++-v3/libsupc++/compare @@ -1057,11 +1057,11 @@ namespace std _GLIBCXX_VISIBILITY(default) }; template<typename _Tp, typename _Up> - concept __op_eq_lt = requires(_Tp&& __t, _Up&& __u) + concept __op_eq_lt = same_as<_Tp, _Up> && requires(_Tp&& __t) { - { static_cast<_Tp&&>(__t) == static_cast<_Up&&>(__u) } + { static_cast<_Tp&&>(__t) == static_cast<_Tp&&>(__t) } -> convertible_to<bool>; - { static_cast<_Tp&&>(__t) < static_cast<_Up&&>(__u) } + { static_cast<_Tp&&>(__t) < static_cast<_Tp&&>(__t) } -> convertible_to<bool>; }; And then adjust the test accordingly. If those fallbacks can no longer support mixed types, does the resolution of https://cplusplus.github.io/LWG/issue3465 even make sense now? If E and F must be the same type now, then E < F already implies F < E. I think we need some library changes to sync with P2468R2.
On Mon, 7 Nov 2022 at 21:56, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Mon, 7 Nov 2022 at 20:58, Jason Merrill <jason@redhat.com> wrote: > > > > Tested x86_64-pc-linux-gnu. Jonathan, what do you want to do about the library > > test failure? > > > > -- >8 -- > > > > This paper is resolving the problem of well-formed C++17 code becoming > > ambiguous in C++20 due to asymmetrical operator== being compared with itself > > in reverse. I had previously implemented a tiebreaker such that if the two > > candidates were functions with the same parameter types, we would prefer the > > non-reversed candidate. But the committee went with a different approach: > > if there's an operator!= with the same parameter types as the operator==, > > don't consider the reversed form of the ==. > > > > So this patch implements that, and changes my old tiebreaker to give a > > pedwarn if it is used. I also noticed that we were giving duplicate errors > > for some testcases, and fixed the tourney logic to avoid that. > > > > As a result, a lot of tests of the form > > > > struct A { bool operator==(const A&); }; > > > > need to be fixed to add a const function-cv-qualifier, e.g. > > > > struct A { bool operator==(const A&) const; }; > > > > The committee thought such code ought to be fixed, so breaking it was fine. > > > > 18_support/comparisons/algorithms/fallback.cc also breaks with this patch, > > because of the similarly asymmetrical > > > > bool operator==(const S&, S&) { return true; } > > > > I assume this was written this way deliberately, so I'm not sure what to do > > about it. > > Yes, that was deliberate. The compare_strong_order_fallback function > has these constraints: > > template<typename _Tp, __decayed_same_as<_Tp> _Up> > requires __strongly_ordered<_Tp, _Up> || __op_eq_lt<_Tp, _Up> > constexpr strong_ordering > operator() [[nodiscard]] (_Tp&& __e, _Up&& __f) const > > And similarly for the other fallbacks. So I wanted to check that two > types that decay to the same type (like S and const S) can be compared > with == and <, and therefore can be used with this function. > > But if such asymmetry is no longer allowed, maybe the library function > is no longer usable with pathological cases like the test, and so the > test should be changed. We can't just replace the decayed_same_as > constraint with same_as because the std::strong_order customization > point still supports similar types, but we could do: > > --- a/libstdc++-v3/libsupc++/compare > +++ b/libstdc++-v3/libsupc++/compare > @@ -1057,11 +1057,11 @@ namespace std _GLIBCXX_VISIBILITY(default) > }; > > template<typename _Tp, typename _Up> > - concept __op_eq_lt = requires(_Tp&& __t, _Up&& __u) > + concept __op_eq_lt = same_as<_Tp, _Up> && requires(_Tp&& __t) > { > - { static_cast<_Tp&&>(__t) == static_cast<_Up&&>(__u) } > + { static_cast<_Tp&&>(__t) == static_cast<_Tp&&>(__t) } > -> convertible_to<bool>; > - { static_cast<_Tp&&>(__t) < static_cast<_Up&&>(__u) } > + { static_cast<_Tp&&>(__t) < static_cast<_Tp&&>(__t) } > -> convertible_to<bool>; > }; No wait, that's nonsense. We can still try to compare similar types, it's just that they won't be comparable unless their comparison ops have two parameters of the same type. > And then adjust the test accordingly. If those fallbacks can no longer > support mixed types, does the resolution of > https://cplusplus.github.io/LWG/issue3465 even make sense now? If E > and F must be the same type now, then E < F already implies F < E. I > think we need some library changes to sync with P2468R2. I think this bit was right though. E and F might be different types, but E < F implies F < E. Is that right?
On 11/7/22 12:04, Jonathan Wakely wrote: > On Mon, 7 Nov 2022 at 21:56, Jonathan Wakely <jwakely@redhat.com> wrote: >> >> On Mon, 7 Nov 2022 at 20:58, Jason Merrill <jason@redhat.com> wrote: >>> >>> Tested x86_64-pc-linux-gnu. Jonathan, what do you want to do about the library >>> test failure? >>> >>> -- >8 -- >>> >>> This paper is resolving the problem of well-formed C++17 code becoming >>> ambiguous in C++20 due to asymmetrical operator== being compared with itself >>> in reverse. I had previously implemented a tiebreaker such that if the two >>> candidates were functions with the same parameter types, we would prefer the >>> non-reversed candidate. But the committee went with a different approach: >>> if there's an operator!= with the same parameter types as the operator==, >>> don't consider the reversed form of the ==. >>> >>> So this patch implements that, and changes my old tiebreaker to give a >>> pedwarn if it is used. I also noticed that we were giving duplicate errors >>> for some testcases, and fixed the tourney logic to avoid that. >>> >>> As a result, a lot of tests of the form >>> >>> struct A { bool operator==(const A&); }; >>> >>> need to be fixed to add a const function-cv-qualifier, e.g. >>> >>> struct A { bool operator==(const A&) const; }; >>> >>> The committee thought such code ought to be fixed, so breaking it was fine. >>> >>> 18_support/comparisons/algorithms/fallback.cc also breaks with this patch, >>> because of the similarly asymmetrical >>> >>> bool operator==(const S&, S&) { return true; } >>> >>> I assume this was written this way deliberately, so I'm not sure what to do >>> about it. >> >> Yes, that was deliberate. The compare_strong_order_fallback function >> has these constraints: >> >> template<typename _Tp, __decayed_same_as<_Tp> _Up> >> requires __strongly_ordered<_Tp, _Up> || __op_eq_lt<_Tp, _Up> >> constexpr strong_ordering >> operator() [[nodiscard]] (_Tp&& __e, _Up&& __f) const >> >> And similarly for the other fallbacks. So I wanted to check that two >> types that decay to the same type (like S and const S) can be compared >> with == and <, and therefore can be used with this function. >> >> But if such asymmetry is no longer allowed, maybe the library function >> is no longer usable with pathological cases like the test, and so the >> test should be changed. We can't just replace the decayed_same_as >> constraint with same_as because the std::strong_order customization >> point still supports similar types, but we could do: >> >> --- a/libstdc++-v3/libsupc++/compare >> +++ b/libstdc++-v3/libsupc++/compare >> @@ -1057,11 +1057,11 @@ namespace std _GLIBCXX_VISIBILITY(default) >> }; >> >> template<typename _Tp, typename _Up> >> - concept __op_eq_lt = requires(_Tp&& __t, _Up&& __u) >> + concept __op_eq_lt = same_as<_Tp, _Up> && requires(_Tp&& __t) >> { >> - { static_cast<_Tp&&>(__t) == static_cast<_Up&&>(__u) } >> + { static_cast<_Tp&&>(__t) == static_cast<_Tp&&>(__t) } >> -> convertible_to<bool>; >> - { static_cast<_Tp&&>(__t) < static_cast<_Up&&>(__u) } >> + { static_cast<_Tp&&>(__t) < static_cast<_Tp&&>(__t) } >> -> convertible_to<bool>; >> }; > > No wait, that's nonsense. We can still try to compare similar types, > it's just that they won't be comparable unless their comparison ops > have two parameters of the same type. Basically, though in this case the problem is that the arguments are the same type and the parameters are different. >> And then adjust the test accordingly. If those fallbacks can no longer >> support mixed types, does the resolution of >> https://cplusplus.github.io/LWG/issue3465 even make sense now? If E >> and F must be the same type now, then E < F already implies F < E. I >> think we need some library changes to sync with P2468R2. > > I think this bit was right though. E and F might be different types, > but E < F implies F < E. Is that right? The P2468 changes only affect ==/!=, not <, so LWG3465 should be unaffected. I think the only problem is the test itself: the <S,S> asserts need to be inverted because S and S cannot be compared for equality with the asymmetrical op== due to the normal candidate being better for arg 2 and the reversed candidate being better for arg 1. The <const S, S> asserts are fine because the arguments match the asymmetry, so the normal candidate is better for both args. So, the below fixes the test, does it make sense to you? Jason
On Mon, 7 Nov 2022 at 22:49, Jason Merrill <jason@redhat.com> wrote: > > On 11/7/22 12:04, Jonathan Wakely wrote: > > On Mon, 7 Nov 2022 at 21:56, Jonathan Wakely <jwakely@redhat.com> wrote: > >> > >> On Mon, 7 Nov 2022 at 20:58, Jason Merrill <jason@redhat.com> wrote: > >>> > >>> Tested x86_64-pc-linux-gnu. Jonathan, what do you want to do about the library > >>> test failure? > >>> > >>> -- >8 -- > >>> > >>> This paper is resolving the problem of well-formed C++17 code becoming > >>> ambiguous in C++20 due to asymmetrical operator== being compared with itself > >>> in reverse. I had previously implemented a tiebreaker such that if the two > >>> candidates were functions with the same parameter types, we would prefer the > >>> non-reversed candidate. But the committee went with a different approach: > >>> if there's an operator!= with the same parameter types as the operator==, > >>> don't consider the reversed form of the ==. > >>> > >>> So this patch implements that, and changes my old tiebreaker to give a > >>> pedwarn if it is used. I also noticed that we were giving duplicate errors > >>> for some testcases, and fixed the tourney logic to avoid that. > >>> > >>> As a result, a lot of tests of the form > >>> > >>> struct A { bool operator==(const A&); }; > >>> > >>> need to be fixed to add a const function-cv-qualifier, e.g. > >>> > >>> struct A { bool operator==(const A&) const; }; > >>> > >>> The committee thought such code ought to be fixed, so breaking it was fine. > >>> > >>> 18_support/comparisons/algorithms/fallback.cc also breaks with this patch, > >>> because of the similarly asymmetrical > >>> > >>> bool operator==(const S&, S&) { return true; } > >>> > >>> I assume this was written this way deliberately, so I'm not sure what to do > >>> about it. > >> > >> Yes, that was deliberate. The compare_strong_order_fallback function > >> has these constraints: > >> > >> template<typename _Tp, __decayed_same_as<_Tp> _Up> > >> requires __strongly_ordered<_Tp, _Up> || __op_eq_lt<_Tp, _Up> > >> constexpr strong_ordering > >> operator() [[nodiscard]] (_Tp&& __e, _Up&& __f) const > >> > >> And similarly for the other fallbacks. So I wanted to check that two > >> types that decay to the same type (like S and const S) can be compared > >> with == and <, and therefore can be used with this function. > >> > >> But if such asymmetry is no longer allowed, maybe the library function > >> is no longer usable with pathological cases like the test, and so the > >> test should be changed. We can't just replace the decayed_same_as > >> constraint with same_as because the std::strong_order customization > >> point still supports similar types, but we could do: > >> > >> --- a/libstdc++-v3/libsupc++/compare > >> +++ b/libstdc++-v3/libsupc++/compare > >> @@ -1057,11 +1057,11 @@ namespace std _GLIBCXX_VISIBILITY(default) > >> }; > >> > >> template<typename _Tp, typename _Up> > >> - concept __op_eq_lt = requires(_Tp&& __t, _Up&& __u) > >> + concept __op_eq_lt = same_as<_Tp, _Up> && requires(_Tp&& __t) > >> { > >> - { static_cast<_Tp&&>(__t) == static_cast<_Up&&>(__u) } > >> + { static_cast<_Tp&&>(__t) == static_cast<_Tp&&>(__t) } > >> -> convertible_to<bool>; > >> - { static_cast<_Tp&&>(__t) < static_cast<_Up&&>(__u) } > >> + { static_cast<_Tp&&>(__t) < static_cast<_Tp&&>(__t) } > >> -> convertible_to<bool>; > >> }; > > > > No wait, that's nonsense. We can still try to compare similar types, > > it's just that they won't be comparable unless their comparison ops > > have two parameters of the same type. > > Basically, though in this case the problem is that the arguments are the > same type and the parameters are different. Ah, so the operator== isn't actually rejected (despite being pathologically dumb) it's just that some uses of it result in ambiguities. > >> And then adjust the test accordingly. If those fallbacks can no longer > >> support mixed types, does the resolution of > >> https://cplusplus.github.io/LWG/issue3465 even make sense now? If E > >> and F must be the same type now, then E < F already implies F < E. I > >> think we need some library changes to sync with P2468R2. > > > > I think this bit was right though. E and F might be different types, > > but E < F implies F < E. Is that right? > > The P2468 changes only affect ==/!=, not <, so LWG3465 should be unaffected. Gotcha. > I think the only problem is the test itself: the <S,S> asserts need to > be inverted because S and S cannot be compared for equality with the > asymmetrical op== due to the normal candidate being better for arg 2 and > the reversed candidate being better for arg 1. The <const S, S> asserts > are fine because the arguments match the asymmetry, so the normal > candidate is better for both args. Gotcha. > So, the below fixes the test, does > it make sense to you? Yep, looks good. Thanks.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index d13bb3d4c0e..bbc8be21900 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6820,6 +6820,7 @@ extern void note_break_stmt (void); extern bool note_iteration_stmt_body_start (void); extern void note_iteration_stmt_body_end (bool); extern void determine_local_discriminator (tree); +extern bool fns_correspond (tree, tree); extern int decls_match (tree, tree, bool = true); extern bool maybe_version_functions (tree, tree, bool); extern bool merge_default_template_args (tree, tree, bool); diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 2c0fa37f53a..492db9b59ad 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -6232,6 +6232,7 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args, bool check_list_ctor = false; bool check_converting = false; unification_kind_t strict; + tree ne_fns = NULL_TREE; if (!fns) return; @@ -6269,6 +6270,32 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args, ctype = conversion_path ? BINFO_TYPE (conversion_path) : NULL_TREE; } + /* P2468: Check if operator== is a rewrite target with first operand + (*args)[0]; for now just do the lookups. */ + if ((flags & (LOOKUP_REWRITTEN | LOOKUP_REVERSED)) + && DECL_OVERLOADED_OPERATOR_IS (fn, EQ_EXPR)) + { + tree ne_name = ovl_op_identifier (false, NE_EXPR); + if (DECL_CLASS_SCOPE_P (fn)) + { + ne_fns = lookup_fnfields (TREE_TYPE ((*args)[0]), ne_name, + 1, tf_none); + if (ne_fns == error_mark_node || ne_fns == NULL_TREE) + ne_fns = NULL_TREE; + else + ne_fns = BASELINK_FUNCTIONS (ne_fns); + } + else + { + tree context = decl_namespace_context (fn); + ne_fns = lookup_qualified_name (context, ne_name, LOOK_want::NORMAL, + /*complain*/false); + if (ne_fns == error_mark_node + || !is_overloaded_fn (ne_fns)) + ne_fns = NULL_TREE; + } + } + if (first_arg) non_static_args = args; else @@ -6345,6 +6372,27 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args, continue; } + /* When considering reversed operator==, if there's a corresponding + operator!= in the same scope, it's not a rewrite target. */ + if (ne_fns) + { + bool found = false; + for (lkp_iterator ne (ne_fns); !found && ne; ++ne) + if (0 && !ne.using_p () + && DECL_NAMESPACE_SCOPE_P (fn) + && DECL_CONTEXT (*ne) != DECL_CONTEXT (fn)) + /* ??? This kludge excludes inline namespace members for the H + test in spaceship-eq15.C, but I don't see why we would want + that behavior. Asked Core 2022-11-04. Disabling for now. */; + else if (fns_correspond (fn, *ne)) + { + found = true; + break; + } + if (found) + continue; + } + if (TREE_CODE (fn) == TEMPLATE_DECL) { if (!add_template_candidate (candidates, @@ -6917,10 +6965,12 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags, gcc_checking_assert (cand->reversed ()); gcc_fallthrough (); case NE_EXPR: + if (result == error_mark_node) + ; /* If a rewritten operator== candidate is selected by overload resolution for an operator @, its return type shall be cv bool.... */ - if (TREE_CODE (TREE_TYPE (result)) != BOOLEAN_TYPE) + else if (TREE_CODE (TREE_TYPE (result)) != BOOLEAN_TYPE) { if (complain & tf_error) { @@ -12488,10 +12538,53 @@ joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn, if (winner && comp != winner) { /* Ambiguity between normal and reversed comparison operators - with the same parameter types; prefer the normal one. */ - if ((cand1->reversed () != cand2->reversed ()) + with the same parameter types. P2468 decided not to go with + this approach to resolving the ambiguity, so pedwarn. */ + if ((complain & tf_warning_or_error) + && (cand1->reversed () != cand2->reversed ()) && cand_parms_match (cand1, cand2)) - return cand1->reversed () ? -1 : 1; + { + struct z_candidate *w, *l; + if (cand2->reversed ()) + winner = 1, w = cand1, l = cand2; + else + winner = -1, w = cand2, l = cand1; + if (warn) + { + auto_diagnostic_group d; + if (pedwarn (input_location, 0, + "C++20 says that these are ambiguous, " + "even though the second is reversed:")) + { + print_z_candidate (input_location, + N_("candidate 1:"), w); + print_z_candidate (input_location, + N_("candidate 2:"), l); + if (w->fn == l->fn + && DECL_NONSTATIC_MEMBER_FUNCTION_P (w->fn) + && (type_memfn_quals (TREE_TYPE (w->fn)) + & TYPE_QUAL_CONST) == 0) + { + /* Suggest adding const to + struct A { bool operator==(const A&); }; */ + tree parmtype + = FUNCTION_FIRST_USER_PARMTYPE (w->fn); + parmtype = TREE_VALUE (parmtype); + if (TYPE_REF_P (parmtype) + && TYPE_READONLY (TREE_TYPE (parmtype)) + && (same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (parmtype), + DECL_CONTEXT (w->fn)))) + inform (DECL_SOURCE_LOCATION (w->fn), + "try making the operator a %<const%> " + "member function"); + } + } + } + else + add_warning (w, l); + return winner; + } winner = 0; goto tweak; @@ -12880,7 +12973,7 @@ tourney (struct z_candidate *candidates, tsubst_flags_t complain) { struct z_candidate *champ = candidates, *challenger; int fate; - int champ_compared_to_predecessor = 0; + struct z_candidate *champ_compared_to_predecessor = nullptr; /* Walk through the list once, comparing each current champ to the next candidate, knocking out a candidate or two with each comparison. */ @@ -12897,12 +12990,12 @@ tourney (struct z_candidate *candidates, tsubst_flags_t complain) champ = challenger->next; if (champ == 0) return NULL; - champ_compared_to_predecessor = 0; + champ_compared_to_predecessor = nullptr; } else { + champ_compared_to_predecessor = champ; champ = challenger; - champ_compared_to_predecessor = 1; } challenger = champ->next; @@ -12914,7 +13007,7 @@ tourney (struct z_candidate *candidates, tsubst_flags_t complain) for (challenger = candidates; challenger != champ - && !(champ_compared_to_predecessor && challenger->next == champ); + && challenger != champ_compared_to_predecessor; challenger = challenger->next) { fate = joust (champ, challenger, 0, complain); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 6e98ea35a39..890cfcabd35 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -980,6 +980,72 @@ function_requirements_equivalent_p (tree newfn, tree oldfn) return cp_tree_equal (reqs1, reqs2); } +/* Two functions of the same name correspond [basic.scope.scope] if + + + both declare functions with the same non-object-parameter-type-list, + equivalent ([temp.over.link]) trailing requires-clauses (if any, except as + specified in [temp.friend]), and, if both are non-static members, they have + corresponding object parameters, or + + + both declare function templates with equivalent + non-object-parameter-type-lists, return types (if any), template-heads, and + trailing requires-clauses (if any), and, if both are non-static members, + they have corresponding object parameters. + + This is a subset of decls_match: it identifies declarations that cannot be + overloaded with one another. This function does not consider DECL_NAME. */ + +bool +fns_correspond (tree newdecl, tree olddecl) +{ + if (TREE_CODE (newdecl) != TREE_CODE (olddecl)) + return false; + + if (TREE_CODE (newdecl) == TEMPLATE_DECL) + { + if (!template_heads_equivalent_p (newdecl, olddecl)) + return 0; + newdecl = DECL_TEMPLATE_RESULT (newdecl); + olddecl = DECL_TEMPLATE_RESULT (olddecl); + } + + tree f1 = TREE_TYPE (newdecl); + tree f2 = TREE_TYPE (olddecl); + + int rq1 = type_memfn_rqual (f1); + int rq2 = type_memfn_rqual (f2); + + /* If only one is a non-static member function, ignore ref-quals. */ + if (TREE_CODE (f1) != TREE_CODE (f2)) + rq1 = rq2; + /* Two non-static member functions have corresponding object parameters if: + + exactly one is an implicit object member function with no ref-qualifier + and the types of their object parameters ([dcl.fct]), after removing + top-level references, are the same, or + + their object parameters have the same type. */ + /* ??? We treat member functions of different classes as corresponding even + though that means the object parameters have different types. */ + else if ((rq1 == REF_QUAL_NONE) != (rq2 == REF_QUAL_NONE)) + rq1 = rq2; + + bool types_match = rq1 == rq2; + + if (types_match) + { + tree p1 = FUNCTION_FIRST_USER_PARMTYPE (newdecl); + tree p2 = FUNCTION_FIRST_USER_PARMTYPE (olddecl); + types_match = compparms (p1, p2); + } + + /* Two function declarations match if either has a requires-clause + then both have a requires-clause and their constraints-expressions + are equivalent. */ + if (types_match && flag_concepts) + types_match = function_requirements_equivalent_p (newdecl, olddecl); + + return types_match; +} + /* Subroutine of duplicate_decls: return truthvalue of whether or not types of these decls match. diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index c3fc56a13ff..57917de321f 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -20937,8 +20937,9 @@ tsubst_copy_and_build (tree t, /* In a lambda fn, we have to be careful to not introduce new this captures. Legacy code can't be using lambdas anyway, so it's ok to be - stricter. Be strict with C++20 template-id ADL too. */ - bool strict = in_lambda || template_id_p; + stricter. Be strict with C++20 template-id ADL too. + And be strict if we're already failing anyway. */ + bool strict = in_lambda || template_id_p || seen_error(); bool diag = true; if (strict) error_at (cp_expr_loc_or_input_loc (t), diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted3.C b/gcc/testsuite/g++.dg/cpp0x/defaulted3.C index 75e89c8ff0c..33de973a1fa 100644 --- a/gcc/testsuite/g++.dg/cpp0x/defaulted3.C +++ b/gcc/testsuite/g++.dg/cpp0x/defaulted3.C @@ -4,7 +4,7 @@ template<class T> struct A { template<class U> - bool operator==(const A<U>&) = delete; // { dg-message "declared" } + bool operator==(const A<U>&) const = delete; // { dg-message "declared" } operator bool () { return true; } }; diff --git a/gcc/testsuite/g++.dg/cpp2a/bit-cast7.C b/gcc/testsuite/g++.dg/cpp2a/bit-cast7.C index 4a3c6820070..6927db3c961 100644 --- a/gcc/testsuite/g++.dg/cpp2a/bit-cast7.C +++ b/gcc/testsuite/g++.dg/cpp2a/bit-cast7.C @@ -16,7 +16,7 @@ struct J struct K { long int a, b : 11, c; - constexpr bool operator == (const K &x) + constexpr bool operator == (const K &x) const { return a == x.a && b == x.b && c == x.c; } @@ -29,7 +29,7 @@ struct L struct M { long long int a, b : 11, c; - constexpr bool operator == (const M &x) + constexpr bool operator == (const M &x) const { return a == x.a && b == x.b && c == x.c; } diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-eq15.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq15.C new file mode 100644 index 00000000000..dc509563140 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq15.C @@ -0,0 +1,208 @@ +// P2468R2 - The Equality Operator You Are Looking For +// { dg-do compile { target c++20 } } + +struct A { + bool operator== (const A &) { return true; } + bool operator!= (const A &) { return false; } +}; +bool a = A{} != A{}; + +template <typename T> +struct B { + bool operator== (const T &) const; + bool operator!= (const T &) const; +}; +struct B1 : B<B1> { }; +bool b1 = B1{} == B1{}; +bool b2 = B1{} != B1{}; + +template <bool> +struct C { + using C1 = C<true>; + using C2 = C<false>; + C () = default; + C (const C2 &); + bool operator== (C1) const; + bool operator!= (C1) const; +}; +using C3 = C<false>; +bool c = C3{} == C3{}; + +struct D { + D (); + D (int *); + bool operator== (const D &) const; // { dg-message "candidate: 'bool D::operator==\\\(const D&\\\) const' \\\(reversed\\\)" } + operator int * () const; +}; +bool d = nullptr != D{}; // { dg-error "ambiguous overload for 'operator!=' in 'nullptr != D\\\(\\\)' \\\(operand types are 'std::nullptr_t' and 'D'\\\)" } + // { dg-message "candidate: 'operator!=\\\(int\\\*, int\\\*\\\)' \\\(built-in\\\)" "" { target *-*-* } .-1 } + +using ubool = unsigned char; + +struct E { + operator bool () const; +}; +unsigned char operator== (E, E);// { dg-message "candidate: 'unsigned char operator==\\\(E, E\\\)'" } + // { dg-message "no known conversion for argument 1 from 'int' to 'E'" "" { target *-*-* } .-1 } +unsigned char e = E{} != E{}; // { dg-error "return type of 'unsigned char operator==\\\(E, E\\\)' is not 'bool'" } + // { dg-message "used as rewritten candidate for comparison of 'E' and 'E'" "" { target *-*-* } .-1 } + +// F-H are the testcase from [over.match.oper] +struct F {}; +template <typename T> +bool operator== (F, T); // { dg-message "candidate: 'template<class T> bool operator==\\\(F, T\\\)'" } + // { dg-message "template argument deduction/substitution failed:" "" { target *-*-* } .-1 } +bool f1 = 0 == F (); // OK, calls reversed == +template <typename T> +bool operator!= (F, T); +bool f2 = 0 == F (); // { dg-error "no match for 'operator==' in '0 == F\\\(\\\)' \\\(operand types are 'int' and 'F'\\\)" } + // { dg-message "cannot convert '0' \\\(type 'int'\\\) to type 'F'" "" { target *-*-* } .-1 } + +struct G { + bool operator== (const G &); +}; +struct G1 : G { + G1 (); + G1 (G); + bool operator!= (const G &); +}; +bool g1 = G () == G1 (); // OK, != prevents rewrite +bool g2 = G1 () == G (); // { dg-error "ambiguous, even though the second is reversed" } + +struct H {}; +template <typename T> +bool operator== (H, T); +inline namespace H1 { + template <typename T> + bool operator!= (H, T); +} +// [over.match.oper] currently says that this is OK because the inline +// namespace isn't searched, but that seems wrong to me, so I'm going to go +// ahead and search it for now. Remove the "0 &&" in add_candidates to +// change this to the currently specified behavior. +// { dg-error "no match" "" { target *-*-* } .+1 } +bool h = 0 == H (); // OK, calls reversed == + +template <class T> +struct I { + int operator== (const double &) const; + friend inline int operator== (const double &, const T &) { return 1; } +}; +struct I1 : I<I1> { }; +bool i = I1{} == 0.; // { dg-error "return type of 'int operator==\\\(const double&, const I1&\\\)' is not 'bool'" } + // { dg-message "used as rewritten candidate for comparison of 'I1' and 'double'" "" { target *-*-* } .-1 } + +struct J { + bool operator== (const J &) const; + bool operator!= (const J &) const; +}; +struct J1 : J { + J1 (const J &); + bool operator== (const J1 &x) const { + return static_cast<const J &> (*this) == x; // { dg-error "ambiguous overload for 'operator==' in '\\\*\\\(const J\\\*\\\)\\\(\\\(const J1\\\*\\\)this\\\) == x' \\\(operand types are 'const J' and 'const J1'\\\)" } + } +}; + +struct K { + bool operator== (const K &); +}; +bool k = K{} == K{}; // { dg-error "ambiguous, even though the second is reversed" } + +struct L { + bool operator== (const L &) const; +}; +bool l = L{} == L{}; + +struct M { + bool operator== (M); +}; +bool m = M () == M (); + +struct N { + virtual bool operator== (const N &) const; +}; +struct N1 : N { + bool operator== (const N &) const override; +}; +bool n = N1 () == N1 (); // { dg-error "ambiguous, even though the second is reversed" } + +struct O { + virtual signed char operator== (const O &) const; + signed char operator!= (const O &x) const { return !operator== (x); } +}; +struct O1 : O { + signed char operator== (const O &) const override; +}; +bool o = O1 () != O1 (); + +template <class T> +bool +foo (T x, T y) +requires requires { x == y; } +{ + return x == y; +} +bool b3 = foo (B1 (), B1 ()); + +struct P {}; +template <typename T, class U = int> +bool operator== (P, T); +template <class T> +bool operator!= (P, T); +bool p = 0 == P (); + +struct Q {}; +template <typename T> +bool operator== (Q, T); +template <typename U> +bool operator!= (Q, U); +bool q = 0 == Q (); // { dg-error "" } + +struct R { + template <typename T> + bool operator== (const T &); +}; +bool r = R () == R (); // { dg-error "ambiguous, even though the second is reversed" } + +struct S { + template <typename T> + bool operator== (const T &) const; + bool operator!= (const S &); +}; +bool s = S () == S (); + +struct T {}; +template <class U = int> +bool operator== (T, int); +bool operator!= (T, int); +bool t = 0 == T (); + +struct U {}; +bool operator== (U, int); +bool u1 = 0 == U (); +namespace U1 { bool operator!= (U, int); } +bool u2 = 0 == U (); +using U1::operator!=; +bool u3 = 0 == U (); // { dg-error "" } + +struct V {}; +template <typename T> +bool operator== (V, T); +bool v1 = 0 == V (); +namespace V1 { template <typename T> bool operator!= (V, T); } +bool v2 = 0 == V (); +using V1::operator!=; +bool v3 = 0 == V (); // { dg-error "" } + +template <int N> +struct W { +bool operator== (int) requires (N == 1); +bool operator!= (int) requires (N == 2); +}; +int w = 0 == W<1> (); + +struct X { + bool operator== (X const &); + static bool operator!= (X const &, X const &); // { dg-error "'static bool X::operator!=\\\(const X&, const X&\\\)' must be either a non-static member function or a non-member function" } +}; +bool x = X () == X (); // { dg-error "ambiguous, even though the second is reversed" } diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite1.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite1.C index c4030cd2f4d..ebe81e4b5e9 100644 --- a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite1.C +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite1.C @@ -11,5 +11,5 @@ int main() { A<int> a1; A<void> a2; - return a1 == a2; + return a1 == a2; // { dg-error "ambiguous, even though the second is reversed" } } diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite5.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite5.C index d0424377e8e..460f6332938 100644 --- a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite5.C +++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite5.C @@ -12,4 +12,4 @@ struct A { A<const int> a; A<int> b; -auto c = (a == b); +auto c = (a == b); // { dg-error "ambiguous, even though the second is reversed" "" { target c++20 } } diff --git a/gcc/testsuite/g++.old-deja/g++.jason/byval2.C b/gcc/testsuite/g++.old-deja/g++.jason/byval2.C index 40bf2a36528..0575109ed1a 100644 --- a/gcc/testsuite/g++.old-deja/g++.jason/byval2.C +++ b/gcc/testsuite/g++.old-deja/g++.jason/byval2.C @@ -18,6 +18,6 @@ inline char operator == (const Char a, const char b) { return 0; } char mystrcmp(Char s[31], Char t[31]) { - for (; *s == *t; ++s, ++t) if (*s == '\0') return 0; + for (; *s == *t; ++s, ++t) if (*s == '\0') return 0; // { dg-error "reversed" "" { target c++20 } } return char(*s - *t); } diff --git a/gcc/testsuite/g++.old-deja/g++.other/overload13.C b/gcc/testsuite/g++.old-deja/g++.other/overload13.C index 54ab404af11..f59bd4a49c3 100644 --- a/gcc/testsuite/g++.old-deja/g++.other/overload13.C +++ b/gcc/testsuite/g++.old-deja/g++.other/overload13.C @@ -2,7 +2,7 @@ // Origin: Nathan Sidwell <nathan@codesourcery.com> struct A { - bool operator== (A const &); + bool operator== (A const &) const; operator bool () const; operator int * () const; };
Tested x86_64-pc-linux-gnu. Jonathan, what do you want to do about the library test failure? -- >8 -- This paper is resolving the problem of well-formed C++17 code becoming ambiguous in C++20 due to asymmetrical operator== being compared with itself in reverse. I had previously implemented a tiebreaker such that if the two candidates were functions with the same parameter types, we would prefer the non-reversed candidate. But the committee went with a different approach: if there's an operator!= with the same parameter types as the operator==, don't consider the reversed form of the ==. So this patch implements that, and changes my old tiebreaker to give a pedwarn if it is used. I also noticed that we were giving duplicate errors for some testcases, and fixed the tourney logic to avoid that. As a result, a lot of tests of the form struct A { bool operator==(const A&); }; need to be fixed to add a const function-cv-qualifier, e.g. struct A { bool operator==(const A&) const; }; The committee thought such code ought to be fixed, so breaking it was fine. 18_support/comparisons/algorithms/fallback.cc also breaks with this patch, because of the similarly asymmetrical bool operator==(const S&, S&) { return true; } I assume this was written this way deliberately, so I'm not sure what to do about it. The build_new_op and tsubst_copy_and_build hunks fix issues with the concept failure diagnostics on that testcase. The H test in spaceship-eq15.C is specified in the standard to be well-formed because the op!= in the inline namespace is not found by the search, but that seems wrong to me. I've implemented that behavior, but disabled it for now; if we decide that is the way we want to go, we can just remove the "0 &&" in add_candidates to enable it. Co-authored-by: Jakub Jelinek <jakub@redhat.com> gcc/cp/ChangeLog: * cp-tree.h (fns_correspond): Declare. * decl.cc (fns_correspond): New. * call.cc (add_candidates): Look for op!= matching op==. (joust): Complain about non-standard reversed tiebreaker. (tourney): Fix champ_compared_to_predecessor logic. (build_new_op): Don't complain about error_mark_node not having 'bool' type. * pt.cc (tsubst_copy_and_build): Don't try to be permissive when seen_error(). gcc/testsuite/ChangeLog: * g++.dg/cpp2a/spaceship-eq15.C: New test. * g++.dg/cpp0x/defaulted3.C: Add const. * g++.dg/cpp2a/bit-cast7.C: Add const. * g++.dg/cpp2a/spaceship-rewrite1.C: Expect error. * g++.dg/cpp2a/spaceship-rewrite5.C: Expect error. * g++.old-deja/g++.jason/byval2.C: Expect error. * g++.old-deja/g++.other/overload13.C: Add const. --- gcc/cp/cp-tree.h | 1 + gcc/cp/call.cc | 109 ++++++++- gcc/cp/decl.cc | 66 ++++++ gcc/cp/pt.cc | 5 +- gcc/testsuite/g++.dg/cpp0x/defaulted3.C | 2 +- gcc/testsuite/g++.dg/cpp2a/bit-cast7.C | 4 +- gcc/testsuite/g++.dg/cpp2a/spaceship-eq15.C | 208 ++++++++++++++++++ .../g++.dg/cpp2a/spaceship-rewrite1.C | 2 +- .../g++.dg/cpp2a/spaceship-rewrite5.C | 2 +- gcc/testsuite/g++.old-deja/g++.jason/byval2.C | 2 +- .../g++.old-deja/g++.other/overload13.C | 2 +- 11 files changed, 386 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-eq15.C base-commit: 071d00e0faabbd45449d2e83f207fca0f8e8ef68