[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 27 16:42:42 PDT 2018
rsmith added inline comments.
================
Comment at: lib/Sema/SemaDecl.cpp:16135
+ for (Module *ImportedModule : Mod->Imports)
+ VisibleModules.setVisible(ImportedModule, ModuleLoc);
+ }
----------------
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.)
================
Comment at: lib/Sema/SemaDecl.cpp:16185-16194
+ if (getLangOpts().ModulesTS) {
+ Module *CurrentModule = getCurrentModule();
+ assert(CurrentModule && "Expected to be in a module scope");
+
+ // If the current module has been loaded from disk, then this is an
+ // implementation unit and hence we shouldn't modify the module.
+ // FIXME: Is that a hacky assumption? We can't just check
----------------
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`.
https://reviews.llvm.org/D40443
More information about the cfe-commits
mailing list