[PATCH] D150258: [clang][parser] Fix namespace dropping after malformed declarations
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu May 11 12:39:03 PDT 2023
aaron.ballman added a comment.
Thank you for working on this! FWIW, I verified that the precommit CI failures are unrelated to this patch.
Can you add a release note to `clang/docs/ReleaseNotes.rst` about the fix?
================
Comment at: clang/lib/Parse/ParseDecl.cpp:2179-2180
- if (isDeclarationSpecifier(ImplicitTypenameContext::No)) {
+ if (isDeclarationSpecifier(ImplicitTypenameContext::No) ||
+ Tok.is(tok::kw_namespace)) {
// If there is an invalid declaration specifier right after the
----------------
It'd help to update the comment below since it doesn't mention why namespaces are handled specially.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:2306-2310
// Otherwise things are very confused and we skip to recover.
if (!isDeclarationSpecifier(ImplicitTypenameContext::No)) {
- SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
- TryConsumeToken(tok::semi);
+ SkipMalformedDecl();
}
}
----------------
Changes for our coding style.
There seems to be some unfortunate interplay here though. Consider:
```
void bar() namespace foo { int i; }
int main() {
foo::i = 12;
}
```
Calling `SkipMalformedDecl()` changes the behavior for this test case because we don't recover as well. With your patch applied, this gives us two diagnostics:
```
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:1:11: error: expected ';' after top level declarator
void bar() namespace foo { int i; }
^
;
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:4:3: error: use of undeclared identifier 'foo'
foo::i = 12;
^
2 errors generated.
```
If the `namespace` keyword is on its own line, then we recover gracefully and don't emit the "use of undeclared identifier" warning.
While this is technically a regression in behavior for this test case, I think the overall changes are still an improvement. I suspect not a whole lot of code puts `namespace` somewhere other than the start of a line (same for `inline namespace` which has the same behavior with your patch).
================
Comment at: clang/lib/Parse/ParseTemplate.cpp:278
if (!DeclaratorInfo.hasName()) {
- // If so, skip until the semi-colon or a }.
- SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
- if (Tok.is(tok::semi))
- ConsumeToken();
+ SkipMalformedDecl();
return nullptr;
----------------
We'll have the same sort of recovery issues here unless the namespace is on its own line, but that also seems like a highly unlikely scenario.
================
Comment at: clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp:1
+// RUN: %clang_cc1 -verify %s
+
----------------
================
Comment at: clang/test/Parser/cxx-template-recovery.cpp:1
+// RUN: %clang_cc1 -verify %s
+
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150258/new/
https://reviews.llvm.org/D150258
More information about the cfe-commits
mailing list