https://github.com/usx95 created https://github.com/llvm/llvm-project/pull/70216
The namespace lookup should give a matching friend operator!= as friend functions belong to the namespace scope (as opposed to the lexical class scope). Thus to resolve ambiguity of operator== with itself, defining a corresponding friend operator!= should suffice. Fixes: https://github.com/llvm/llvm-project/issues/70210 >From 8aa439a97a56ef80bfc9ccc90a9f093680e455f5 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Sun, 22 Oct 2023 11:11:53 +0200 Subject: [PATCH 1/4] rebase... --- clang/docs/ReleaseNotes.rst | 4 + clang/lib/Sema/SemaOverload.cpp | 19 ++--- .../over.match.oper/p3-2a.cpp | 78 +++++++++++++++++++ 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3a9a67f7451c092..cef857244387e8b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -85,6 +85,10 @@ C/C++ Language Potentially Breaking Changes ``__has_c_attribute(warn_unused_result)``, 202003, 0 ``__has_c_attribute(gnu::warn_unused_result)``, 202003, 1 +- Fixed a bug in finding matching `operator!=` while adding reversed `operator==` as + outlined in "The Equality Operator You Are Looking For" (`P2468 <http://wg21.link/p2468r2>`_). + Fixes (`#68901: <https://github.com/llvm/llvm-project/issues/68901>`_). + C++ Specific Potentially Breaking Changes ----------------------------------------- - The name mangling rules for function templates has been changed to take into diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index d57a7ad8f46859a..75e4184c8aa6cb8 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -960,18 +960,13 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc, return true; } // Otherwise the search scope is the namespace scope of which F is a member. - LookupResult NonMembers(S, NotEqOp, OpLoc, - Sema::LookupNameKind::LookupOperatorName); - S.LookupName(NonMembers, - S.getScopeForContext(EqFD->getEnclosingNamespaceContext())); - NonMembers.suppressAccessDiagnostics(); - for (NamedDecl *Op : NonMembers) { - auto *FD = Op->getAsFunction(); - if(auto* UD = dyn_cast<UsingShadowDecl>(Op)) - FD = UD->getUnderlyingDecl()->getAsFunction(); - if (FunctionsCorrespond(S.Context, EqFD, FD) && - declaresSameEntity(cast<Decl>(EqFD->getDeclContext()), - cast<Decl>(Op->getDeclContext()))) + for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) { + auto *NotEqFD = Op->getAsFunction(); + if (auto *UD = dyn_cast<UsingShadowDecl>(Op)) + NotEqFD = UD->getUnderlyingDecl()->getAsFunction(); + if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) && + declaresSameEntity(cast<Decl>(EqFD->getEnclosingNamespaceContext()), + cast<Decl>(Op->getLexicalDeclContext()))) return false; } return true; 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 5727d506fe5e61d..9dc5ee8db565341 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 @@ -374,6 +374,84 @@ bool fine(A a, B b) { return a == b; } // Ok. Found a matching operator!=. } } + +namespace ADL_GH68901{ +namespace A { +struct S {}; +bool operator==(S, int); // expected-note {{no known conversion from 'int' to 'S' for 1st argument}} +bool operator!=(S, int); +} // namespace A +bool a = 0 == A::S(); // expected-error {{invalid operands to binary expression ('int' and 'A::S')}} + +namespace B { +struct Derived {}; +struct Base : Derived {}; + +bool operator==(Derived& a, Base& b); +bool operator!=(Derived& a, Base& b); +} // namespace B + +void foo() { + // B::Base{} == B::Base{}; + B::Base a,b; + bool v = a == b; +} + +namespace ns { +template <class T> +struct A { +}; + +template <class T> +struct B : A<T> { +}; + +template <class T> bool operator == (B<T>, A<T>); // expected-note {{candidate template ignored: could not match 'B' against 'A'}} +template <class T> bool operator != (B<T>, A<T>); +} + +void test() { + ns::A<int> a; + ns::B<int> b; + a == b; // expected-error {{invalid operands to binary expression}} +} + + +} //namespace ADL_GH68901 + +namespace function_scope_operator_eqeq { +// For non-members, we always lookup for matching operator!= in the namespace scope of +// operator== (and not in the scope of operator==). +struct X { operator int(); }; +namespace test1{ +bool h(X x) { + bool operator==(X, int); // expected-note {{reversed}} + return x == x; // expected-warning {{ambiguous}} +} + +bool g(X x) { + bool operator==(X, int); // expected-note {{reversed}} + bool operator!=(X, int); + return x == x; // expected-warning {{ambiguous}} +} +} // namespace test1 + +namespace test2 { +bool operator!=(X, int); + +bool h(X x) { + bool operator==(X, int); + return x == x; +} + +bool i(X x) { + bool operator==(X, int); + bool operator!=(X, int); + return x == x; +} +} // namespace test2 +} // namespace function_scope_operator_eqeq + namespace non_member_template_2 { struct P {}; template<class S> >From df6f585d8cd1dbe328733ecb1dde164a312d2f9f Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Sun, 22 Oct 2023 21:27:17 +0200 Subject: [PATCH 2/4] added more test and visibility check. --- clang/lib/Sema/SemaOverload.cpp | 2 +- .../over.match.oper/p3-2a.cpp | 49 ++++++++++++++----- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 75e4184c8aa6cb8..070e4218ea3afcd 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -964,7 +964,7 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc, auto *NotEqFD = Op->getAsFunction(); if (auto *UD = dyn_cast<UsingShadowDecl>(Op)) NotEqFD = UD->getUnderlyingDecl()->getAsFunction(); - if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) && + if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) && S.isVisible(NotEqFD) && declaresSameEntity(cast<Decl>(EqFD->getEnclosingNamespaceContext()), cast<Decl>(Op->getLexicalDeclContext()))) return false; 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 9dc5ee8db565341..016eaf7f52876d5 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 @@ -376,13 +376,17 @@ bool fine(A a, B b) { return a == b; } // Ok. Found a matching operator!=. namespace ADL_GH68901{ +namespace test1 { namespace A { struct S {}; bool operator==(S, int); // expected-note {{no known conversion from 'int' to 'S' for 1st argument}} +bool a = 0 == A::S(); // Ok. Operator!= not visible. bool operator!=(S, int); } // namespace A bool a = 0 == A::S(); // expected-error {{invalid operands to binary expression ('int' and 'A::S')}} +} // namespace test1 +namespace test2 { namespace B { struct Derived {}; struct Base : Derived {}; @@ -391,23 +395,20 @@ bool operator==(Derived& a, Base& b); bool operator!=(Derived& a, Base& b); } // namespace B -void foo() { - // B::Base{} == B::Base{}; +bool foo() { B::Base a,b; - bool v = a == b; + return a == b; } +} // namespace test2 -namespace ns { -template <class T> -struct A { -}; -template <class T> -struct B : A<T> { -}; +namespace template_ { +namespace ns { +template <class T> struct A {}; +template <class T> struct B : A<T> {}; -template <class T> bool operator == (B<T>, A<T>); // expected-note {{candidate template ignored: could not match 'B' against 'A'}} -template <class T> bool operator != (B<T>, A<T>); +template <class T> bool operator==(B<T>, A<T>); // expected-note {{candidate template ignored: could not match 'B' against 'A'}} +template <class T> bool operator!=(B<T>, A<T>); } void test() { @@ -415,7 +416,31 @@ void test() { ns::B<int> b; a == b; // expected-error {{invalid operands to binary expression}} } +} // namespace test3 + +namespace using_not_eq { +namespace A { +struct S {}; +namespace B { +bool operator!=(S, int); +} +bool operator==(S, int); // expected-note {{candidate}} +using B::operator!=; +} // namespace A +bool a = 0 == A::S(); // expected-error {{invalid operands to binary expression}} +} // namespace reversed_lookup_not_like_ADL +namespace using_eqeq { +namespace A { +struct S {}; +namespace B { +bool operator==(S, int); // expected-note {{candidate}} +bool operator!=(S, int); +} +using B::operator==; +} // namespace A +bool a = 0 == A::S(); // expected-error {{invalid operands to binary expression}} +} } //namespace ADL_GH68901 >From 24e874ca5e6f1b4ac58068cbfb68cbb48fd5e2e3 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 23 Oct 2023 14:04:42 +0200 Subject: [PATCH 3/4] added test for visibility --- .../over.match.oper/p2468R2.cppm | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p2468R2.cppm diff --git a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p2468R2.cppm b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p2468R2.cppm new file mode 100644 index 000000000000000..05891618991da07 --- /dev/null +++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p2468R2.cppm @@ -0,0 +1,24 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/p2468r2.cpp -verify + +//--- A.cppm +module; +export module A; +export { +namespace NS { +struct S {}; +bool operator==(S, int); +} // namespace NS +} + +namespace NS { bool operator!=(S, int); } // Not visible. + + +//--- p2468r2.cpp +// expected-no-diagnostics +import A; +bool x = 0 == NS::S(); // Ok. operator!= from module A is not visible. >From 183209f71b08a1fdba335d89c906cd26483115c1 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 25 Oct 2023 16:32:52 +0200 Subject: [PATCH 4/4] friend operator!= --- clang/lib/Sema/SemaOverload.cpp | 18 ++++++++++----- .../over.match.oper/p3-2a.cpp | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 070e4218ea3afcd..30bde49636b2f69 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -14,6 +14,7 @@ #include "clang/AST/ASTLambda.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclFriend.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DependenceFlags.h" #include "clang/AST/Expr.h" @@ -960,13 +961,16 @@ static bool shouldAddReversedEqEq(Sema &S, SourceLocation OpLoc, return true; } // Otherwise the search scope is the namespace scope of which F is a member. - for (NamedDecl *Op : EqFD->getEnclosingNamespaceContext()->lookup(NotEqOp)) { + DeclContext *EqDC = EqFD->getEnclosingNamespaceContext(); + for (NamedDecl *Op : EqDC->lookup(NotEqOp)) { auto *NotEqFD = Op->getAsFunction(); + DeclContext *NotEqDC = Op->getFriendObjectKind() + ? NotEqFD->getEnclosingNamespaceContext() + : Op->getLexicalDeclContext(); if (auto *UD = dyn_cast<UsingShadowDecl>(Op)) NotEqFD = UD->getUnderlyingDecl()->getAsFunction(); if (FunctionsCorrespond(S.Context, EqFD, NotEqFD) && S.isVisible(NotEqFD) && - declaresSameEntity(cast<Decl>(EqFD->getEnclosingNamespaceContext()), - cast<Decl>(Op->getLexicalDeclContext()))) + declaresSameEntity(cast<Decl>(EqDC), cast<Decl>(NotEqDC))) return false; } return true; @@ -10086,9 +10090,11 @@ static bool allowAmbiguity(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; + if (F1->isTemplateInstantiation() && F2->isTemplateInstantiation()) { + FunctionTemplateDecl *P1 = F1->getPrimaryTemplate(); + FunctionTemplateDecl *P2 = F2->getPrimaryTemplate(); + if (declaresSameEntity(P1, P2)) + return true; } // TODO: It is not clear whether comparing parameters is necessary (i.e. // different functions with same params). Consider removing this (as no test 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 016eaf7f52876d5..2292822e230dde6 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 @@ -374,6 +374,28 @@ bool fine(A a, B b) { return a == b; } // Ok. Found a matching operator!=. } } +namespace friend_opNE_GH{ +namespace test1 { +struct S { + operator int(); + friend bool operator==(const S &, int); // expected-note {{reversed}} +}; +struct A : S {}; +struct B : S {}; +bool x = A{} == B{}; // expected-warning {{ambiguous}} +} + +namespace test2 { +struct S { + operator int(); + friend bool operator==(const S &, int); + friend bool operator!=(const S &, int); +}; +struct A : S {}; +struct B : S {}; +bool x = A{} == B{}; +} +} namespace ADL_GH68901{ namespace test1 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
