Skip to content

[C] Disallow declarations where a statement is required #92908

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

Merged
merged 9 commits into from
May 28, 2024
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a little mysterious, I only see that we check it but it is not obvious how it gets set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shafik L481 below

/// 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)) ||
Comment on lines +247 to +250
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a standard quote?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added one, let me know if you're happy with it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks (somehow i did not get notified)

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
Loading