[clang-scan-deps] Ignore import/include directives with missing filenames #99520
Conversation
filenames Previously source input like `#import ` resulted in infinite calls append the same token into `CurDirTokens`. This patch now ignores those directive lines if they won't actually end up being compiled. (e.g. macro guarded)
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Cyndy Ishida (cyndyishida) ChangesPreviously source input like resolves: rdar://121247565 Full diff: https://github.com/llvm/llvm-project/pull/99520.diff 2 Files Affected:
diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 57652be8244b4..4bb57a0751d42 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -88,8 +88,8 @@ struct Scanner {
[[nodiscard]] dependency_directives_scan::Token &
lexToken(const char *&First, const char *const End);
- dependency_directives_scan::Token &lexIncludeFilename(const char *&First,
- const char *const End);
+ [[nodiscard]] dependency_directives_scan::Token &
+ lexIncludeFilename(const char *&First, const char *const End);
void skipLine(const char *&First, const char *const End);
void skipDirective(StringRef Name, const char *&First, const char *const End);
@@ -544,7 +544,7 @@ Scanner::lexIncludeFilename(const char *&First, const char *const End) {
void Scanner::lexPPDirectiveBody(const char *&First, const char *const End) {
while (true) {
const dependency_directives_scan::Token &Tok = lexToken(First, End);
- if (Tok.is(tok::eod))
+ if (Tok.is(tok::eod) || Tok.is(tok::eof))
break;
}
}
@@ -901,7 +901,11 @@ bool Scanner::lexPPLine(const char *&First, const char *const End) {
case pp___include_macros:
case pp_include_next:
case pp_import:
- lexIncludeFilename(First, End);
+ // Ignore missing filenames in include or import directives.
+ if (lexIncludeFilename(First, End).is(tok::eod)) {
+ skipDirective(Id, First, End);
+ return true;
+ }
break;
default:
break;
diff --git a/clang/test/ClangScanDeps/missing-import-filenames.m b/clang/test/ClangScanDeps/missing-import-filenames.m
new file mode 100644
index 0000000000000..169060614af80
--- /dev/null
+++ b/clang/test/ClangScanDeps/missing-import-filenames.m
@@ -0,0 +1,98 @@
+// This test checks that import directives with missing filenames are ignored when scanning but will result
+// in compile time errors if they need to be parsed.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- tu.m
+#import "zeroth.h"
+
+//--- zeroth/module.modulemap
+module zeroth { header "zeroth.h" }
+//--- zeroth/zeroth.h
+#ifdef BAD_IMPORT
+@import;
+#import
+#endif
+@import first;
+
+//--- first/module.modulemap
+module first { header "first.h" }
+//--- first/first.h
+
+// RUN: clang-scan-deps -format experimental-full -o %t/result.json \
+// RUN: -- %clang -fmodules -fmodules-cache-path=%t/cache -I %t/zeroth -I %t/first -I %t/second -c %t/tu.m -o %t/tu.o
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: {
+// CHECK-NEXT: "modules": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/first/first.h",
+// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "link-libraries": [],
+// CHECK-NEXT: "name": "first"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "first"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/zeroth/module.modulemap",
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/first/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/zeroth/module.modulemap",
+// CHECK-NEXT: "[[PREFIX]]/zeroth/zeroth.h"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "link-libraries": [],
+// CHECK-NEXT: "name": "zeroth"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "translation-units": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "commands": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "clang-context-hash": "{{.*}}",
+// CHECK-NEXT: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "context-hash": "{{.*}}",
+// CHECK-NEXT: "module-name": "zeroth"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "command-line": [
+// CHECK: ],
+// CHECK-NEXT: "executable": "{{.*}}",
+// CHECK-NEXT: "file-deps": [
+// CHECK-NEXT: "[[PREFIX]]/tu.m"
+// CHECK-NEXT: ],
+// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.m"
+// CHECK-NEXT: }
+// CHECK: ]
+// CHECK: }
+// CHECK: ]
+// CHECK: }
+
+// RUN: %deps-to-rsp --module-name=first %t/result.json > %t/first.cc1.rsp
+// RUN: %deps-to-rsp --module-name=zeroth %t/result.json > %t/zeroth.cc1.rsp
+// RUN: %clang @%t/first.cc1.rsp
+// RUN: %clang @%t/zeroth.cc1.rsp
+
+// Validate that invalid directive that will be parsed results clang error.
+// RUN: not clang-scan-deps -format experimental-full -o %t/result_with_bad_imports.json \
+// RUN: -- %clang -fmodules -fmodules-cache-path=%t/diff_cache -I %t/zeroth -I %t/first \
+// RUN: -I %t/second -c %t/tu.m -o %t/tu.o -DBAD_IMPORT=1 2>&1 | FileCheck %s --check-prefix=BAD_IMPORT
+
+// BAD_IMPORT: error: expected a module name after 'import'
+// BAD_IMPORT: error: expected "FILENAME" or <FILENAME>
+
|
|
This is similar feedback that I gave for #97654, could you test the change with a unit test in For reference see the test change in that PR. |
I switched to the unit test, though I didn't figure out how to capture the same sort of round-trip testing as with the lit test. |
Are you not seeing the same "infinite calls append the same token into CurDirTokens" issue via the unit test? |
Oh yeah, that part is sufficiently exercised in the unit test. |
…#99520) Previously source input like `#import ` resulted in infinite calls append the same token into `CurDirTokens`. This patch now ignores those directive lines if they won't actually end up being compiled. (e.g. macro guarded) resolves: rdar://121247565
Previously source input like
#importresulted in infinite calls append the same token intoCurDirTokens. This patch now ignores those directive lines if they won't actually end up being compiled. (e.g. macro guarded)resolves: rdar://121247565