[clang][Sema] Fix crash on decomposition decl missing initializer - #210151
Conversation
|
@llvm/pr-subscribers-clang Author: Patryk Stefanski (patrykstefanski) ChangesActOnUninitializedDecl dereferenced the std::optional<Token> from Lexer::findNextToken() unconditionally when diagnosing a structured binding with no initializer. Guard the optional and fall back to the declaration's location. Full diff: https://github.com/llvm/llvm-project/pull/210151.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index c5920f03ed6e1..b7679ece0bfb5 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14521,12 +14521,16 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
}
// C++1z [dcl.dcl]p1 grammar implies that an initializer is mandatory.
if (isa<DecompositionDecl>(RealDecl)) {
- // Point the caret to the token immediately after the closing bracket.
- auto NextLoc = dyn_cast<DecompositionDecl>(RealDecl)->getRSquareLoc();
- NextLoc =
- Lexer::findNextToken(NextLoc, PP.getSourceManager(), PP.getLangOpts())
- ->getLocation();
- Diag(NextLoc, diag::err_decomp_decl_requires_init) << Var;
+ // Point the caret to the token immediately after the closing bracket if
+ // it can be found; otherwise fall back to the declaration's location.
+ SourceLocation Loc = Var->getLocation();
+ SourceLocation RSquareLoc =
+ dyn_cast<DecompositionDecl>(RealDecl)->getRSquareLoc();
+ if (std::optional<Token> Next = Lexer::findNextToken(
+ RSquareLoc, PP.getSourceManager(), PP.getLangOpts());
+ Next.has_value())
+ Loc = Next->getLocation();
+ Diag(Loc, diag::err_decomp_decl_requires_init) << Var;
Var->setInvalidDecl();
return;
}
diff --git a/clang/test/Parser/cxx1z-decomposition.cpp b/clang/test/Parser/cxx1z-decomposition.cpp
index fb22364ddb802..607a628506e8a 100644
--- a/clang/test/Parser/cxx1z-decomposition.cpp
+++ b/clang/test/Parser/cxx1z-decomposition.cpp
@@ -152,6 +152,7 @@ namespace Template {
}
#define MYC C
+#define CLOSE_NO_INIT ] ;
namespace Init {
template<typename T> T f(T t) {
@@ -171,6 +172,8 @@ namespace Init {
T t1 = t; // check that uninitialized structured binding declaration error works with templates and macros
auto [t0, t2] MYC = {t, t1}; // expected-error{{structured binding declaration '[t0, t2]' requires an initializer; expected '=' or braced initializer list}} expected-error{{expected ';' at end of declaration}}
// CHECK: :[[@LINE-1]]:19: error: structured binding declaration '[t0, t2]' requires an initializer; expected '=' or braced initializer list
+ auto [bad4 CLOSE_NO_INIT // expected-error {{structured binding declaration '[bad4]' requires an initializer; expected '=' or braced initializer list}}
+ // CHECK: :[[@LINE-1]]:10: error: structured binding declaration '[bad4]' requires an initializer; expected '=' or braced initializer list
}
}
|
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) cross-project-testscross-project-tests.intrinsic-header-tests/riscv_packed_simd.cIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
erichkeane
left a comment
There was a problem hiding this comment.
1 suggestion, else I think this is sensible to me.
| SourceLocation Loc = Var->getLocation(); | ||
| SourceLocation RSquareLoc = | ||
| dyn_cast<DecompositionDecl>(RealDecl)->getRSquareLoc(); | ||
| if (std::optional<Token> Next = Lexer::findNextToken( |
There was a problem hiding this comment.
optional has an operator bool, right? Why are you not just testing it?
Ala:
if (std::optional<Token> Next = Lexer::findNextToken(RSquareLoc, PP.getSourceManager(), PP.getLangOpts()) ?
There was a problem hiding this comment.
Thanks for the feedback, updated to your suggestion.
a28f981 to
f874288
Compare
erichkeane
left a comment
There was a problem hiding this comment.
Just needs a release note, else this LGTM.
A "week ago" is still old enough to have made it into the 23.0 branch! So we still need a release note HERE, but we probably also want to cherry-pick this to the release branch. Also, see you have a formatter error that you have to fix, linux test seems unrelated. |
f874288 to
868542c
Compare
If we're cherry-picking to the release branch, no need for the release note at all, right? Clang 22.x is fine today and Clang 23.x will be fine after the cherry-pick, so Clang 24.x (main branch) doesn't need a note either? |
Oh, right! For some reason I thought we were already 23.1. Yep, @patrykstefanski : Please remove teh release note now (sorry!), and feel free to merge once CI agrees. After that, you/we can do the cherry-pick. |
ActOnUninitializedDecl dereferenced the std::optional<Token> from Lexer::findNextToken() unconditionally when diagnosing a structured binding with no initializer. Guard the optional and fall back to the declaration's location.
868542c to
dd698af
Compare
|
Unrelated test |
|
/cherry-pick 0e3852f |
|
/pull-request #210444 |
…vm#210151) ActOnUninitializedDecl dereferenced the std::optional<Token> from Lexer::findNextToken() unconditionally when diagnosing a structured binding with no initializer. Guard the optional and fall back to the declaration's location. (cherry picked from commit 0e3852f)
ActOnUninitializedDecl dereferenced the std::optional from Lexer::findNextToken() unconditionally when diagnosing a structured binding with no initializer. Guard the optional and fall back to the declaration's location.