Skip to content

Commit

Permalink
[C] Disallow declarations where a statement is required (llvm#92908)
Browse files Browse the repository at this point in the history
This fixes a regression introduced in
8bd06d5 where Clang began to accept a
declaration where a statement is required. e.g.,
```
if (1)
  int x; // Previously accepted, now properly rejected
```

Fixes llvm#92775
  • Loading branch information
AaronBallman authored May 28, 2024
1 parent d33864d commit 5901d40
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 7 deletions.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
9 changes: 6 additions & 3 deletions clang/include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};
Expand Down
10 changes: 9 additions & 1 deletion clang/lib/Parse/ParseStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,15 @@ 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)) ||
// In C, the grammar production for statement (C23 6.8.1p1) does not allow
// for declarations, which is different from C++ (C++23 [stmt.pre]p1). So
// in C++, we always allow a declaration, but in C we need to check whether
// we're in a statement context that allows declarations. e.g., in C, the
// following is invalid: if (1) int x;
if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
(StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
ParsedStmtContext()) &&
((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
isDeclarationStatement())) {
SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
DeclGroupPtrTy Decl;
Expand Down
3 changes: 2 additions & 1 deletion clang/test/C/C99/block-scopes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
39 changes: 39 additions & 0 deletions clang/test/Parser/decls.c
Original file line number Diff line number Diff line change
@@ -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;
}

6 changes: 4 additions & 2 deletions clang/test/SemaOpenACC/parallel-loc-and-stmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit 5901d40

Please sign in to comment.