https://github.com/usx95 created https://github.com/llvm/llvm-project/pull/69595
https://github.com/llvm/llvm-project/pull/68999 correctly computed conversion sequence for reversed args to a template operators. This was a breaking change as code, previously accepted in C++17, starts to break in C++20. Example: ```cpp struct P {}; template<class S> bool operator==(const P&, const S &); struct A : public P {}; struct B : public P {}; bool check(A a, B b) { return a == b; } // This is now ambiguous in C++20. ``` In order to minimise widespread breakages, as a clang extension, we had previously accepted such ambiguities with a warning (`-Wambiguous-reversed-operator`) for non-template operators. Due to the same reasons, we extend this relaxation for template operators. Fixes https://github.com/llvm/llvm-project/issues/53954 >From 4460b02508d21d98cf05103bece99fc5bd474ab0 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 19 Oct 2023 12:33:45 +0200 Subject: [PATCH 1/2] Reapply "Correctly compute conversion seq for args to fn with reversed param order (#68999)" This reverts commit a3a0f59a1e1cb0ac02f06b19f730ea05a6541c96. --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/Sema/SemaOverload.cpp | 2 +- .../over.match.oper/p3-2a.cpp | 35 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index eee48431d716878..f6ad453cb17f9d9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -117,6 +117,8 @@ C++ Language Changes C++20 Feature Support ^^^^^^^^^^^^^^^^^^^^^ +- Fix a bug in conversion sequence of arguments to a function with reversed parameter order. + Fixes `GH <https://github.com/llvm/llvm-project/issues/53954>`_. C++23 Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index ce78994e6553814..c271cebb9eb638f 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -7688,7 +7688,7 @@ bool Sema::CheckNonDependentConversions( QualType ParamType = ParamTypes[I + Offset]; if (!ParamType->isDependentType()) { unsigned ConvIdx = PO == OverloadCandidateParamOrder::Reversed - ? 0 + ? Args.size() - 1 - (ThisConversions + I) : (ThisConversions + I); Conversions[ConvIdx] = TryCopyInitialization(*this, Args[I], ParamType, diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp index 5c6804eb7726b5f..02fe37dc1be5058 100644 --- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp +++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp @@ -324,6 +324,41 @@ bool x = X() == X(); // expected-warning {{ambiguous}} } } // namespace P2468R2 +namespace GH53954{ +namespace test1 { +struct P { + template <class T> + friend bool operator==(const P&, const T&); // expected-note {{candidate}} \ + // expected-note {{reversed parameter order}} +}; +struct A : public P {}; +struct B : public P {}; +bool check(A a, B b) { return a == b; } // expected-error {{ '==' is ambiguous}} +} + +namespace test2 { +struct P { + template <class T> + friend bool operator==(const T&, const P&); // expected-note {{candidate}} \ + // expected-note {{reversed parameter order}} +}; +struct A : public P {}; +struct B : public P {}; +bool check(A a, B b) { return a == b; } // expected-error {{ '==' is ambiguous}} +} + +namespace test3 { +struct P { + template<class S> + bool operator==(const S &) const; // expected-note {{candidate}} \ + // expected-note {{reversed parameter order}} +}; +struct A : public P {}; +struct B : public P {}; +bool check(A a, B b) { return a == b; } // expected-error {{ '==' is ambiguous}} +} +} + #else // NO_ERRORS namespace problem_cases { >From 12f5b5e43e51fbd966fc99d82c5d04019cfa3adf Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 19 Oct 2023 14:02:24 +0200 Subject: [PATCH 2/2] Do not hard error for ambiguity with reversed arguments for templated operators --- clang/docs/ReleaseNotes.rst | 18 ++++++++- clang/lib/Sema/SemaOverload.cpp | 10 +++-- .../over.match.oper/p3-2a.cpp | 40 ++++++++++++++----- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index f6ad453cb17f9d9..a38cd74b6165c84 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -37,6 +37,22 @@ These changes are ones which we think may surprise users when upgrading to Clang |release| because of the opportunity they pose for disruption to existing code bases. +- Fix a bug in reversed argument for templated operators. + This breaks code in C++20 which was previously accepted in C++17. Eg: + ``` + struct P {}; + template<class S> bool operator==(const P&, const S &); + + struct A : public P {}; + struct B : public P {}; + // This equality is now ambiguous in C++20. + bool check(A a, B b) { return a == b; } + ``` + To reduce widespread breakages, as an extension, clang would accept this with a + `-Wambiguous-reversed-operator` warning. + Fixes `GH <https://github.com/llvm/llvm-project/issues/53954>`_. + + C/C++ Language Potentially Breaking Changes ------------------------------------------- @@ -117,8 +133,6 @@ C++ Language Changes C++20 Feature Support ^^^^^^^^^^^^^^^^^^^^^ -- Fix a bug in conversion sequence of arguments to a function with reversed parameter order. - Fixes `GH <https://github.com/llvm/llvm-project/issues/53954>`_. C++23 Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index c271cebb9eb638f..53de7cb783a6ced 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -10085,10 +10085,14 @@ getImplicitObjectParamType(ASTContext &Context, const FunctionDecl *F) { return M->getFunctionObjectParameterReferenceType(); } -static bool haveSameParameterTypes(ASTContext &Context, const FunctionDecl *F1, +static bool allowAmbiguityWithSelf(ASTContext &Context, const FunctionDecl *F1, const FunctionDecl *F2) { if (declaresSameEntity(F1, F2)) return true; + if (F1->isTemplateInstantiation() && F2->isTemplateInstantiation() && + declaresSameEntity(F1->getPrimaryTemplate(), F2->getPrimaryTemplate())) { + return true; + } auto NextParam = [&](const FunctionDecl *F, unsigned &I, bool First) { if (First) { @@ -10274,7 +10278,7 @@ bool clang::isBetterOverloadCandidate( case ImplicitConversionSequence::Worse: if (Cand1.Function && Cand2.Function && Cand1.isReversed() != Cand2.isReversed() && - haveSameParameterTypes(S.Context, Cand1.Function, Cand2.Function)) { + allowAmbiguityWithSelf(S.Context, Cand1.Function, Cand2.Function)) { // Work around large-scale breakage caused by considering reversed // forms of operator== in C++20: // @@ -14421,7 +14425,7 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc, llvm::SmallVector<FunctionDecl*, 4> AmbiguousWith; for (OverloadCandidate &Cand : CandidateSet) { if (Cand.Viable && Cand.Function && Cand.isReversed() && - haveSameParameterTypes(Context, Cand.Function, FnDecl)) { + allowAmbiguityWithSelf(Context, Cand.Function, FnDecl)) { for (unsigned ArgIdx = 0; ArgIdx < 2; ++ArgIdx) { if (CompareImplicitConversionSequences( *this, OpLoc, Cand.Conversions[ArgIdx], diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp index 02fe37dc1be5058..e075580828f42e5 100644 --- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp +++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp @@ -325,38 +325,60 @@ bool x = X() == X(); // expected-warning {{ambiguous}} } // namespace P2468R2 namespace GH53954{ -namespace test1 { +namespace friend_template_1 { struct P { template <class T> friend bool operator==(const P&, const T&); // expected-note {{candidate}} \ - // expected-note {{reversed parameter order}} + // expected-note {{ambiguous candidate function with reversed arguments}} }; struct A : public P {}; struct B : public P {}; -bool check(A a, B b) { return a == b; } // expected-error {{ '==' is ambiguous}} +bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}} } -namespace test2 { +namespace friend_template_2 { struct P { template <class T> friend bool operator==(const T&, const P&); // expected-note {{candidate}} \ - // expected-note {{reversed parameter order}} + // expected-note {{ambiguous candidate function with reversed arguments}} }; struct A : public P {}; struct B : public P {}; -bool check(A a, B b) { return a == b; } // expected-error {{ '==' is ambiguous}} +bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}} } -namespace test3 { +namespace member_template { struct P { template<class S> bool operator==(const S &) const; // expected-note {{candidate}} \ - // expected-note {{reversed parameter order}} + // expected-note {{ambiguous candidate function with reversed arguments}} }; struct A : public P {}; struct B : public P {}; -bool check(A a, B b) { return a == b; } // expected-error {{ '==' is ambiguous}} +bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}} } + +namespace non_member_template_1 { +struct P {}; +template<class S> +bool operator==(const P&, const S &); // expected-note {{candidate}} \ + // expected-note {{ambiguous candidate function with reversed arguments}} + +struct A : public P {}; +struct B : public P {}; +bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}} +} +} + +namespace non_member_template_2 { +struct P {}; +template<class S> +bool operator==(const S&, const P&); // expected-note {{candidate}} \ + // expected-note {{ambiguous candidate function with reversed arguments}} + +struct A : public P {}; +struct B : public P {}; +bool check(A a, B b) { return a == b; } // expected-warning {{use of overloaded operator '==' (with operand types 'A' and 'B') to be ambiguous}} } #else // NO_ERRORS _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
