[clang][Parser] Warn when the body of expansion statement is not a compound statement - #209229
Merged
Conversation
llvmorg-github-actions
Bot
added
clang
|
@llvm/pr-subscribers-clang Author: Yanzuo Liu (zwuis) Changes<https://eel.is/c++draft/stmt.expand#nt:expansion-statement>: expansion-statement: Full diff: https://github.com/llvm/llvm-project/pull/209229.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 55b26deed0750..389900040399a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -457,6 +457,9 @@ def err_expansion_stmt_requires_cxx2c : Error<
"expansion statements are only supported in C++2c">;
def err_for_template : Error<
"'for template' is invalid; use 'template for' instead">;
+def ext_expansion_stmt_requires_braced_body : ExtWarn<
+ "ISO C++ requires a compound statement to be the body of expansion statement">,
+ InGroup<DiagGroup<"expansion-stmt-braced-body">>, DefaultIgnore;
def err_expected_case_before_expression: Error<
"expected 'case' keyword before expression">;
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index bdaea72cf52a1..bbde87794b652 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -2329,6 +2329,10 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc,
// the other parts.
getCurScope()->EnterLoopBody(PrecedingLabel);
+ bool BodyIsCompoundStmt = Tok.is(tok::l_brace);
+ // attribute-specifier without attribute (`[[]]`) isn't in AST.
+ SourceLocation BodyBeginLoc = Tok.getLocation();
+
// C99 6.8.5p5 - In C99, the body of the for statement is a scope, even if
// there is no compound stmt. C90 does not have this clause. We only do this
// if the body isn't a compound statement to avoid push/pop in common cases.
@@ -2341,7 +2345,7 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc,
// for-init-statement/condition and a new scope for substatement in C++.
//
ParseScope InnerScope(this, Scope::DeclScope, C99orCXXorObjC,
- Tok.is(tok::l_brace));
+ BodyIsCompoundStmt);
// The body of the for loop has the same local mangling number as the
// for-init-statement.
@@ -2379,6 +2383,9 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc,
return StmtError();
}
+ if (!BodyIsCompoundStmt)
+ Diag(BodyBeginLoc, diag::ext_expansion_stmt_requires_braced_body);
+
return Actions.FinishCXXExpansionStmt(ForRangeStmt.get(), Body.get());
}
diff --git a/clang/test/Parser/cxx2c-expansion-statements.cpp b/clang/test/Parser/cxx2c-expansion-statements.cpp
index 736f9fead383c..2ca94270cd60a 100644
--- a/clang/test/Parser/cxx2c-expansion-statements.cpp
+++ b/clang/test/Parser/cxx2c-expansion-statements.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify=expected,brace -Wexpansion-stmt-braced-body
namespace std {
template <typename T>
struct initializer_list {
@@ -14,31 +15,82 @@ void bad() {
template for (;); // expected-error {{expected ';' in 'for' statement specifier}} expected-error {{expansion statement must use the syntax of a range-based for loop}}
template for (;;); // expected-error {{expansion statement must use the syntax of a range-based for loop}}
template for (int x;;); // expected-error {{expansion statement must use the syntax of a range-based for loop}}
- template for (x : {1}); // expected-error {{expansion statement requires type for expansion variable}}
+ template for (x : {1});
+ // expected-error@-1 {{expansion statement requires type for expansion variable}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
template for (: {1}); // expected-error {{expected expression}} expected-error {{expected ';' in 'for' statement specifier}} expected-error {{expansion statement must use the syntax of a range-based for loop}}
template for (auto y : {1})]; // expected-error {{expected expression}}
- template for (auto y : {1}; // expected-error {{expected ')'}} expected-note {{to match this '('}}
- template for (extern auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'extern'}}
- template for (register auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'register'}} expected-error {{ISO C++17 does not allow 'register' storage class specifier}}
- template for (__private_extern__ auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'extern'}}
- template for (extern static auto y : {1, 2}); // expected-error {{cannot combine with previous 'extern' declaration specifier}} expected-error {{expansion variable 'y' may not be declared 'extern'}}
- template for (static auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}}
- template for (thread_local auto y : {1, 2}); // expected-error {{'thread_local' variables must have global storage}}
- template for (static thread_local auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'thread_local'}}
- template for (__thread auto y : {1, 2}); // expected-error {{'__thread' variables must have global storage}}
- template for (static __thread auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}}
- template for (constinit auto y : {1, 2}); // expected-error {{local variable cannot be declared 'constinit'}}
- template for (consteval auto y : {1, 2}); // expected-error {{consteval can only be used in function declarations}}
- template for (int x; extern auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'extern'}}
- template for (int x; extern static auto y : {1, 2}); // expected-error {{cannot combine with previous 'extern' declaration specifier}} expected-error {{expansion variable 'y' may not be declared 'extern'}}
- template for (int x; static auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}}
- template for (int x; thread_local auto y : {1, 2}); // expected-error {{'thread_local' variables must have global storage}}
- template for (int x; static thread_local auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'thread_local'}}
- template for (int x; __thread auto y : {1, 2}); // expected-error {{'__thread' variables must have global storage}}
- template for (int x; static __thread auto y : {1, 2}); // expected-error {{expansion variable 'y' may not be declared 'static'}}
- template for (int x; constinit auto y : {1, 2}); // expected-error {{local variable cannot be declared 'constinit'}}
- template for (int x; consteval auto y : {1, 2}); // expected-error {{consteval can only be used in function declarations}}
- template for (auto y : {abc, -+, }); // expected-error {{use of undeclared identifier 'abc'}} expected-error {{expected expression}}
+ template for (auto y : {1};
+ // expected-error@-1 {{expected ')'}}
+ // expected-note@-2 {{to match this '('}}
+ // brace-warning@-3 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (extern auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'extern'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (register auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'register'}}
+ // expected-error@-2 {{ISO C++17 does not allow 'register' storage class specifier}}
+ // brace-warning@-3 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (__private_extern__ auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'extern'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (extern static auto y : {1, 2});
+ // expected-error@-1 {{cannot combine with previous 'extern' declaration specifier}}
+ // expected-error@-2 {{expansion variable 'y' may not be declared 'extern'}}
+ // brace-warning@-3 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (static auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'static'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (thread_local auto y : {1, 2});
+ // expected-error@-1 {{'thread_local' variables must have global storage}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (static thread_local auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'thread_local'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (__thread auto y : {1, 2});
+ // expected-error@-1 {{'__thread' variables must have global storage}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (static __thread auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'static'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (constinit auto y : {1, 2});
+ // expected-error@-1 {{local variable cannot be declared 'constinit'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (consteval auto y : {1, 2});
+ // expected-error@-1 {{consteval can only be used in function declarations}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; extern auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'extern'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; extern static auto y : {1, 2});
+ // expected-error@-1 {{cannot combine with previous 'extern' declaration specifier}}
+ // expected-error@-2 {{expansion variable 'y' may not be declared 'extern'}}
+ // brace-warning@-3 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; static auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'static'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; thread_local auto y : {1, 2});
+ // expected-error@-1 {{'thread_local' variables must have global storage}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; static thread_local auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'thread_local'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; __thread auto y : {1, 2});
+ // expected-error@-1 {{'__thread' variables must have global storage}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; static __thread auto y : {1, 2});
+ // expected-error@-1 {{expansion variable 'y' may not be declared 'static'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; constinit auto y : {1, 2});
+ // expected-error@-1 {{local variable cannot be declared 'constinit'}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; consteval auto y : {1, 2});
+ // expected-error@-1 {{consteval can only be used in function declarations}}
+ // brace-warning@-2 {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (auto y : {abc, -+, });
+ // expected-error@-1 {{use of undeclared identifier 'abc'}}
+ // expected-error@-2 {{expected expression}}
+ // brace-warning@-3 {{ISO C++ requires a compound statement to be the body of expansion statement}}
template for (3 : "error") // expected-error {{expansion statement declaration must declare a variable}} \
expected-error {{expansion statement must use the syntax of a range-based for loop}}
;
@@ -46,18 +98,21 @@ void bad() {
; // Semicolon for synchronisation; otherwise, the parser skips over next statement...
template do {} while (true); // expected-error {{expected '<' after 'template'}}
for template (int x : {}) {} // expected-error {{'for template' is invalid; use 'template for' instead}}
+ template for (int x : {1})
+ [ // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ []] {}
}
void good() {
- template for (auto y : {});
- template for (auto y : {1, 2});
- template for (int x; auto y : {1, 2});
- template for (int x; int y : {1, 2});
- template for (int x; constexpr auto y : {1, 2});
- template for (int x; constexpr int y : {1, 2});
+ template for (auto y : {}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (auto y : {1, 2}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; auto y : {1, 2}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; int y : {1, 2}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; constexpr auto y : {1, 2}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
+ template for (int x; constexpr int y : {1, 2}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
template for (constexpr int a : {1, 2}) {
template for (constexpr int b : {1, 2}) {
- template for (constexpr int c : {1, 2});
+ template for (constexpr int c : {1, 2}); // brace-warning {{ISO C++ requires a compound statement to be the body of expansion statement}}
}
}
}
|
Sirraide
reviewed
Jul 13, 2026
Member
There was a problem hiding this comment.
I’m not sure it makes sense to put this into a new diagnostic group because I don’t think anyone is going to enable this. It might make more sense to put this in -pedantic, but also GCC doesn’t warn on this at all (even w/ -pedantic).
I also find it rather strange that the standard even requires a compound-statement here; that just seems like an arbitrary restriction. I wonder if we should file either a core issue or a GCC bug for this?
CC @AaronBallman, @cor3ntin for opinions
Member
|
We just discussed this during Aaron’s office hours today, and we haven’t been able to find a reason as to why this is specified to require a compound-statement rather than just a statement, so we want to spend some time investigating whether it wouldn’t make more sense to update standard to say statement instead. |
Contributor
Author
|
Contributor
|
Let's make it a pedantic warning for now. If WG21 changes the design we can adjust. |
Contributor
A pedantic warning sounds good to me. I also can not figure out why this is specified to require a compound-statement rather than just a statement. The standard seem to have no reason to refuse a single statement. |
cor3ntin
approved these changes
Jul 14, 2026
cor3ntin
reviewed
Jul 14, 2026
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
cor3ntin
approved these changes
Jul 15, 2026
Contributor
|
Can we get into the same issue with gnu/msvc attributes? |
|
|
||
| // attribute-specifier without attribute (`[[]]`) isn't in AST. | ||
| // `__declspec()` is only applied to declarations, so we can ignore it. | ||
| if (!isa<CompoundStmt>(Body.get()) || BodyStartsWithAttr) |
Contributor
There was a problem hiding this comment.
Whelp, I think this logic is correct but I really don't agree with the design in the standard and think there's another extension hiding in here:
int main() {
template for (int x : {1}) [[clang::atomic(remote_memory)]] {}
}
That's invalid per the standard because the standard requires a compound-statement and that grammar production cannot have a leading attribute. If WG21 used statement instead, then a leading attribute would be accepted as you'd expect.
I think the diagnostic text will be confusing in that situation because there is a compound statement. That suggests we want to handle attributes with their own diagnostic, but that would mean we'd need to get much better about recognizing attributes instead of looking for a single token.
Ideally, we'd have a CompoundStmt or an AttributedStmt which wraps a CompoundStmt, but there are two scenarios I think that won't cover:
template for (int x : {1}) [[]] { } // Empty attributes don't make an AttributedStmt
template for (int x : {1}) [[unknown]] {} // Unknown attributes don't make an AttributedStmt either
Other test cases to consider would be Objective-C++ or lambdas in C++, so it's not just attributes:
template for (int x : {1}) [obj msg];
template for (int x : {1}) [](){}();
Contributor
There was a problem hiding this comment.
I mean, our logic could actually warn by checking the next token is { - that cover all the cases, i think
https://eel.is/c++draft/stmt.block#nt:compound-statement
Contributor
There was a problem hiding this comment.
Possibly. The cases I was worried about for token lookahead also involved lexer stuff like
template for (int x : {1})
#pragma clang diagnostic ...
{}
and whether anything can usefully produce an annotation token before the compound statement.
Member
There was a problem hiding this comment.
Yes, I also think we should just check if the next token is {; but also, all of these issues really make it seem like the wording should just say statement instead...
whether anything can usefully produce an annotation token before the compound statement.
Code completion maybe?
cor3ntin
reviewed
Jul 20, 2026
cor3ntin
approved these changes
Jul 21, 2026
Contributor
Author
|
Thanks for the review! Merging now. |
github-project-automation
Bot
moved this from Needs Triage
to Done
in LLVM Release Status
Contributor
Author
|
/cherry-pick d13b862 |
Member
|
/pull-request #210908 |
dyung
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jul 22, 2026
…mpound statement (llvm#209229) <https://eel.is/c++draft/stmt.expand#nt:expansion-statement>: _expansion-statement_: template for ( _init-statement<sub>opt</sub>_ _for-range-declaration_ : _expansion-initializer_ ) _compound-statement_ (cherry picked from commit d13b862)
midhuncodes7
pushed a commit
to midhuncodes7/llvm-project
that referenced
this pull request
Jul 28, 2026
…mpound statement (llvm#209229) <https://eel.is/c++draft/stmt.expand#nt:expansion-statement>: _expansion-statement_: template for ( _init-statement<sub>opt</sub>_ _for-range-declaration_ : _expansion-initializer_ ) _compound-statement_
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
https://eel.is/c++draft/stmt.expand#nt:expansion-statement:
expansion-statement:
template for ( init-statementopt for-range-declaration : expansion-initializer ) compound-statement