[clang] c68f247 - [clang-scan-deps] ignore top-level module dependencies that aren't actually imported

Alex Lorenz via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 20 11:11:35 PDT 2021


Author: Alex Lorenz
Date: 2021-07-20T11:11:28-07:00
New Revision: c68f247275eed94a4f4b97ad53b4d599acfd181a

URL: https://github.com/llvm/llvm-project/commit/c68f247275eed94a4f4b97ad53b4d599acfd181a
DIFF: https://github.com/llvm/llvm-project/commit/c68f247275eed94a4f4b97ad53b4d599acfd181a.diff

LOG: [clang-scan-deps] ignore top-level module dependencies that aren't actually imported

Whenever -fmodule-name=top_level_module name is parsed, and clang actually tries to
import top_level_module, the headers are imported textually and the module isn't actually
built. However, the dependency scanner could still record it as a potential dependency
if the module was reimported and thus recorded by the preprocessor callbacks.
This change avoids collecting this kind of module as a dependency by verifying that we don't
collect top level modules without actual PCM files.

Differential Revision: https://reviews.llvm.org/D106100

Added: 
    clang/test/ClangScanDeps/Inputs/header3.h
    clang/test/ClangScanDeps/Inputs/module_fmodule_name_cdb.json
    clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m

Modified: 
    clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
    clang/test/ClangScanDeps/Inputs/module.modulemap

Removed: 
    


################################################################################
diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 690bd6d476e58..88cee63c98aa5 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -170,8 +170,15 @@ void ModuleDepCollectorPP::EndOfMainFile() {
   if (!Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
     MDC.FileDeps.push_back(Instance.getPreprocessorOpts().ImplicitPCHInclude);
 
-  for (const Module *M : DirectModularDeps)
+  for (const Module *M : DirectModularDeps) {
+    // A top-level module might not be actually imported as a module when
+    // -fmodule-name is used to compile a translation unit that imports this
+    // module. In that case it can be skipped. The appropriate header
+    // dependencies will still be reported as expected.
+    if (!M->getASTFile())
+      continue;
     handleTopLevelModule(M);
+  }
 
   MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
 

diff  --git a/clang/test/ClangScanDeps/Inputs/header3.h b/clang/test/ClangScanDeps/Inputs/header3.h
new file mode 100644
index 0000000000000..5bd7e64bcfe36
--- /dev/null
+++ b/clang/test/ClangScanDeps/Inputs/header3.h
@@ -0,0 +1,2 @@
+// import another header from the same module
+#include "header.h"

diff  --git a/clang/test/ClangScanDeps/Inputs/module.modulemap b/clang/test/ClangScanDeps/Inputs/module.modulemap
index 13171f29d69af..ae8534247c150 100644
--- a/clang/test/ClangScanDeps/Inputs/module.modulemap
+++ b/clang/test/ClangScanDeps/Inputs/module.modulemap
@@ -5,3 +5,8 @@ module header1 {
 module header2 {
     header "header2.h"
 }
+
+module header3 {
+  header "header.h"
+  header "header3.h"
+}

diff  --git a/clang/test/ClangScanDeps/Inputs/module_fmodule_name_cdb.json b/clang/test/ClangScanDeps/Inputs/module_fmodule_name_cdb.json
new file mode 100644
index 0000000000000..dec425171875a
--- /dev/null
+++ b/clang/test/ClangScanDeps/Inputs/module_fmodule_name_cdb.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/modules-fmodule-name-no-module-built.m -IInputs -D INCLUDE_HEADER2 -MD -MF DIR/modules_cdb.d -fmodules -fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fmodule-name=header3 -fimplicit-module-maps",
+  "file": "DIR/modules-fmodule-name-no-module-built.m"
+}
+]

diff  --git a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
new file mode 100644
index 0000000000000..e30e312b6a8da
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
@@ -0,0 +1,59 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules-fmodule-name-no-module-built.m
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/header3.h %t.dir/Inputs/header3.h
+// RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/module_fmodule_name_cdb.json > %t.cdb
+
+// RUN: echo %t.dir > %t.result
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -format experimental-full \
+// RUN:   -generate-modules-path-args -mode preprocess-minimized-sources >> %t.result
+// RUN: cat %t.result | sed 's:\\\\\?:/:g' | FileCheck --check-prefixes=CHECK %s
+
+#import "header3.h"
+#import "header.h"
+
+// CHECK: [[PREFIX:.*]]
+// CHECK-NEXT: {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": []
+// CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/Inputs/module.modulemap",
+// CHECK-NEXT:       "command-line": [
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "[[HASH_H2:[A-Z0-9]+]]",
+// CHECK-NEXT:       "file-deps": [
+// CHECK-NEXT:         "[[PREFIX]]/Inputs/header2.h",
+// CHECK-NEXT:         "[[PREFIX]]/Inputs/module.modulemap"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "name": "header2"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
+// CHECK-NEXT:       "clang-module-deps": [
+// CHECK-NEXT:         {
+// CHECK-NEXT:           "context-hash": "[[HASH_H2]]",
+// CHECK-NEXT:           "module-name": "header2"
+// CHECK-NEXT:         }
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "command-line": [
+// CHECK-NEXT:         "-fno-implicit-modules"
+// CHECK-NEXT:         "-fno-implicit-module-maps"
+// CHECK-NEXT:         "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2]]/header2-{{[A-Z0-9]+}}.pcm"
+// CHECK-NEXT:         "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "file-deps": [
+// CHECK-NEXT:         "[[PREFIX]]/modules-fmodule-name-no-module-built.m"
+// CHECK-NEXT:         "[[PREFIX]]/Inputs/header3.h"
+// CHECK-NEXT:         "[[PREFIX]]/Inputs/header.h"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "input-file": "[[PREFIX]]/modules-fmodule-name-no-module-built.m"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }


        


More information about the cfe-commits mailing list