[clang] b321738 - [clang][parser] Fix namespace dropping after malformed declarations

[Aaron Ballman via cfe-commits]

Author: Alejandro Álvarez Ayllón
Date: 2023-05-15T07:39:58-04:00
New Revision: b321738f71259d138c9b2002944eb65f099ec2a6

URL: https://github.com/llvm/llvm-project/commit/b321738f71259d138c9b2002944eb65f099ec2a6
DIFF: https://github.com/llvm/llvm-project/commit/b321738f71259d138c9b2002944eb65f099ec2a6.diff

LOG: [clang][parser] Fix namespace dropping after malformed declarations

* After a malformed top-level declaration
* After a malformed templated class method declaration

In both cases, when there is a malformed declaration, any following
namespace is dropped from the AST. This can trigger a cascade of
confusing diagnostics that may hide the original error. An example:
```
// Start #include "SomeFile.h"
template <class T>
void Foo<T>::Bar(void* aRawPtr) {
    (void)(aRawPtr);
}
// End #include "SomeFile.h"

int main() {}
```
We get the original error, plus 19 others from the standard library.
With this patch, we only get the original error.

clangd can also benefit from this patch, as namespaces following the
malformed declaration is now preserved. i.e.
```

MACRO_FROM_MISSING_INCLUDE("X")

namespace my_namespace {
    //...
}
```
Before this patch, my_namespace is not visible for auto-completion.

Differential Revision: https://reviews.llvm.org/D150258

Added: 
    clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp
    clang/test/Parser/cxx-template-recovery.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Parse/ParseDecl.cpp
    clang/lib/Parse/ParseTemplate.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bb2178bfa5f96..00edb842ae715 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -449,6 +449,8 @@ Bug Fixes to C++ Support
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Preserve ``namespace`` definitions that follow malformed declarations.
+
 Miscellaneous Bug Fixes
 ^^^^^^^^^^^^^^^^^^^^^^^
 

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 2bbe2ba82139d..92fa7d8a8a759 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2176,13 +2176,16 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
           return Actions.ConvertDeclToDeclGroup(TheDecl);
         }
 
-        if (isDeclarationSpecifier(ImplicitTypenameContext::No)) {
-          // If there is an invalid declaration specifier right after the
-          // function prototype, then we must be in a missing semicolon case
-          // where this isn't actually a body.  Just fall through into the code
-          // that handles it as a prototype, and let the top-level code handle
-          // the erroneous declspec where it would otherwise expect a comma or
-          // semicolon.
+        if (isDeclarationSpecifier(ImplicitTypenameContext::No) ||
+            Tok.is(tok::kw_namespace)) {
+          // If there is an invalid declaration specifier or a namespace
+          // definition right after the function prototype, then we must be in a
+          // missing semicolon case where this isn't actually a body.  Just fall
+          // through into the code that handles it as a prototype, and let the
+          // top-level code handle the erroneous declspec where it would
+          // otherwise expect a comma or semicolon. Note that
+          // isDeclarationSpecifier already covers 'inline namespace', since
+          // 'inline' can be a declaration specifier.
         } else {
           Diag(Tok, diag::err_expected_fn_body);
           SkipUntil(tok::semi);
@@ -2303,10 +2306,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
     // Okay, there was no semicolon and one was expected.  If we see a
     // declaration specifier, just assume it was missing and continue parsing.
     // Otherwise things are very confused and we skip to recover.
-    if (!isDeclarationSpecifier(ImplicitTypenameContext::No)) {
-      SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
-      TryConsumeToken(tok::semi);
-    }
+    if (!isDeclarationSpecifier(ImplicitTypenameContext::No))
+      SkipMalformedDecl();
   }
 
   return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, DeclsInGroup);

diff  --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index 72df117c123e6..964d985b6b6e6 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -275,10 +275,7 @@ Decl *Parser::ParseSingleDeclarationAfterTemplate(
 
   // Error parsing the declarator?
   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;
   }
 

diff  --git a/clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp b/clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp
new file mode 100644
index 0000000000000..6d2dc606ef642
--- /dev/null
+++ b/clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+UNKNOWN_MACRO_1("z", 1) // expected-error {{a type specifier is required for all declarations}}
+// expected-error at -1 {{expected ';' after top level declarator}}
+
+namespace foo {
+  class bar {};
+}
+
+int variable = 0; // ok
+foo::bar something; // ok
+
+UNKNOWN_MACRO_2(void) // expected-error {{a type specifier is required for all declarations}}
+// expected-error at -1 {{expected ';' after top level declarator}}
+
+namespace baz {
+  using Bool = bool;
+}
+
+int variable2 = 2; // ok
+const baz::Bool flag = false;  // ok

diff  --git a/clang/test/Parser/cxx-template-recovery.cpp b/clang/test/Parser/cxx-template-recovery.cpp
new file mode 100644
index 0000000000000..e22f6c55e3f0c
--- /dev/null
+++ b/clang/test/Parser/cxx-template-recovery.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template <class T>
+void Foo<T>::Bar(void* aRawPtr) { // expected-error {{no template named 'Foo'}}
+    (void)(aRawPtr);
+}
+
+namespace baz {
+  class klass {};
+}
+
+int *variable = 0; // ok
+const baz::klass object; // ok