Skip to content

Commit

Permalink
Use the correct namespace for looking up matching operator!= (#68922)
Browse files Browse the repository at this point in the history
`S.getScopeForContext` determins the **active** scope associated with
the given `declContext`.
This fails to find the matching `operator!=` if candidate `operator==`
was found via ADL since that scope is not active.

Instead, just directly lookup using the namespace decl of `operator==`

Fixes #68901
  • Loading branch information
usx95 committed Oct 23, 2023
1 parent 596b598 commit 9330261
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 12 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -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
Expand Down
19 changes: 7 additions & 12 deletions clang/lib/Sema/SemaOverload.cpp
Expand Up @@ -962,18 +962,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) && S.isVisible(NotEqFD) &&
declaresSameEntity(cast<Decl>(EqFD->getEnclosingNamespaceContext()),
cast<Decl>(Op->getLexicalDeclContext())))
return false;
}
return true;
Expand Down
@@ -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.
Expand Up @@ -374,6 +374,109 @@ 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 {};

bool operator==(Derived& a, Base& b);
bool operator!=(Derived& a, Base& b);
} // namespace B

bool foo() {
B::Base a,b;
return a == b;
}
} // namespace test2


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>);
}

void test() {
ns::A<int> a;
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

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>
Expand Down

0 comments on commit 9330261

Please sign in to comment.