Page MenuHomePhabricator

[clang][modules] Umbrella with missing submodule: unify implicit & explicit
Needs ReviewPublic

Authored by jansvoboda11 on Nov 12 2021, 7:32 AM.

Details

Summary

Submodules not covered by existing umbrella header are being imported as textual with implicit modules. Explicit modules error out, since they still expect to be provided a module file that covers such header.

Unify the behavior of explicit modules with implicit modules by allowing such cases and treating them as textual as well.

Depends on D113761.

Diff Detail

Event Timeline

jansvoboda11 requested review of this revision.Nov 12 2021, 7:32 AM
jansvoboda11 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 7:32 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
vsapsai added inline comments.Nov 12 2021, 1:27 PM
clang/test/Modules/missing-submodule.m
3

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.

jansvoboda11 added inline comments.Nov 15 2021, 3:51 AM
clang/test/Modules/missing-submodule.m
3

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.

Add -fno-implicit-modules to test case to make it more obvious.