rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land.
Thanks, this looks great. A couple of minor suggestions. ================ Comment at: clang/include/clang/Lex/ModuleMap.h:542-543 + /// unit's Module until later, because we don't know what it will be called + /// usually. (NOTE: See https://eel.is/c++draft/module.unit#7.2 for the case + /// we could know its parent.) + Module *createGlobalModuleFragmentForModuleUnit(SourceLocation Loc, ---------------- We generally refer to sections of the C++ standard by name not by URL. ================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16158 + PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true); + D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::Visible); + D->setLocalOwningModule(GlobalModule); ---------------- I think this should be `ModulePrivate` -- such a global module fragment should not be visible to importers of this module. (I think it probably doesn't matter in practice since visibility is generally monotonically increasing while compiling a module unit, but `ModulePrivate` seems more correct in principle.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110215/new/ https://reviews.llvm.org/D110215 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits