[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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