[clang] fix a crash in error recovery in expressions resolving to templates #135893
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesWe were using AssumedTemplate incorrectly for error recovery. Fixes #135621 Full diff: https://github.com/llvm/llvm-project/pull/135893.diff 4 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 38142ad32bea0..562ac29f4b998 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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. (#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
@@ -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.
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 62e48062cf241..53620003c9655 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -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 =
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3ac7d61546ceb..5d9fd78c52968 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -21111,11 +21111,14 @@ 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: Can't qualify an assumed template because it explicitly models the
+ // unqualified-id case. Just dropping the qualifiers is wrong though.
+ 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;
diff --git a/clang/test/SemaTemplate/recovery-crash.cpp b/clang/test/SemaTemplate/recovery-crash.cpp
index ac8053da101ab..9b106f1f21fc5 100644
--- a/clang/test/SemaTemplate/recovery-crash.cpp
+++ b/clang/test/SemaTemplate/recovery-crash.cpp
@@ -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
|
mizvekov
force-pushed
the
users/mizvekov/GH135621
branch
from
c2a06ac to
9f3ea29
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
mizvekov
force-pushed
the
users/mizvekov/GH135621
branch
from
9f3ea29 to
c52264e
Compare
erichkeane
approved these changes
Apr 16, 2025
| @@ -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, | |||
There was a problem hiding this comment.
Its a shame we cannot do a getQualifiedAssumedTemplateName here, and only treat the base-name as assumed.
There was a problem hiding this comment.
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.
…plates We were using AssumedTemplate incorrectly for error recovery. Fixes #135621
mizvekov
force-pushed
the
users/mizvekov/GH135621
branch
from
c52264e to
d06164c
Compare
zyn0217
approved these changes
Apr 16, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16102 Here is the relevant piece of the build log for the reference |
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…plates (llvm#135893) We were using AssumedTemplate incorrectly for error recovery. Fixes llvm#135621
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were using AssumedTemplate incorrectly for error recovery.
Fixes #135621