Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++20 reversed operator== not properly suppressed by operator!= in namespace #68901

Closed
jyknight opened this issue Oct 12, 2023 · 8 comments · Fixed by #68922 or llvm/llvm-project-release-prs#778
Assignees
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party release:backport release:merged

Comments

@jyknight
Copy link
Member

jyknight commented Oct 12, 2023

namespace N {
  struct A {};
  bool operator==(A, int);
  bool operator!=(A, int);
}
bool a = 0 == N::A();

This should fail to compile, because the existence of operator!= should suppress the reversed operator==(int, A). However, on Clang it does not.

Moving bool a within the namespace does make it correctly fail to compile, which suggests that name lookup for the operator!= is not being done in the correct scope.

@usx95

@jyknight jyknight added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 12, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 12, 2023

@llvm/issue-subscribers-clang-frontend

Author: James Y Knight (jyknight)

``` namespace N { struct A {}; bool operator==(A, int); bool operator!=(A, int); } bool a = 0 == N::A(); ```

This should fail to compile, because the existence of operator!= should suppress the reversed operator==(int, A). However, on Clang it does not.

Moving bool a within the namespace does make it correctly fail to compile, which suggests that name lookup for the operator!= is not being done in the correct scope.

@usx95

@llvmbot
Copy link
Member

llvmbot commented Oct 12, 2023

@llvm/issue-subscribers-c-20

Author: James Y Knight (jyknight)

``` namespace N { struct A {}; bool operator==(A, int); bool operator!=(A, int); } bool a = 0 == N::A(); ```

This should fail to compile, because the existence of operator!= should suppress the reversed operator==(int, A). However, on Clang it does not.

Moving bool a within the namespace does make it correctly fail to compile, which suggests that name lookup for the operator!= is not being done in the correct scope.

@usx95

@usx95 usx95 self-assigned this Oct 12, 2023
@usx95 usx95 added the confirmed Verified by a second party label Oct 12, 2023
@usx95
Copy link
Contributor

usx95 commented Oct 12, 2023

Same issue but this errors instead:

namespace A {
struct Derived {};
struct Base : Derived {};

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

void foo() {
    A::Base a, b;
    (void)(a == b);
}

https://godbolt.org/z/xvEjrEYre

@usx95 usx95 moved this to In Progress in C++ 20 in Clang Oct 12, 2023
usx95 added a commit that referenced this issue Oct 23, 2023
`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
@github-project-automation github-project-automation bot moved this from In Progress to Done in C++ 20 in Clang Oct 23, 2023
@usx95 usx95 added this to the LLVM 17.0.X Release milestone Nov 15, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Nov 15, 2023
@usx95
Copy link
Contributor

usx95 commented Nov 15, 2023

/cherry-pick 9330261

@usx95 usx95 reopened this Nov 15, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2023

Failed to cherry-pick: 9330261

https://github.com/llvm/llvm-project/actions/runs/6876823292

Please manually backport the fix and push it to your github fork. Once this is done, please add a comment like this:

/branch <user>/<repo>/<branch>

usx95 added a commit to usx95/llvm-project that referenced this issue Nov 15, 2023
)

`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 llvm#68901

(cherry picked from commit 9330261)
@usx95
Copy link
Contributor

usx95 commented Nov 15, 2023

/branch usx95/llvm-project/release/17.x

@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2023

/pull-request llvm/llvm-project-release-prs#778

@usx95
Copy link
Contributor

usx95 commented Nov 15, 2023

/branch usx95/llvm-project/release/17.x

@tru tru moved this from Needs Triage to Needs Review in LLVM Release Status Nov 20, 2023
@tru tru moved this from Needs Review to Done in LLVM Release Status Nov 20, 2023
tru pushed a commit that referenced this issue Nov 20, 2023
`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

(cherry picked from commit 9330261)
AlexisPerry pushed a commit to AlexisPerry/kitsune-1 that referenced this issue Jul 23, 2024
)

`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 llvm#68901

(cherry picked from commit 9330261)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party release:backport release:merged
Projects
Status: Done
4 participants