[PATCH] D150258: [clang][parser] Fix namespace dropping after malformed declarations

[Alejandro Álvarez Ayllón via Phabricator via cfe-commits]

alejandro-alvarez-sonarsource created this revision.
alejandro-alvarez-sonarsource added a reviewer: aaron.ballman.
alejandro-alvarez-sonarsource added a project: clang.
Herald added a subscriber: kadircet.
Herald added a project: All.
alejandro-alvarez-sonarsource requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.

- 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"
  
  #include <iostream>
  
  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.

#pragma once

MACRO_FROM_MISSING_INCLUDE("X")

namespace my_namespace {

  //...

}

  Before this patch, `my_namespace` is not visible for auto-completion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150258

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp
  clang/test/Parser/cxx-template-recovery.cpp


Index: clang/test/Parser/cxx-template-recovery.cpp
===================================================================
--- /dev/null
+++ clang/test/Parser/cxx-template-recovery.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -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
Index: clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp
===================================================================
--- /dev/null
+++ clang/test/Parser/cxx-namespace-after-missing-semicolon.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -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
Index: clang/lib/Parse/ParseTemplate.cpp
===================================================================
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -275,10 +275,7 @@
 
   // 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;
   }
 
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2176,7 +2176,8 @@
           return Actions.ConvertDeclToDeclGroup(TheDecl);
         }
 
-        if (isDeclarationSpecifier(ImplicitTypenameContext::No)) {
+        if (isDeclarationSpecifier(ImplicitTypenameContext::No) ||
+            Tok.is(tok::kw_namespace)) {
           // 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
@@ -2304,8 +2305,7 @@
     // 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);
+      SkipMalformedDecl();
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150258.520935.patch
Type: text/x-patch
Size: 2796 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230510/e4b0aa83/attachment-0001.bin>