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_ish...@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 +@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> + >From a3f43da776257218452a0fd12f9371653fa5433d Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@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 -@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> - 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits