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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits