This provides an implementation for the changes outlined in section 4.1 of P0731r0, which clarifies the intended behaviour regarding implementation units being able to see imports made within their corresponding interface unit.
I've been investigating an issue regarding visibility during code synthesis, and I noticed that this patch half fixes it. I've added the rest of the fix since it's related to what's going on here, and I guess this is now a "correctly handle imports from an interface unit" patch.
I believe this fixes this bug posted on the LLVM bug tracker.
@boris if this is still okay with you after seeing the changes I've made, would you mind accepting the revision so I can commit this without Phabricator shouting at me?
I don't think it will be wise for me to accept this revision since I can't claim to have good understanding of Clang's internal modules model. I think it will be wise to have Richard take a look.
This comment (not sure about the code) sounds wrong to me: names from the interface unit before the module purview (I assume this is what "global module fragment" refers to) should not be visible in the implementation units.
Yes, the documentation comment for this function (in the header) notes that things visible to the interface unit aren’t necessarily visible to the implementation units.
There’re a few reasons why I chose for the function to count modules that’re only visible from the interface unit:
For completeness, you should also call getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, Loc); here. (It turns out to not actually matter right now because ModulesTS implies LocalSubmoduleVisibility, under which makeModuleVisible happens to be a no-op.)
This is not appropriate; generally whether we serialize to an AST file should be treated as orthogonal to whether we're in / importing a module.
The right check here is probably getLangOpts().getCompilingModule() == CMK_ModuleInterface.