[clang] check constant template parameters in dependent contexts #159463

mizvekov · 2025-09-17T21:29:43Z

This patch makes sure constant template parameters are checked even in dependent contexts.

This can for example diagnose narrowings earlier, but this is permitted as these templates would have no valid instantiations.

llvmbot · 2025-09-17T21:30:16Z

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This patch makes sure constant template parameters are checked even in dependent contexts.

This can for example diagnose narrowings earlier, but this is permitted as these templates would have no valid instantiations.

Full diff: https://github.com/llvm/llvm-project/pull/159463.diff

3 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2-1)
(modified) clang/lib/Sema/SemaTemplate.cpp (+2-3)
(modified) clang/test/SemaTemplate/temp_arg_template.cpp (+7-9)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 51e5973098c14..0eadc1c773597 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -241,7 +241,8 @@ Improvements to Clang's diagnostics
 - Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
   parenthesis when diagnosing malformed fold expressions. (#GH151787)
 - Added fix-it hint for when scoped enumerations require explicit conversions for binary operations. (#GH24265)
-
+- Constant template parameters are now type checked in template definitions,
+  including template template parameters.
 - Fixed an issue where emitted format-signedness diagnostics were not associated with an appropriate
   diagnostic id. Besides being incorrect from an API standpoint, this was user visible, e.g.:
   "format specifies type 'unsigned int' but the argument has type 'int' [-Wformat]"
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index d6b25c2d83613..faeba6ada0c6b 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5482,9 +5482,7 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
     if (NTTP->isParameterPack() && NTTP->isExpandedParameterPack())
       NTTPType = NTTP->getExpansionType(ArgumentPackIndex);
 
-    if (NTTPType->isInstantiationDependentType() &&
-        !isa<TemplateTemplateParmDecl>(Template) &&
-        !Template->getDeclContext()->isDependentContext()) {
+    if (NTTPType->isInstantiationDependentType()) {
       // Do substitution on the type of the non-type template parameter.
       InstantiatingTemplate Inst(*this, TemplateLoc, Template, NTTP,
                                  CTAI.SugaredConverted,
@@ -5494,6 +5492,7 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
 
       MultiLevelTemplateArgumentList MLTAL(Template, CTAI.SugaredConverted,
                                            /*Final=*/true);
+      MLTAL.addOuterRetainedLevels(NTTP->getDepth());
       // If the parameter is a pack expansion, expand this slice of the pack.
       if (auto *PET = NTTPType->getAs<PackExpansionType>()) {
         Sema::ArgPackSubstIndexRAII SubstIndex(*this, ArgumentPackIndex);
diff --git a/clang/test/SemaTemplate/temp_arg_template.cpp b/clang/test/SemaTemplate/temp_arg_template.cpp
index aa53dba652050..763ca629eae3e 100644
--- a/clang/test/SemaTemplate/temp_arg_template.cpp
+++ b/clang/test/SemaTemplate/temp_arg_template.cpp
@@ -117,16 +117,8 @@ namespace CheckDependentNonTypeParamTypes {
       X<int, long, 3> x;
     }
     void h() {
-      // FIXME: If we accept A<B> at all, it's not obvious what should happen
-      // here. While parsing the template, we form
-      //   X<unsigned char, int, (unsigned char)1234>
-      // but in the final instantiation do we get
-      //   B<unsigned char, int, (int)1234>
-      // or
-      //   B<unsigned char, int, (int)(unsigned char)1234>
-      // ?
       X<unsigned char, int, 1234> x;
-      int check[x.value == 1234 ? 1 : -1];
+      // expected-error@-1 {{evaluates to 1234, which cannot be narrowed to type 'unsigned char'}}
     }
   };
 
@@ -143,6 +135,12 @@ namespace CheckDependentNonTypeParamTypes {
     ab.g();
     ab.h();
   }
+
+  template<class> struct C {
+    template<class T, T V> struct D {};
+    using T = D<char, 1234>;
+    // expected-error@-1 {{evaluates to 1234, which cannot be narrowed to type 'char'}}
+  };
 }
 
 namespace PR32185 {

erichkeane

I think this makes sense, it seems to do as described at least. I'm not sure WHAT, but it feels like we could/should do a better job with some demonstrative tests though.

mizvekov · 2025-09-17T21:53:13Z

I think this makes sense, it seems to do as described at least. I'm not sure WHAT, but it feels like we could/should do a better job with some demonstrative tests though.

I am open to suggestions, I can't think of anything which would demonstrate this better or show some interesting corner case not shown in the current test changes already.

erichkeane · 2025-09-17T22:11:24Z

I think this makes sense, it seems to do as described at least. I'm not sure WHAT, but it feels like we could/should do a better job with some demonstrative tests though.

I am open to suggestions, I can't think of anything which would demonstrate this better or show some interesting corner case not shown in the current test changes already.

I couldn't come up with anything either... lets hold this off to give @cor3ntin a chance to take a look (or others?) to see if anyone else has a more demonstrative/canonical example for the tests, else this LGTM.

zyn0217

Thanks, this looks neat

Though I don't have a demonstrative test either...

erichkeane · 2025-09-18T13:23:57Z

Alright, I guess I was dreaming about a better test. Ship it.

This patch makes sure constant template parameters are checked even in dependent contexts. This can for example diagnose narrowings earlier, but this is permitted as these templates would have no valid instantiations.

mizvekov · 2025-09-18T19:35:11Z

Alright, I just thought of a new test case, which my patch doesn't actually change anything.
I'll try to get that one covered in another patch.

erichkeane · 2025-09-19T13:15:02Z

Note @mizvekov : we observed on an internal test (I am going to have someone work on minimizing it) that this causes an assertion. As you can see, it is all in boost, so a reduction might take a while, but was hoping you might see something obvious in advance:

clang++: /proj/build/llvm/Linux_x86_64/clang/lib/AST/ExprClassification.cpp:57: Cl clang::Expr::ClassifyImpl(clang::ASTContext&, clang::SourceLocation*) const: Assertion `isLValue()' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang++ -c -o library/SolverUtils/Core/CouplingFile.o -DSPEC -DNDEBUG -DSPEC_AUTO_SUPPRESS_THREADING -Ithirdparty/zlib-1.3 -DTIXML_USE_STL -DBOOST_ENDIAN_NO_INTRINSICS -DBOOST_DISABLE_THREADS -Ithirdparty/tinyxml-2.6.2 -Ithirdparty/boost-1.84.0/include -Ilibrary/ -Isolvers/ --std=c++17 -fno-fast-math -DSPEC_LP64 library/SolverUtils/Core/CouplingFile.cpp
1.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:226:18: at annotation token
2.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:29:1: parsing namespace 'boost'
3.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:31:1: parsing namespace 'boost::mp11'
4.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:199:1: parsing namespace 'boost::mp11::detail'
5.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:224:57: parsing struct/union/class body 'boost::mp11::detail::mp_fill_impl<L<A...>, V>'
 #0 0x0000000004622358 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x4622358)
 #1 0x000000000461f77c llvm::sys::CleanupOnSignal(unsigned long) (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x461f77c)
 #2 0x000000000456c088 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x000014e6172cf520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #4 0x000014e6173239fc __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x000014e6173239fc __pthread_kill_internal ./nptl/pthread_kill.c:78:10
 #6 0x000014e6173239fc pthread_kill ./nptl/pthread_kill.c:89:10
 #7 0x000014e6172cf476 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #8 0x000014e6172b57f3 abort ./stdlib/abort.c:81:7
 #9 0x000014e6172b571b _nl_load_domain ./intl/loadmsgcat.c:1177:9
#10 0x000014e6172c6e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)
#11 0x000000000811f5c6 clang::Expr::ClassifyImpl(clang::ASTContext&, clang::SourceLocation*) const (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x811f5c6)
#12 0x0000000007a27d90 clang::Sema::DeduceAutoType(clang::TypeLoc, clang::Expr*, clang::QualType&, clang::sema::TemplateDeductionInfo&, bool, bool, clang::TemplateSpecCandidateSet*) (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x7a27d90)```

erichkeane · 2025-09-19T13:20:08Z

Note @mizvekov : we observed on an internal test (I am going to have someone work on minimizing it) that this causes an assertion. As you can see, it is all in boost, so a reduction might take a while, but was hoping you might see something obvious in advance:

clang++: /proj/build/llvm/Linux_x86_64/clang/lib/AST/ExprClassification.cpp:57: Cl clang::Expr::ClassifyImpl(clang::ASTContext&, clang::SourceLocation*) const: Assertion `isLValue()' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang++ -c -o library/SolverUtils/Core/CouplingFile.o -DSPEC -DNDEBUG -DSPEC_AUTO_SUPPRESS_THREADING -Ithirdparty/zlib-1.3 -DTIXML_USE_STL -DBOOST_ENDIAN_NO_INTRINSICS -DBOOST_DISABLE_THREADS -Ithirdparty/tinyxml-2.6.2 -Ithirdparty/boost-1.84.0/include -Ilibrary/ -Isolvers/ --std=c++17 -fno-fast-math -DSPEC_LP64 library/SolverUtils/Core/CouplingFile.cpp
1.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:226:18: at annotation token
2.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:29:1: parsing namespace 'boost'
3.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:31:1: parsing namespace 'boost::mp11'
4.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:199:1: parsing namespace 'boost::mp11::detail'
5.	thirdparty/boost-1.84.0/include/boost/mp11/algorithm.hpp:224:57: parsing struct/union/class body 'boost::mp11::detail::mp_fill_impl<L<A...>, V>'
 #0 0x0000000004622358 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x4622358)
 #1 0x000000000461f77c llvm::sys::CleanupOnSignal(unsigned long) (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x461f77c)
 #2 0x000000000456c088 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x000014e6172cf520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #4 0x000014e6173239fc __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x000014e6173239fc __pthread_kill_internal ./nptl/pthread_kill.c:78:10
 #6 0x000014e6173239fc pthread_kill ./nptl/pthread_kill.c:89:10
 #7 0x000014e6172cf476 gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #8 0x000014e6172b57f3 abort ./stdlib/abort.c:81:7
 #9 0x000014e6172b571b _nl_load_domain ./intl/loadmsgcat.c:1177:9
#10 0x000014e6172c6e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)
#11 0x000000000811f5c6 clang::Expr::ClassifyImpl(clang::ASTContext&, clang::SourceLocation*) const (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x811f5c6)
#12 0x0000000007a27d90 clang::Sema::DeduceAutoType(clang::TypeLoc, clang::Expr*, clang::QualType&, clang::sema::TemplateDeductionInfo&, bool, bool, clang::TemplateSpecCandidateSet*) (/proj/nv/llvm/Linux_x86_64/llvm-6094/bin/clang+++0x7a27d90)```

I actually repro'd off the stack trace :)
https://godbolt.org/z/WGz58WdEd

template<class L, class V> struct mp_fill_impl
{};

template<template<auto...> class L, auto... A, class V> struct mp_fill_impl<L<A...>, V>
{
    using type = L<((void)A, V::value)...>;
};

AS this is a part of Boost/major libraries and is a pretty simple bit of valid code, if you can't get a fix reasonably fast, please consider a revert.

This fixes a regression reported here: #159463 (comment) Since this regression was never released, there are no release notes.

…159819) This fixes a regression reported here: #159463 (comment) Since this regression was never released, there are no release notes.

… operators (#159819) This fixes a regression reported here: llvm/llvm-project#159463 (comment) Since this regression was never released, there are no release notes.

mizvekov requested review from cor3ntin, erichkeane, zygoloid and zyn0217 September 17, 2025 21:29

mizvekov self-assigned this Sep 17, 2025

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 17, 2025

erichkeane approved these changes Sep 17, 2025

View reviewed changes

zyn0217 approved these changes Sep 18, 2025

View reviewed changes

cor3ntin approved these changes Sep 18, 2025

View reviewed changes

[clang] check constant template parameters in dependent contexts

f0e3578

This patch makes sure constant template parameters are checked even in dependent contexts. This can for example diagnose narrowings earlier, but this is permitted as these templates would have no valid instantiations.

mizvekov force-pushed the users/mizvekov/check-NTTP-dependent-context branch from 614e4a7 to f0e3578 Compare September 18, 2025 19:34

mizvekov merged commit 3fe85ca into main Sep 18, 2025
10 checks passed

mizvekov deleted the users/mizvekov/check-NTTP-dependent-context branch September 18, 2025 20:07

mizvekov mentioned this pull request Sep 19, 2025

[clang] fix expression classification for dependent binary operators #159819

Merged

mizvekov added a commit that referenced this pull request Sep 19, 2025

[clang] fix expression classification for dependent binary operators

9216ca7

This fixes a regression reported here: #159463 (comment) Since this regression was never released, there are no release notes.

mizvekov added a commit that referenced this pull request Sep 19, 2025

[clang] fix expression classification for dependent binary operators (#…

bbf657d

…159819) This fixes a regression reported here: #159463 (comment) Since this regression was never released, there are no release notes.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] check constant template parameters in dependent contexts #159463

[clang] check constant template parameters in dependent contexts #159463

Uh oh!

mizvekov commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

erichkeane left a comment

Uh oh!

mizvekov commented Sep 17, 2025

Uh oh!

erichkeane commented Sep 17, 2025

Uh oh!

zyn0217 left a comment

Uh oh!

erichkeane commented Sep 18, 2025

Uh oh!

mizvekov commented Sep 18, 2025

Uh oh!

Uh oh!

erichkeane commented Sep 19, 2025

Uh oh!

erichkeane commented Sep 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[clang] check constant template parameters in dependent contexts #159463

[clang] check constant template parameters in dependent contexts #159463

Uh oh!

Conversation

mizvekov commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

mizvekov commented Sep 17, 2025

Uh oh!

erichkeane commented Sep 17, 2025

Uh oh!

zyn0217 left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane commented Sep 18, 2025

Uh oh!

mizvekov commented Sep 18, 2025

Uh oh!

Uh oh!

erichkeane commented Sep 19, 2025

Uh oh!

erichkeane commented Sep 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants