[clang] Preserve externs following broken declarations
#161641
Conversation
Treat them as namespaces: if they are at the beginning of the line, they are likely a good recovery point. CPP-4478
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource) ChangesTreat them as namespaces: if they are at the beginning of the line, they are likely a good recovery point. For instance, in 1.3.0
extern "C" {
extern int foo();
extern "C++" {
namespace bar {
void baz();
};
}
}
namespace {}Everything until CPP-4478 Full diff: https://github.com/llvm/llvm-project/pull/161641.diff 2 Files Affected:
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 22c01c4e371f3..d6cd7eb8c2c3d 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2083,6 +2083,9 @@ void Parser::SkipMalformedDecl() {
return;
break;
+ case tok::kw_extern:
+ // 'extern' at the start of a line is almost certainly a good
+ // place to pick back up parsing
case tok::kw_namespace:
// 'namespace' at the start of a line is almost certainly a good
// place to pick back up parsing, except in an Objective-C
diff --git a/clang/test/Parser/recovery-after-expected-unqualified-id.cpp b/clang/test/Parser/recovery-after-expected-unqualified-id.cpp
new file mode 100644
index 0000000000000..8019b46df1e7b
--- /dev/null
+++ b/clang/test/Parser/recovery-after-expected-unqualified-id.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify %s
+
+3.2 // expected-error {{expected unqualified-id}}
+
+extern "C" {
+ typedef int Int;
+}
+
+Int foo(); // Ok
|
|
Can you add a changelog entry? LGTM otherwise |
There was a problem hiding this comment.
Given the summary explicitly mentions that previously things went missing from the AST, should we not have a test that verifies that this does not happen anymore? CC @erichkeane
AST Dump tests are the easiest way, you don't need a valid TU to do so, but can do the ast-dump whenever. I'd love to see that sort of test added here. |
Treat them as namespaces: if they are at the beginning of the line, they
are likely a good recovery point.
For instance, in
```cpp
1.3.0
extern "C" {
extern int foo();
extern "C++" {
namespace bar {
void baz();
};
}
}
namespace {}
```
Everything until `namespace`... is gone from the AST. Headers (like
libc's C++ `math.h`) can be included from an `extern "C"` context, and
they do an `extern "C++"` back again before including C++ headers (like
`__type_traits`).
However, a malformed declaration just before the include (as the orphan
`1.3.0` in the example) causes everything from these standard headers to
go missing. This patch updates the heuristic to try to recover from the
first `extern` keyword seen, pretty much as it is done for `namespace`.
CPP-4478
That was the point of this line. Before it would complain about unknown type Int, but I see that an AST dump would have been more explicit about what is going on. |
Treat them as namespaces: if they are at the beginning of the line, they are likely a good recovery point.
For instance, in
Everything until
namespace... is gone from the AST. Headers (like libc's C++math.h) can be included from anextern "C"context, and they do anextern "C++"back again before including C++ headers (like__type_traits).However, a malformed declaration just before the include (as the orphan
1.3.0in the example) causes everything from these standard headers to go missing. This patch updates the heuristic to try to recover from the firstexternkeyword seen, pretty much as it is done fornamespace.CPP-4478