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

incorrect overload resolution with deducing this #74494

Closed
Tsche opened this issue Dec 5, 2023 · 3 comments · Fixed by #83279
Closed

incorrect overload resolution with deducing this #74494

Tsche opened this issue Dec 5, 2023 · 3 comments · Fixed by #83279
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party

Comments

@Tsche
Copy link

Tsche commented Dec 5, 2023

Hey,
@Waffl3x mentioned a possible bug in clang earlier and asked me to report it.

template<typename T>
concept AlwaysTrue = true;

struct S {
  int f(AlwaysTrue auto) { return 1; }; // f1
  int f(this S&&, auto) { return 2; };  // f2
};

int main() {
  return S{}.f(0);
}

https://clang.godbolt.org/z/x516PWsvd

According to [basic.scope.scope]/3 f1 and f2 should have corresponding signatures. Considering [over.match.funcs.general]/5 I believe f1 should be called here since it is more constrained. Clang however calls f2.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Dec 5, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 5, 2023

@llvm/issue-subscribers-clang-frontend

Author: None (Tsche)

Hey, @Waffl3x mentioned a possible bug in clang earlier and asked me to report it.
template&lt;typename T&gt;
concept AlwaysTrue = true;

struct S {
  int f(AlwaysTrue auto) { return 1; }; // f1
  int f(this S&amp;&amp;, auto) { return 2; };  // f2
};

int main() {
  return S{}.f(0);
}

https://clang.godbolt.org/z/x516PWsvd

According to [[basic.scope.scope]/3](https://eel.is/c++draft/basic.scope.scope#3) f1 and f2 should have corresponding signatures. Considering [[over.match.funcs.general]/5](https://eel.is/c++draft/over.match.funcs.general#5.sentence-3) I believe f1 should be called here since it is more constrained. Clang however calls f2.

@shafik
Copy link
Collaborator

shafik commented Dec 6, 2023

I believe this is cwg2789: Overload Resolution with implicit and explicit object member functions which is not available yet but should be soon. I believe clang is wrong here.

@shafik shafik added the confirmed Verified by a second party label Dec 6, 2023
@Waffl3x
Copy link

Waffl3x commented Dec 6, 2023

I believe this is cwg2789: Overload Resolution with implicit and explicit object member functions which is not available yet but should be soon. I believe clang is wrong here.

https://cplusplus.github.io/CWG/issues/2789.html

No, that isn't relevant here, the bullet being changed refers to non-template functions, these are templates.
"F1 and F2 are non-template functions"
The linked issue fixes constraints on non-template member functions, which funny enough already works in clang.
https://godbolt.org/z/fWveeqTqb

whisperity pushed a commit that referenced this issue Mar 6, 2024
There was a bug in Clang where it couldn't choose which overload
candidate was more specialized if it was comparing a member-function to
a non-member function. Previously, this was detected as an ambiguity,
now Clang chooses correctly.

This patch fixes the bug by fully implementing CWG2445 and moving the
template transformation described in `[temp.func.order]` paragraph 3
from `isAtLeastAsSpecializedAs()` to
`Sema::getMoreSpecializedTemplate()` so we have the transformed
parameter list during the whole comparison. Also, to be able to add the
correct type for the implicit object parameter
`Sema::getMoreSpecializedTemplate()` has new parameters for the object
type.

Fixes #74494, fixes #82509
HoBoIs added a commit to HoBoIs/llvm-project that referenced this issue Mar 11, 2024
…#83279)

There was a bug in Clang where it couldn't choose which overload
candidate was more specialized if it was comparing a member-function to
a non-member function. Previously, this was detected as an ambiguity,
now Clang chooses correctly.

This patch fixes the bug by fully implementing CWG2445 and moving the
template transformation described in `[temp.func.order]` paragraph 3
from `isAtLeastAsSpecializedAs()` to
`Sema::getMoreSpecializedTemplate()` so we have the transformed
parameter list during the whole comparison. Also, to be able to add the
correct type for the implicit object parameter
`Sema::getMoreSpecializedTemplate()` has new parameters for the object
type.

Fixes llvm#74494, fixes llvm#82509
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants