-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[Clang] [Sema] Allow non-local/non-variable declarations in for loop #129737
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2269,10 +2269,11 @@ StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, | |
for (auto *DI : DS->decls()) { | ||
if (VarDecl *VD = dyn_cast<VarDecl>(DI)) { | ||
VarDeclSeen = true; | ||
if (VD->isLocalVarDecl() && !VD->hasLocalStorage()) { | ||
Diag(DI->getLocation(), diag::err_non_local_variable_decl_in_for); | ||
DI->setInvalidDecl(); | ||
} | ||
if (VD->isLocalVarDecl() && !VD->hasLocalStorage()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a big fan of quoting the section of the standard that allows various features. This allows folks in the future to know where to look to better understand the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a C99 quote further up above on line 2264; we could add a comment that mentions that this changed at some point though. Something like ‘C23 has no such restriction’ maybe? |
||
Diag(DI->getLocation(), | ||
getLangOpts().C23 | ||
? diag::warn_c17_non_local_variable_decl_in_for | ||
: diag::ext_c23_non_local_variable_decl_in_for); | ||
} else if (!NonVarSeen) { | ||
// Keep track of the first non-variable declaration we saw so that | ||
// we can diagnose if we don't see any variable declarations. This | ||
|
@@ -2284,7 +2285,9 @@ StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, | |
// Diagnose if we saw a non-variable declaration but no variable | ||
// declarations. | ||
if (NonVarSeen && !VarDeclSeen) | ||
Diag(NonVarSeen->getLocation(), diag::err_non_variable_decl_in_for); | ||
Diag(NonVarSeen->getLocation(), | ||
getLangOpts().C23 ? diag::warn_c17_non_variable_decl_in_for | ||
: diag::ext_c23_non_variable_decl_in_for); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,21 @@ | ||
// RUN: %clang_cc1 -fsyntax-only -verify %s | ||
// RUN: %clang_cc1 -fsyntax-only -verify=c11 -std=c11 -pedantic %s | ||
// RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 -Wpre-c23-compat %s | ||
|
||
// Check C99 6.8.5p3 | ||
void b1 (void) { for (void (*f) (void);;); } | ||
void b2 (void) { for (void f (void);;); } // expected-error {{non-variable declaration in 'for' loop}} | ||
void b3 (void) { for (static int f;;); } // expected-error {{declaration of non-local variable}} | ||
void b4 (void) { for (typedef int f;;); } // expected-error {{non-variable declaration in 'for' loop}} | ||
void b2 (void) { for (void f (void);;); } /* c11-warning {{non-variable declaration in 'for' loop is a C23 extension}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these test should have show the non-local variables being used within the loops as well. Otherwise the tests are not fully testing the functionality and are not fully protecting against regressions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, I can reference them in the loop condition I suppose. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies for the drive by after commit review but I really would like to see that. Having spent some time digging into regression the past few months a main pattern is lack of testing and so I am trying to push asking for more in the testing arena. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense; I’m a bit busy at the moment but I should be able to do that next week considering that it shouldn’t take too long. |
||
c23-warning {{non-variable declaration in 'for' loop is incompatible with C standards before C23}} */ | ||
void b3 (void) { for (static int f;;); } /* c11-warning {{declaration of non-local variable in 'for' loop is a C23 extension}} | ||
c23-warning {{declaration of non-local variable in 'for' loop is incompatible with C standards before C23}} */ | ||
|
||
void b4 (void) { for (typedef int f;;); } /* c11-warning {{non-variable declaration in 'for' loop is a C23 extension}} | ||
c23-warning {{non-variable declaration in 'for' loop is incompatible with C standards before C23}} */ | ||
void b5 (void) { for (struct { int i; } s;;); } | ||
void b6 (void) { for (enum { zero, ten = 10 } i;;); } | ||
void b7 (void) { for (struct s { int i; };;); } // expected-error {{non-variable declaration in 'for' loop}} | ||
void b8 (void) { for (static struct { int i; } s;;); } // expected-error {{declaration of non-local variable}} | ||
void b7 (void) { for (struct s { int i; };;); } /* c11-warning {{non-variable declaration in 'for' loop is a C23 extension}} | ||
c23-warning {{non-variable declaration in 'for' loop is incompatible with C standards before C23}} */ | ||
void b8 (void) { for (static struct { int i; } s;;); } /* c11-warning {{declaration of non-local variable in 'for' loop is a C23 extension}} | ||
c23-warning {{declaration of non-local variable in 'for' loop is incompatible with C standards before C23}} */ | ||
void b9 (void) { for (struct { int i; } (*s)(struct { int j; } o) = 0;;); } | ||
void b10(void) { for (typedef struct { int i; } (*s)(struct { int j; });;); } // expected-error {{non-variable declaration in 'for' loop}} | ||
void b10(void) { for (typedef struct { int i; } (*s)(struct { int j; });;); } /* c11-warning {{non-variable declaration in 'for' loop is a C23 extension}} | ||
c23-warning {{non-variable declaration in 'for' loop is incompatible with C standards before C23}} */ |
Uh oh!
There was an error while loading. Please reload this page.