Skip to content

[clang] fix a crash in error recovery in expressions resolving to templates #135893

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 1 commit into from
Apr 16, 2025
Merged

[clang] fix a crash in error recovery in expressions resolving to templates #135893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ Bug Fixes in This Version
when using the ``INTn_C`` macros. (#GH85995)
- Fixed an assertion failure in the expansion of builtin macros like ``__has_embed()`` with line breaks before the
closing paren. (#GH133574)
- Fixed a crash in error recovery for expressions resolving to templates. (#GH135621)
- Clang no longer accepts invalid integer constants which are too large to fit
into any (standard or extended) integer type when the constant is unevaluated.
Merely forming the token is sufficient to render the program invalid. Code
Expand Down Expand Up @@ -549,7 +550,7 @@ Arm and AArch64 Support
^^^^^^^^^^^^^^^^^^^^^^^
- For ARM targets, cc1as now considers the FPU's features for the selected CPU or Architecture.
- The ``+nosimd`` attribute is now fully supported for ARM. Previously, this had no effect when being used with
ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
also now printed when the ``--print-supported-extensions`` option is used.

- Support for __ptrauth type qualifier has been added.
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4401,7 +4401,8 @@ TemplateSpecializationType::TemplateSpecializationType(
T.getKind() == TemplateName::SubstTemplateTemplateParmPack ||
T.getKind() == TemplateName::UsingTemplate ||
T.getKind() == TemplateName::QualifiedTemplate ||
T.getKind() == TemplateName::DeducedTemplate) &&
T.getKind() == TemplateName::DeducedTemplate ||
T.getKind() == TemplateName::AssumedTemplate) &&
"Unexpected template name for TemplateSpecializationType");

auto *TemplateArgs =
Expand Down
12 changes: 8 additions & 4 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21111,11 +21111,15 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
const bool IsTypeAliasTemplateDecl = isa<TypeAliasTemplateDecl>(Temp);

NestedNameSpecifier *NNS = ULE->getQualifierLoc().getNestedNameSpecifier();
TemplateName TN(dyn_cast<TemplateDecl>(Temp));
if (TN.isNull())
// FIXME: AssumedTemplate is not very appropriate for error recovery here,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Its a shame we cannot do a getQualifiedAssumedTemplateName here, and only treat the base-name as assumed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are other solutions to explore.

It seems that for this specific error, we could fish out the actual template declaration this is referring to from the NestedNameSpecifier. Or maybe there is something we could do with the name lookup implementation to improve this and return an actual template.

But this would be a bunch of work for an error recovery path that stayed untested up until this point.

// as it models only the unqualified-id case, where this case can clearly be
// qualified. Thus we can't just qualify an assumed template.
TemplateName TN;
if (auto *TD = dyn_cast<TemplateDecl>(Temp))
TN = Context.getQualifiedTemplateName(NNS, ULE->hasTemplateKeyword(),
TemplateName(TD));
else
TN = Context.getAssumedTemplateName(NameInfo.getName());
TN = Context.getQualifiedTemplateName(NNS,
/*TemplateKeyword=*/true, TN);

Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
<< TN << ULE->getSourceRange() << IsTypeAliasTemplateDecl;
Expand Down
11 changes: 11 additions & 0 deletions clang/test/SemaTemplate/recovery-crash.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,14 @@ namespace test1 {
// expected-note@#defined-here {{defined here}}
void NonTemplateClass::UndeclaredMethod() {}
}

namespace GH135621 {
template <class T> struct S {};
// expected-note@-1 {{class template declared here}}
template <class T2> void f() {
S<T2>::template S<int>;
// expected-error@-1 {{'S' is expected to be a non-type template, but instantiated to a class template}}
}
template void f<int>();
// expected-note@-1 {{requested here}}
} // namespace GH135621
Loading