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 19 regression matching constrained template to unconstrained template template parameter #104189

Closed
CaseyCarter opened this issue Aug 14, 2024 · 9 comments
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts regression:19 Regression in 19 release

Comments

@CaseyCarter
Copy link
Member

Compiling this program fragment:

template <template <class, class...> class>
constexpr bool f() { return true; }

template <class> class unconstrained {};
static_assert(f<unconstrained>()); // Fine

template <class> concept True = true;
template <True> class constrained {};
static_assert(f<constrained>()); // error: too many template arguments for class template 'constrained'

with clang 19.1.0-rc2 under -std=c++20 diagnoses:

repro.cpp:9:17: error: too many template arguments for class template 'constrained'
    1 | template <template <class, class...> class>
      |                                    ~
    2 | constexpr bool f() { return true; }
    3 |
    4 | template <class> class unconstrained {};
    5 | static_assert(f<unconstrained>()); // Fine
    6 |
    7 | template <class> concept True = true;
    8 | template <True> class constrained {};
    9 | static_assert(f<constrained>()); // error: too many template arguments for class template 'constrained'
      |                 ^
1 error generated.

The constrained case should be as well-formed as the unconstrained case since [temp.arg.template]/3 says the constraints on the argument should be ignored when the template template parameter is not constrained. Note that the release versions of the "big three" (including Clang 18) compile this fragment without issue (https://godbolt.org/z/qYr8orqz7).

@CaseyCarter CaseyCarter added clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts regression:19 Regression in 19 release labels Aug 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 14, 2024

@llvm/issue-subscribers-clang-frontend

Author: Casey Carter (CaseyCarter)

Compiling this program fragment: ```c++ template <template <class, class...> class> constexpr bool f() { return true; }

template <class> class unconstrained {};
static_assert(f<unconstrained>()); // Fine

template <class> concept True = true;
template <True> class constrained {};
static_assert(f<constrained>()); // error: too many template arguments for class template 'constrained'

with clang 19.1.0-rc2 under `-std=c++20` diagnoses:

repro.cpp:9:17: error: too many template arguments for class template 'constrained'
1 | template <template <class, class...> class>
| ~
2 | constexpr bool f() { return true; }
3 |
4 | template <class> class unconstrained {};
5 | static_assert(f<unconstrained>()); // Fine
6 |
7 | template <class> concept True = true;
8 | template <True> class constrained {};
9 | static_assert(f<constrained>()); // error: too many template arguments for class template 'constrained'
| ^
1 error generated.

The `constrained` case should be as well-formed as the `unconstrained` case since [temp.arg.template]/3 says the constraints on the argument should be ignored when the template template parameter is not constrained. Note that the release versions of the "big three" (including Clang 18) compile this fragment without issue (https://godbolt.org/z/qYr8orqz7).

</details>

@cor3ntin
Copy link
Contributor

@mizvekov

@mizvekov
Copy link
Contributor

This will be fixed by #96023

That might be too big of a change to backport though.

I am looking for which commit introduced the regression.

@mizvekov
Copy link
Contributor

That was from the enablement of relaxed-template-template args by default: https://godbolt.org/z/ErKj9M3Ga

#96023 is the best fix here.

The other problem is that it has also a lot of merge conflicts with other pending patches, and I put it at the top of other patches that are also taking a while to get reviewed.

@cor3ntin
Copy link
Contributor

@AaronBallman for visibility

@CaseyCarter
Copy link
Member Author

FWIW, this impacts a single MSVCSTL test, and the workaround is to change the template <class, class...> class parameter to template <class...> class. It's not worth heroic effort to backport the fix just for us.

@cor3ntin
Copy link
Contributor

@mizvekov this is not actually fixed fyi

@mizvekov
Copy link
Contributor

Yeah, the patch was reverted a while ago, needs a fixup and reapplying.

@mizvekov
Copy link
Contributor

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" concepts C++20 concepts regression:19 Regression in 19 release
Projects
None yet
Development

No branches or pull requests

4 participants