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

clang incorrectly considers top-level cv-qualifiers when determining the parameter mapping for subsumption #75404

Open
CaseyCarter opened this issue Dec 13, 2023 · 8 comments · Fixed by #81449
Labels
bug Indicates an unexpected problem or unintended behavior c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts

Comments

@CaseyCarter
Copy link
Member

Clang considers overload resolution for f(42) in this program to be ambiguous:

template <class> concept True = true;
template <class T> constexpr bool f(const T) { return false;} 
template <True T> constexpr bool f(T) { return true; } 
static_assert(f(42));

which I believe is (or at least should be - the wording around parameter mappings is a bit sparse) non-conforming. Top-level cv-qualifiers on function parameters in C++ should have no effect outside the body of the function definition. This behavior notably diverges from GCC and MSVC (https://godbolt.org/z/jhvnsf8nf).

@CaseyCarter CaseyCarter added bug Indicates an unexpected problem or unintended behavior clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts labels Dec 13, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2023

@llvm/issue-subscribers-clang-frontend

Author: Casey Carter (CaseyCarter)

Clang considers overload resolution for `f(42)` in this program to be ambiguous: ```c++ template <class> concept True = true; template <class T> constexpr bool f(const T) { return false;} template <True T> constexpr bool f(T) { return true; } static_assert(f(42)); ``` which I believe is (or at least should be - the wording around parameter mappings is a bit sparse) non-conforming. Top-level cv-qualifiers on function parameters in C++ should have no effect outside the body of the function definition. This behavior notably diverges from GCC and MSVC (https://godbolt.org/z/jhvnsf8nf).

@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2023

@llvm/issue-subscribers-bug

Author: Casey Carter (CaseyCarter)

Clang considers overload resolution for `f(42)` in this program to be ambiguous: ```c++ template <class> concept True = true; template <class T> constexpr bool f(const T) { return false;} template <True T> constexpr bool f(T) { return true; } static_assert(f(42)); ``` which I believe is (or at least should be - the wording around parameter mappings is a bit sparse) non-conforming. Top-level cv-qualifiers on function parameters in C++ should have no effect outside the body of the function definition. This behavior notably diverges from GCC and MSVC (https://godbolt.org/z/jhvnsf8nf).

@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2023

@llvm/issue-subscribers-c-20

Author: Casey Carter (CaseyCarter)

Clang considers overload resolution for `f(42)` in this program to be ambiguous: ```c++ template <class> concept True = true; template <class T> constexpr bool f(const T) { return false;} template <True T> constexpr bool f(T) { return true; } static_assert(f(42)); ``` which I believe is (or at least should be - the wording around parameter mappings is a bit sparse) non-conforming. Top-level cv-qualifiers on function parameters in C++ should have no effect outside the body of the function definition. This behavior notably diverges from GCC and MSVC (https://godbolt.org/z/jhvnsf8nf).

@zyn0217
Copy link
Contributor

zyn0217 commented Feb 11, 2024

for (unsigned i = 0; i < NumParams1; ++i)
if (!Context.hasSameType(FD1->getParamDecl(i)->getType(),
FD2->getParamDecl(i)->getType()))
return nullptr;

Perhaps this should be Context.hasSameUnqualifiedType.

zyn0217 added a commit to zyn0217/llvm-project that referenced this issue Feb 12, 2024
…ial orderings

This fixes a regression since
llvm@340eac0,
from which we compared function parameter types with cv-qualifiers
taken into account. However, as per [dcl.fct]/p5:

> After producing the list of parameter types, any top-level cv-qualifiers modifying
> a parameter type are deleted when forming the function type.

Thus I think we should use hasSameUnqualifiedType for type comparison.

This fixes llvm#75404.
@CaseyCarter
Copy link
Member Author

For posterity: CWG 1001 says that I'm wrong about this particular case, and that top-level const is significant when applied to a parameter of dependent type.

@zyn0217
Copy link
Contributor

zyn0217 commented Mar 14, 2024

Thank you for following up. I'll reopen the issue and see if I can fix that later.

@zyn0217 zyn0217 reopened this Mar 14, 2024
@zyn0217 zyn0217 removed their assignment Mar 14, 2024
@zyn0217
Copy link
Contributor

zyn0217 commented Mar 14, 2024

(Considering that CWG 1001 has been open for over ~15 years, I think we should probably not revert PR #81449 until that issue gets fully resolved.)

@zyn0217 zyn0217 reopened this May 31, 2024
@ken-matsui
Copy link
Member

Oops, sorry about this. My private fork accidentally closed this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts
Projects
None yet
5 participants