[PATCH] D113775: [clang][modules] Umbrella with missing submodule: unify implicit & explicit

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.


================
Comment at: clang/test/Modules/missing-submodule.m:3
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps                         -F %S/Inputs %s -verify
 #include <Module/NotInModule.h> // expected-warning{{missing submodule 'Module.NotInModule'}}
----------------
vsapsai wrote:
> Haven't checked the implementation but does the test cover any new behavior? Based on description it should test explicit modules. But the added line is testing `-fimplicit-module-maps`, just using the system-wide shared modules cache.
The system-wide shared modules cache flag is being generated by the driver. Here I'm invoking `-cc1` without `-fmodules-cache-path=`, which I think effectively disables implicit modules. I'll add `-fno-implicit-modules` to make this more obvious. So yes, this test does cover the new behavior.

However, I spent some time investigating why we're using `-fimplicit-module-maps` in the command lines for explicit build produced by the dependency scanner and came to the conclusion this is due to inferred modules. I tried to fix the underlying issue in D113880. If we don't pass `-fimplicit-module-maps` to Clang, we don't hit the case this patch solves. I'm not sure whether I should abandon this patch in favor of D113880, or commit it anyway, since it still might be useful for some.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113775