================
@@ -263,6 +263,10 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) {
   // units.
   CI.getFrontendOpts().Inputs.clear();
   CI.getFrontendOpts().OutputFile.clear();
+  CI.getFrontendOpts().GenReducedBMI = false;
+  CI.getFrontendOpts().ModuleOutputPath.clear();
+  CI.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = false;
+  CI.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = false;
----------------
naveen-seth wrote:

> Can you clarify why you're now also resetting ModulesSkipHeaderSearchPaths 
> and ModulesSkipDiagnosticOptions for the build command lines here in 
> ModuleDepCollector.cpp? AFAIK these are only used within the scanner to 
> increase performance - they don't make it into the build invocations, nor 
> should the driver invocations that scanner takes as an input have them 
> enabled.

I believe that they are also set for all compiler invocations which generate a 
BMI (here):

https://github.com/llvm/llvm-project/blob/36dc2a941f531d6b5662f2ad2c11e7264e5d9622/clang/lib/Frontend/CompilerInvocation.cpp#L5016-L5023

and then also make it into the build invocations: [gist with debug 
print](https://gist.github.com/naveen-seth/82eab9977618b8ba66480a769571d315)

> Your change in DependencyScannerImpl.cpp looks good, but it'd be good to have 
> a test that checks the scanner itself doesn't produce more than one PCM 
> variant in this scenario.

Do all the the scanning modules appear in the output of `clang-scan-deps 
-format=full-experimental`? If so, this part of the test should check this.
https://github.com/naveen-seth/llvm-project/blob/147a39de68a981a51de7d9333f7b476139c43599/clang/test/ClangScanDeps/modules-context-hash-from-named-module.cpp#L39C1-L51C20

I’m not too familiar with the internals of the scanning tooling and scanning 
modules yet. If not by adding a regression test, how could I check this? Where 
would the logic that controls this behavior live?

> Is this assumption correct?

I believe so, but maybe some else with more knowledge of this would have to 
confirm.

Thanks also for reviewing!

https://github.com/llvm/llvm-project/pull/161486
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to