[clang] [clang][DependencyScanning] Reset options generated for named module compilations. (PR #161486)

[Naveen Seth Hanig via cfe-commits]

================
@@ -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