[Clang] [Sema] Allow non-local/non-variable declarations in for loop #129737

Sirraide · 2025-03-04T16:40:00Z

Currently, we error on non-variable or non-local variable declarations in for loops such as for (struct S {}; 0; ) {}. However, this is valid in C23, so this patch changes the error to a compatibilty warning and also allows this as an extension in earlier language modes. This also matches GCC’s behaviour.

llvmbot · 2025-03-04T16:40:38Z

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

Currently, we error on non-variable or non-local variable declarations in for loops such as for (struct S {}; 0; ) {}. However, this is valid in C23, so this patch changes the error to a compatibilty warning and also allows this as an extension in earlier language modes. This also matches GCC’s behaviour.

Full diff: https://github.com/llvm/llvm-project/pull/129737.diff

4 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+2)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+17)
(modified) clang/lib/Sema/SemaStmt.cpp (+8-5)
(modified) clang/test/Sema/for.c (+15-7)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 97b8e32f03b57..cf205e7064aa8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -237,6 +237,8 @@ Bug Fixes in This Version
   when it can affect template argument deduction (#GH122306).
 - Fix crash on code completion of function calls involving partial order of function templates
   (#GH125500).
+- Non-local variable and non-variable declarations in the first clause of a ``for`` loop in C are no longer erroneously
+  considered an error in C23 mode and are allowed as an extension in earlier language modes.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d89648a8a2e83..c87ef62310cb9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10794,6 +10794,23 @@ def err_non_local_variable_decl_in_for : Error<
   "declaration of non-local variable in 'for' loop">;
 def err_non_variable_decl_in_for : Error<
   "non-variable declaration in 'for' loop">;
+
+def ext_c23_non_local_variable_decl_in_for : Extension<
+  "declaration of non-local variable in 'for' loop is a C23 extension">,
+  InGroup<C23>;
+
+def warn_c17_non_local_variable_decl_in_for : Warning<
+  "declaration of non-local variable in 'for' loop is incompatible with C standards before C23">,
+  DefaultIgnore, InGroup<CPre23Compat>;
+
+def ext_c23_non_variable_decl_in_for : Extension<
+  "non-variable declaration in 'for' loop is a C23 extension">,
+  InGroup<C23>;
+
+def warn_c17_non_variable_decl_in_for : Warning<
+  "non-variable declaration in 'for' loop is incompatible with C standards before C23">,
+  DefaultIgnore, InGroup<CPre23Compat>;
+
 def err_toomany_element_decls : Error<
   "only one element declaration is allowed">;
 def err_selector_element_not_lvalue : Error<
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index d0b713f074c33..0a193b5299bcc 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -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())
+            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);
     }
   }
 
diff --git a/clang/test/Sema/for.c b/clang/test/Sema/for.c
index d0c2f7f21a960..c21ef62247037 100644
--- a/clang/test/Sema/for.c
+++ b/clang/test/Sema/for.c
@@ -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}}
+                                               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}} */

clang/docs/ReleaseNotes.rst

clang/include/clang/Basic/DiagnosticSemaKinds.td

AaronBallman

LGTM

shafik

Should this have updated: https://clang.llvm.org/c_status.html

Also it would have been nice to mention the paper number in the release notes, I am assuming there was a paper?

CC @AaronBallman

shafik · 2025-03-07T01:55:44Z

clang/lib/Sema/SemaStmt.cpp

-            Diag(DI->getLocation(), diag::err_non_local_variable_decl_in_for);
-            DI->setInvalidDecl();
-          }
+          if (VD->isLocalVarDecl() && !VD->hasLocalStorage())


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 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?

shafik · 2025-03-07T02:03:09Z

clang/test/Sema/for.c

-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}}


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.

I mean, I can reference them in the loop condition I suppose.

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.

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.

Sirraide · 2025-03-07T13:46:50Z

Should this have updated: https://clang.llvm.org/c_status.html

Not according to Aaron: #129737 (comment)

shafik · 2025-03-07T15:36:40Z

Should this have updated: https://clang.llvm.org/c_status.html

Not according to Aaron: #129737 (comment)

Oh wow, my bad, I missed that 😬

[Clang] Allow non-local/non-variable declarations in for loop

f0605e8

Sirraide added the clang:frontend label Mar 4, 2025

Sirraide requested review from cor3ntin, AaronBallman and Fznamznon March 4, 2025 16:40

llvmbot added the clang label Mar 4, 2025

Fznamznon reviewed Mar 4, 2025

View reviewed changes

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

clang/include/clang/Basic/DiagnosticSemaKinds.td Show resolved Hide resolved

cor3ntin added the c23 label Mar 5, 2025

Update release notes

3ab8a48

AaronBallman approved these changes Mar 5, 2025

View reviewed changes

Merge branch 'main' into c-for-loop

95a2d39

Sirraide merged commit 5b3ba26 into llvm:main Mar 6, 2025
12 checks passed

Sirraide deleted the c-for-loop branch March 6, 2025 01:03

frederick-vs-ja mentioned this pull request Mar 6, 2025

_Static_assert declaration can not be first of for-triplet #56471

Open

shafik reviewed Mar 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] [Sema] Allow non-local/non-variable declarations in for loop #129737

[Clang] [Sema] Allow non-local/non-variable declarations in for loop #129737

Sirraide commented Mar 4, 2025 •

edited

Loading

llvmbot commented Mar 4, 2025

AaronBallman left a comment

shafik left a comment

shafik Mar 7, 2025

Sirraide Mar 7, 2025

shafik Mar 7, 2025

Sirraide Mar 7, 2025

shafik Mar 7, 2025

Sirraide Mar 7, 2025

Sirraide commented Mar 7, 2025

shafik commented Mar 7, 2025

[Clang] [Sema] Allow non-local/non-variable declarations in for loop #129737

[Clang] [Sema] Allow non-local/non-variable declarations in for loop #129737

Conversation

Sirraide commented Mar 4, 2025 • edited Loading

llvmbot commented Mar 4, 2025

AaronBallman left a comment

Choose a reason for hiding this comment

shafik left a comment

Choose a reason for hiding this comment

shafik Mar 7, 2025

Choose a reason for hiding this comment

Sirraide Mar 7, 2025

Choose a reason for hiding this comment

shafik Mar 7, 2025

Choose a reason for hiding this comment

Sirraide Mar 7, 2025

Choose a reason for hiding this comment

shafik Mar 7, 2025

Choose a reason for hiding this comment

Sirraide Mar 7, 2025

Choose a reason for hiding this comment

Sirraide commented Mar 7, 2025

shafik commented Mar 7, 2025

Sirraide commented Mar 4, 2025 •

edited

Loading