Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
May 7, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ccab3dc
Handle invalid variable template specialization whose type depends on…
zwuis Apr 6, 2025
c23525e
Recognize things like current instantiation
zwuis Apr 28, 2025
50d4065
Introduce `ASTContext::isSameTemplateArgument`, remove underlines in …
zwuis Apr 29, 2025
f474f05
Merge branch 'main' into fix#51347
zwuis Apr 29, 2025
781d3c6
Promote `isSameTemplateArg`, and use `llvm::APSInt::isSameValue`
zwuis Apr 30, 2025
ca31c14
Merge branch 'main' into fix#51347
zwuis May 6, 2025
f09953f
Merge branch 'main' into fix#51347
zwuis May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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">>;
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Sema/SemaTemplate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
18 changes: 18 additions & 0 deletions clang/test/SemaTemplate/instantiate-var-template.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>'}}
Copy link
Collaborator

@erichkeane erichkeane Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@zwuis zwuis Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator

@erichkeane erichkeane Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>;

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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'}}
}
}