[clang] [clang-scan-deps] Ignore import/include directives with missing filenames (PR #99520)
Cyndy Ishida via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 10:32:24 PDT 2024
https://github.com/cyndyishida updated https://github.com/llvm/llvm-project/pull/99520
>From bcddefdce00a1e15f29181bc92eab86098a8b328 Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Thu, 18 Jul 2024 08:56:24 -0700
Subject: [PATCH 1/2] [clang-scan-deps] Ignore import/include directives with
missing 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)
---
clang/lib/Lex/DependencyDirectivesScanner.cpp | 12 ++-
.../ClangScanDeps/missing-import-filenames.m | 98 +++++++++++++++++++
2 files changed, 106 insertions(+), 4 deletions(-)
create mode 100644 clang/test/ClangScanDeps/missing-import-filenames.m
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
+ at import;
+#import
+#endif
+ at 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>
+
>From a3f43da776257218452a0fd12f9371653fa5433d Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Mon, 22 Jul 2024 10:30:19 -0700
Subject: [PATCH 2/2] Switch to unit test
---
.../ClangScanDeps/missing-import-filenames.m | 98 -------------------
.../Lex/DependencyDirectivesScannerTest.cpp | 11 +++
2 files changed, 11 insertions(+), 98 deletions(-)
delete mode 100644 clang/test/ClangScanDeps/missing-import-filenames.m
diff --git a/clang/test/ClangScanDeps/missing-import-filenames.m b/clang/test/ClangScanDeps/missing-import-filenames.m
deleted file mode 100644
index 169060614af80..0000000000000
--- a/clang/test/ClangScanDeps/missing-import-filenames.m
+++ /dev/null
@@ -1,98 +0,0 @@
-// 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
- at import;
-#import
-#endif
- at 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>
-
diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 94af9688a96e2..91f1dca2aadd5 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -650,6 +650,17 @@ TEST(MinimizeSourceToDependencyDirectivesTest, AtImport) {
EXPECT_STREQ("@import A.B;\n", Out.data());
}
+TEST(MinimizeSourceToDependencyDirectivesTest, EmptyIncludesAndImports) {
+ SmallVector<char, 128> Out;
+
+ ASSERT_TRUE(minimizeSourceToDependencyDirectives("#import\n", Out));
+ ASSERT_TRUE(minimizeSourceToDependencyDirectives("#include\n", Out));
+ ASSERT_TRUE(minimizeSourceToDependencyDirectives("#ifdef A\n"
+ "#import \n"
+ "#endif\n",
+ Out));
+}
+
TEST(MinimizeSourceToDependencyDirectivesTest, AtImportFailures) {
SmallVector<char, 128> Out;
More information about the cfe-commits
mailing list