[Clang][Sema] Handle invalid variable template specialization whose type depends on itself #134522

zwuis · 2025-04-06T07:20:38Z

Fixes #51347
Fixes #55872

… itself

llvmbot · 2025-04-06T07:21:11Z

@llvm/pr-subscribers-clang

Author: Yanzuo Liu (zwuis)

Changes

Fixes #51347

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

4 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
(modified) clang/lib/Sema/SemaTemplate.cpp (+7)
(modified) clang/test/SemaTemplate/instantiate-var-template.cpp (+18)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5217e04b5e83f..9e0fe8a94729b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -414,6 +414,8 @@ Bug Fixes to C++ Support
 - Clang now issues an error when placement new is used to modify a const-qualified variable
   in a ``constexpr`` function. (#GH131432)
 - Clang now emits a warning when class template argument deduction for alias templates is used in C++17. (#GH133806)
+- No longer crashes when instantiating invalid variable template specialization
+  whose type depends on itself. (#GH51347)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 393bfecf9a36b..0c1da40dba388 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5259,6 +5259,9 @@ def err_template_member_noparams : Error<
   "extraneous 'template<>' in declaration of member %0">;
 def err_template_tag_noparams : Error<
   "extraneous 'template<>' in declaration of %0 %1">;
+def err_var_template_spec_type_depends_on_self : Error<
+  "the type of variable template specialization %0 declared with deduced type "
+  "%1 depends on itself">;
 
 def warn_unqualified_call_to_std_cast_function : Warning<
   "unqualified call to '%0'">, InGroup<DiagGroup<"unqualified-std-cast-call">>;
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 153f44f8ec67a..ce54dbbb3b9fe 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4377,6 +4377,13 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
   if (VarTemplateSpecializationDecl *Spec =
           Template->findSpecialization(CTAI.CanonicalConverted, InsertPos)) {
     checkSpecializationReachability(TemplateNameLoc, Spec);
+    if (Spec->getType()->isUndeducedType()) {
+      // We are substituting the initializer of this variable template
+      // specialization.
+      Diag(TemplateNameLoc, diag::err_var_template_spec_type_depends_on_self)
+          << Spec << Spec->getType();
+      return true;
+    }
     // If we already have a variable template specialization, return it.
     return Spec;
   }
diff --git a/clang/test/SemaTemplate/instantiate-var-template.cpp b/clang/test/SemaTemplate/instantiate-var-template.cpp
index 60d3bd3b59f53..34a1b9710814b 100644
--- a/clang/test/SemaTemplate/instantiate-var-template.cpp
+++ b/clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -47,3 +47,21 @@ namespace InvalidInsertPos {
   template<> int v<int, 0>;
   int k = v<int, 500>;
 }
+
+namespace GH51347 {
+  template <typename T>
+  auto p = p<T>; // expected-error {{the type of variable template specialization 'p<int>'}}
+
+  auto x = p<int>; // expected-note {{in instantiation of variable template specialization 'GH51347::p'}}
+}
+
+namespace GH97881_comment {
+  template <bool B>
+  auto g = sizeof(g<!B>);
+  // expected-error@-1 {{the type of variable template specialization 'g<false>'}}
+  // expected-note@-2 {{in instantiation of variable template specialization 'GH97881_comment::g'}}
+
+  void test() {
+    (void)sizeof(g<false>); // expected-note {{in instantiation of variable template specialization 'GH97881_comment::g'}}
+  }
+}

zyn0217

LGTM

erichkeane · 2025-04-07T13:50:17Z

clang/test/SemaTemplate/instantiate-var-template.cpp

+
+namespace GH51347 {
+  template <typename T>
+  auto p = p<T>; // expected-error {{the type of variable template specialization 'p<int>'}}


Are there cases where this pattern is legal and we should allow this anyway? If not, should we diagnose this not at instantiation, but earlier at definition of the variable template?

I think this pattern is always illegal. But it's a big work (compared with this PR) to recognize this pattern (as something like current instantiation).

It should be the same/similar as when we recognize the auto x= x +1 or int x = x + 1; (one is an error, the other is a warning).

This is probably fine if this isn't a primary template, right?

Generally, Clang templates try to diagnose things as absolute early as possible so we get ONE error instead of one-per-instantiation.

That said, it is perhaps worth spending some time to see if there is a reason we can't, or, by language rule, shouldn't diagnose this on auto p = p <T>;

It seems that [temp.res.general]/6 allows us to eagerly diagnose this...

zyn0217

thanks, it looks much better

clang/lib/Sema/SemaTemplate.cpp

zyn0217 · 2025-04-28T16:10:58Z

clang/lib/Sema/SemaTemplate.cpp

+      Diag(TemplateNameLoc,
+           diag::err_auto_variable_cannot_appear_in_own_initializer)
+          << diag::ParsingInitFor::VarTemplate << Var << Var->getType()
+          << SourceRange(TemplateNameLoc, TemplateArgs.getRAngleLoc());


I'm also wondering if it is necessary to draw underlines under the RHS of the declaration - would that be noisy if the declaration is too long?

…diagnostic messages, and add test

clang/lib/AST/ASTContext.cpp

zyn0217

Thanks! This looks nice

@erichkeane are you happy with the current state?

cor3ntin

LGTM

cor3ntin · 2025-05-06T16:38:59Z

@zwuis Do you need me to merge that for you?

zwuis · 2025-05-07T00:15:31Z

Yes. Please help me merge this PR.

Thank you for your review!

…ype depends on itself (llvm#134522)

shafik · 2025-05-09T20:49:01Z

This was flagged in the following issue: #139067 (comment)

… whose type depends on itself (llvm#134522)" This reverts commit 91f1830.

…primary variable template `std::format_kind` with libstdc++ (#139560) #134522 triggers compilation error with libstdc++, in which primary variable template `std::format_kind` is defined like ```cpp template <typename R> constexpr auto format_kind = __primary_template_not_defined( format_kind<R> ); ``` See #139067 or <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120190>. This PR disables checking template id in initializer of primary variable template `std::format_kind` in libstdc++ (by checking `__GLIBCXX__`). Fixes #139067

Handle invalid variable template specialization whose type depends on…

ccab3dc

… itself

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

zyn0217 approved these changes Apr 6, 2025

View reviewed changes

zyn0217 requested review from cor3ntin and erichkeane April 6, 2025 10:27

cor3ntin approved these changes Apr 7, 2025

View reviewed changes

erichkeane reviewed Apr 7, 2025

View reviewed changes

Recognize things like current instantiation

c23525e

zyn0217 reviewed Apr 28, 2025

View reviewed changes

clang/lib/Sema/SemaTemplate.cpp Outdated Show resolved Hide resolved

zyn0217 reviewed Apr 28, 2025

View reviewed changes

zwuis added 2 commits April 29, 2025 23:50

Introduce ASTContext::isSameTemplateArgument, remove underlines in …

50d4065

…diagnostic messages, and add test

Merge branch 'main' into fix#51347

f474f05

zyn0217 reviewed Apr 29, 2025

View reviewed changes

clang/lib/AST/ASTContext.cpp Show resolved Hide resolved

Promote isSameTemplateArg, and use llvm::APSInt::isSameValue

781d3c6

zyn0217 approved these changes May 6, 2025

View reviewed changes

Merge branch 'main' into fix#51347

ca31c14

cor3ntin approved these changes May 6, 2025

View reviewed changes

Merge branch 'main' into fix#51347

f09953f

zyn0217 merged commit 91f1830 into llvm:main May 7, 2025
7 of 11 checks passed

GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025

[Clang][Sema] Handle invalid variable template specialization whose t…

ff9e5dc

…ype depends on itself (llvm#134522)

frederick-vs-ja mentioned this pull request May 9, 2025

[clang] fails to compile print("{}", 5s) when using libstdc++ #139067

Closed

stbergmann mentioned this pull request May 11, 2025

C++23 #include <format> from libstdc++ fails with "variable template ... cannot appear in its own initializer" #139466

Closed

zwuis added a commit to zwuis/llvm-project that referenced this pull request May 12, 2025

Revert "[Clang][Sema] Handle invalid variable template specialization…

4878337

… whose type depends on itself (llvm#134522)" This reverts commit 91f1830.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang][Sema] Handle invalid variable template specialization whose type depends on itself #134522

[Clang][Sema] Handle invalid variable template specialization whose type depends on itself #134522

zwuis commented Apr 6, 2025 •

edited

Loading

llvmbot commented Apr 6, 2025

zyn0217 left a comment

erichkeane Apr 7, 2025

zwuis Apr 8, 2025

erichkeane Apr 8, 2025

frederick-vs-ja May 9, 2025

zyn0217 left a comment

zyn0217 Apr 28, 2025

zyn0217 left a comment

cor3ntin left a comment

cor3ntin commented May 6, 2025

zwuis commented May 7, 2025

shafik commented May 9, 2025

[Clang][Sema] Handle invalid variable template specialization whose type depends on itself #134522

[Clang][Sema] Handle invalid variable template specialization whose type depends on itself #134522

Conversation

zwuis commented Apr 6, 2025 • edited Loading

llvmbot commented Apr 6, 2025

zyn0217 left a comment

Choose a reason for hiding this comment

erichkeane Apr 7, 2025

Choose a reason for hiding this comment

zwuis Apr 8, 2025

Choose a reason for hiding this comment

erichkeane Apr 8, 2025

Choose a reason for hiding this comment

frederick-vs-ja May 9, 2025

Choose a reason for hiding this comment

zyn0217 left a comment

Choose a reason for hiding this comment

zyn0217 Apr 28, 2025

Choose a reason for hiding this comment

zyn0217 left a comment

Choose a reason for hiding this comment

cor3ntin left a comment

Choose a reason for hiding this comment

cor3ntin commented May 6, 2025

zwuis commented May 7, 2025

shafik commented May 9, 2025

zwuis commented Apr 6, 2025 •

edited

Loading