[PATCH] D132430: [clang][modules] Track affecting modules

Jan Svoboda via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 23 14:29:30 PDT 2022


jansvoboda11 added inline comments.


================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:311
+    if (!MDC.isPrebuiltModule(M))
+      DirectModularDeps.insert(M);
+
----------------
benlangmuir wrote:
> If using eager loading, this will cause us to load the module, right? Does that behaviour match the implicit build?
That's correct. The implicit build does the same.


================
Comment at: clang/test/ClangScanDeps/modules-incomplete-umbrella.c:82
+// CHECK_TU-NEXT:       "command-line": [
+// CHECK_TU:            ],
+// CHECK_TU-NEXT:       "file-deps": [
----------------
benlangmuir wrote:
> The test should probably cover the imports in the command-lines.
Most `-fmodule-file=` checks were removed in D132066, since these are likely to change in the future, and I'm okay with relying on `"clang-module-deps"`. I can see the argument that in this particular case the dependencies are not ordinary imports, but somewhat of an edge case, so I'm fine with adding the checks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132430/new/

https://reviews.llvm.org/D132430



More information about the cfe-commits mailing list