[clang] [C] Disallow declarations where a statement is required (PR #92908)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue May 28 06:39:20 PDT 2024


https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/92908

>From 219dae02c3235c17bc4568496a7df9763d798e2a Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Mon, 20 May 2024 13:40:28 -0400
Subject: [PATCH 1/4] [C] Fix declaration parsing

8bd06d5b65845e5e01dd899a2deb773580460b89 removed the notion of parsed
statement contexts that track whether a declaration is valid for C or
not. This resurrects the concept, because C still does not allow a
declaration as a substatement.
---
 clang/include/clang/Parse/Parser.h | 9 ++++++---
 clang/lib/Parse/ParseStmt.cpp      | 5 ++++-
 clang/test/C/C99/block-scopes.c    | 3 ++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 5f04664141d29..5e455b16fb8c0 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -467,15 +467,18 @@ class Parser : public CodeCompletionHandler {
 
   /// Flags describing a context in which we're parsing a statement.
   enum class ParsedStmtContext {
+    /// This context permits declarations in language modes where declarations
+    /// are not statements.
+    AllowDeclarationsInC = 0x1,
     /// This context permits standalone OpenMP directives.
-    AllowStandaloneOpenMPDirectives = 0x1,
+    AllowStandaloneOpenMPDirectives = 0x2,
     /// This context is at the top level of a GNU statement expression.
-    InStmtExpr = 0x2,
+    InStmtExpr = 0x4,
 
     /// The context of a regular substatement.
     SubStmt = 0,
     /// The context of a compound-statement.
-    Compound = AllowStandaloneOpenMPDirectives,
+    Compound = AllowDeclarationsInC | AllowStandaloneOpenMPDirectives,
 
     LLVM_MARK_AS_BITMASK_ENUM(InStmtExpr)
   };
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index b0af04451166c..57b9fb10a1e08 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -239,7 +239,10 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
     auto IsStmtAttr = [](ParsedAttr &Attr) { return Attr.isStmtAttr(); };
     bool AllAttrsAreStmtAttrs = llvm::all_of(CXX11Attrs, IsStmtAttr) &&
                                 llvm::all_of(GNUAttrs, IsStmtAttr);
-    if (((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
+    if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
+         (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
+             ParsedStmtContext()) &&
+        ((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
          isDeclarationStatement())) {
       SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
       DeclGroupPtrTy Decl;
diff --git a/clang/test/C/C99/block-scopes.c b/clang/test/C/C99/block-scopes.c
index 589047df3e52b..116e5d922593e 100644
--- a/clang/test/C/C99/block-scopes.c
+++ b/clang/test/C/C99/block-scopes.c
@@ -18,8 +18,9 @@
 
 enum {a, b};
 void different(void) {
-  if (sizeof(enum {b, a}) != sizeof(int))
+  if (sizeof(enum {b, a}) != sizeof(int)) {
     _Static_assert(a == 1, "");
+  }
   /* In C89, the 'b' found here would have been from the enum declaration in
    * the controlling expression of the selection statement, not from the global
    * declaration. In C99 and later, that enumeration is scoped to the 'if'

>From c67502ddeb0c1db5c71fe1eb115250bd238c0d9d Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 21 May 2024 08:34:01 -0400
Subject: [PATCH 2/4] Update OpenACC test for slight behavioral change

---
 clang/test/SemaOpenACC/parallel-loc-and-stmt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang/test/SemaOpenACC/parallel-loc-and-stmt.c b/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
index ba29f6da8ba25..bbcdd823483a5 100644
--- a/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
+++ b/clang/test/SemaOpenACC/parallel-loc-and-stmt.c
@@ -33,9 +33,11 @@ int foo3;
 
 void func() {
   // FIXME: Should we disallow this on declarations, or consider this to be on
-  // the initialization?
+  // the initialization? This is currently rejected in C because
+  // Parser::ParseOpenACCDirectiveStmt() calls ParseStatement() and passes the
+  // statement context as "SubStmt" which does not allow for a declaration in C.
 #pragma acc parallel
-  int foo;
+  int foo; // expected-error {{expected expression}}
 
 #pragma acc parallel
   {

>From 6e2e9c015ba5c60204f3ef71ebd63db8d986835a Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 21 May 2024 08:40:46 -0400
Subject: [PATCH 3/4] Add test coverage for changes

---
 clang/test/Parser/decls.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 clang/test/Parser/decls.c

diff --git a/clang/test/Parser/decls.c b/clang/test/Parser/decls.c
new file mode 100644
index 0000000000000..39ef05bf4bd99
--- /dev/null
+++ b/clang/test/Parser/decls.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic
+
+// Test that we can parse declarations at global scope.
+int v;
+
+void func(void) {
+  // Test that we can parse declarations within a compound statement.
+  int a;
+  {
+    int b;
+  }
+
+  int z = ({ // expected-warning {{use of GNU statement expression extension}}
+	// Test that we can parse declarations within a GNU statement expression.
+	int w = 12;
+	w;
+  });
+
+  // Test that we diagnose declarations where a statement is required.
+  // See GH92775.
+  if (1)
+    int x; // expected-error {{expected expression}}
+  for (;;)
+    int c; // expected-error {{expected expression}}
+
+  label:
+    int y; // expected-warning {{label followed by a declaration is a C23 extension}}
+
+  // Test that lookup works as expected.
+  (void)a;
+  (void)v;
+  (void)z;
+  (void)b; // expected-error {{use of undeclared identifier 'b'}}
+  (void)w; // expected-error {{use of undeclared identifier 'w'}}
+  (void)x; // expected-error {{use of undeclared identifier 'x'}}
+  (void)c; // expected-error {{use of undeclared identifier 'c'}}
+  (void)y;
+}
+

>From f59942f6c3ef3e7b8ae6f977238b23a1b7a1f986 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 21 May 2024 08:42:04 -0400
Subject: [PATCH 4/4] Add release note

---
 clang/docs/ReleaseNotes.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81e9d0423f96a..a47252c536c0f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -589,6 +589,9 @@ Bug Fixes in This Version
 - ``__is_array`` and ``__is_bounded_array`` no longer return ``true`` for
   zero-sized arrays. Fixes (#GH54705).
 
+- Correctly reject declarations where a statement is required in C.
+  Fixes #GH92775
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 



More information about the cfe-commits mailing list