[C++20][Modules][3/8] Initial handling for module partitions.

[iains]

Summary

This implements the parsing and recognition of module partition CMIs
and removes the FIXMEs in the parser.

Module partitions are recognised in the base computation of visibility,
however additional amendments to visibility follow in subsequent patches.

Add initial testcases for partition handling, derived from the examples in
Section 10 of the C++20 standard, which identifies what should be accepted
and/or rejected.

[iains]

rebased onto the import state machine.

[iains]

reapply rebase

[iains]

Rebased onto other modules work.

Also had to bump DIAG_SIZE_PARSE, since the existing allocation was full.

[iains]

update after fixing (most) lint suggestions.

The change to DiagnosticIDs.h is not going to be applicable - it would need the
whole file to be rewritten and that change is incidental to this patch series.

[iains]

this is the third patch in an 8 patch series to implement basic module partition support.
The change to clang/include/clang/Basic/DiagnosticIDs.h is required for this to build (at present) but is otherwise incidental.

[ChuanqiXu]

Oh, I did similar thing in our downstream version. I planned to show it in next few weeks. Now it looks I could only give up on it...

[iains]

Update to rebase on changes to parent patches and formatting checks.

while doing this made a check for module partitions more efficient.

[urnathan]

clang/include/clang/Sema/Sema.h

2976–2977

Is NamePath really a better name? You've not consistently changed all Path's to this, and it doesn;t strike me as particularly mnemonic.

clang/lib/Parse/Parser.cpp

2368

perhaps now we should just remove modules-ts references as drive-by cleanups?

[ChuanqiXu]

I think we should add 2 more tests at least:
Import a partition a module purview but we failed to find one. And test reflects the successful case.

clang/include/clang/Sema/Sema.h

2976–2977

I use Path in my implementation.

clang/lib/Sema/SemaDecl.cpp

1611

We lack a comma here.

clang/lib/Sema/SemaModule.cpp

218–219

I prefer to add more words here to tell why we create an interface unit but the above condition is ModuleDeclKind::Implementation.

254

How about: Mod->Kind != Module::ModulePartitionImplementation.

336–348

I think we could change the signature of the lat 2 fields to

ModuleIdPath Path,
bool IsPartition)

I feel this is more simpler.

340

I prefer to remove the support for getLangOpts().ModulesTS on the fly.

348–352

I prefer to move the variable to the following block.

353

I prefer to add an assertion:

assert (ModuleScopes.back().Module.isModulePurview());

359–360

I would like to wrap this one as a method of class Module:

StringRef getPrimaryModuleName(Module *M) {
    if (not partition)
       return M->Name;
    return M->Name.split(":").first;
}

365

We could move this below the if statement.

431–450

I don't find the reason to refactor here. It looks NFC and I think the original form is simpler.

475–478

What does it assert? I don't get the point.

clang/test/Modules/cxx20-import-diagnostics-a.cpp

57 ↗

(On Diff #409201)

What if we don't add this? I think the original is good. So we should add a new test if we feel needed.

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

171

I chose '-' in my implementation since here ModuleName refers to the name in filesystem, right? And I remember '-' is the choice of GCC. So let's keep consistency.

[ChuanqiXu]

clang/test/Modules/cxx20-partition-diagnostics-a.cpp

3–5 ↗

(On Diff #409201)

We could use -fsyntax-only instead of -o /dev/null

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

359–360

Oh, I found it made it in following patches. I think the series of patch is a little bit sparse.

[iains]

address review comments, rebase onto parent patches.

[iains]

some comments remain to be addressed - this update addressed Nathan's primarily - but also cover some of Chuanqi's

clang/include/clang/Sema/Sema.h

2976–2977

I\ve reverted my changes - leaving the variable as "Path"

This is a problem with the code serving two masters ...

... for clang hierachical modules "Path" makes some sense
... for C++20 not really - it is not a pathname (confused me when first reading the code).

however "NamedModuleNameAsPath" is too long ;)

clang/lib/Parse/Parser.cpp

2368

will do this generally now .. given consensus amongst active reviewers.

clang/lib/Sema/SemaModule.cpp

171

This is not my understanding.

ModuleName is the "user-facing" name of the module that is described in the source, and we use this in diagnostics etc.

The translation of that name to an on-disk name can be arbitrary and there are several mechanisms in clang already (e.g. -fmodule-file=A:B=foo.pcm) the module loader uses these to find the module on the basis of its user-facing name where required.

When we have P1184, it is even more important that the interface is supplied with the name that the user will put into the source code.

218–219

I am sorry, perhaps 4/8 should have been squashed with this patch - but this was getting quite large anyway - please see 4/8 for reorganisation of this code.

254

please see 4/8

336–348

please see 4/8

340

agreed in comments, but I think it is outside of my remit to remove the functionality?

365

please see 4/8 and later

475–478

that we should have created a module if the compilation task was to build one. IIRC this was an existing assert .. but I will check.

[urnathan]

clang/lib/Sema/SemaModule.cpp

171

I agree with Iain, we should use ':' is module names here. When mapping a named module to the file system some translation is needed because ':' is not permitted in file names on some filesystems (you know the ones!)

340

We can't just drop the -fmodules-ts option. I think its semantics should be[come] the same as CPlusPlusModules. That's not a change for this patch.

[iains]

address review comments

clang/lib/Sema/SemaModule.cpp

171

(just to expand a little more)

the on-disk name needs to be chosen suitably for the platform and by the user and/or the build system.

When the FE chooses a default filename (which is done in building jobs, not in the Parser of course) it chooses one based on the source filename. It would be most unfortunate if the Parser/Sema needed to understand platform filename rules.

When you do 'clang -module-file-info' (with the existing or updated version) you should see the module name as per the source code, the translation would only apply to the filename itself.

• to answer a later comment:

when we do -fmodule-file=A_B.pcm and A_B.pcm contains a module named A:B the loader notices this pairing when it pre-loads the module, so that when we ask for "A:B" the loader already knows which on-disk file contains. it.

if we use the HeaderSearch mechanisms (when we want to figure out a module-name<=> filename pairing without loading the module) we can use -fmodule-name=A:B=A_B.pcm,

These mechanisms work today, but P1184 is a more flexible mechanism and avoids having massive command lines with many -fmodule-file= cases.

340

that was what I expected.

348–352

again the code is reorganised in 4/8

431–450

it is not NFC:

1. we add the C++20 case (the name is a single identifier)
2. we avoid repeating the check for an empty Path (the first loop would effectively check that too).

clang/test/Modules/cxx20-import-diagnostics-a.cpp

57 ↗

(On Diff #409201)

OK, probably it should have been in from the start - but we will also check these things in the tests taken from the standard, so I don't think it is important here; removed.

clang/test/Modules/cxx20-partition-diagnostics-a.cpp

3–5 ↗

(On Diff #409201)

heh, I usually do this not sure why I did not here; changed.

[urnathan]

thanks

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

171

But what if we need to import multiple modules? In our current workflow, we would like to compile importing module in following style:

clang++ -std=c++20 -fprebuilt-module-path=path1 -fprebuilt-module-path=path2 ... unit.cpp(m) ...

And unit.cpp(m) may look like:

export module M;
import :parta;
import :partb;
import :partc;

And our compiler would lookup M-parta.pcm, M-partb.pcm and M-partc.pcm in the path given in the command line. However, in current implementation, it would lookup M:parta.pcm, M:partb.pcm and M:partc.pcm in the path but it might fail. So my point here is that the current implementation doesn't work well with fprebuilt-module-path. And I don't think we should give up fprebuilt-module-path to support multiple fmodule-file. The fprebuilt-module-path is easier to understand and use for real projects. Even if we support multiple -fmodule-file, people might be frustrating to add many fmodule-file if they need to import many units.

So I really insist on this. Or at least let's add one option, something like -fmodule-partition-separator here.

[iains]

clang/lib/Sema/SemaModule.cpp

171

The semantics of clang hierarchical module names are different from C++20 module names.

C++20 does not specify any relationship between the module name and the filename - that task is for the user, build system (or in the case we have with clang where it can integrate some of that functionality, the lookup mechanism).

out of interest if the user puts:

export module A.B:C.D;

how do you represent that on disk in your implementation?

---

In my understanding, if the user does:

export module A.B:C.D;

clang -cc1 -std=c++20 -emit-module-interface foo.cpp -o xyzzy.pcm

then we should see :

clang -module-file-info xyzzy.pcm

Information for module file 'xyzzy.pcm':
  Module format: raw
  Generated by this Clang: (/repos/llvm-git.git 97fc6544d6a09f161d90417db05674a2b3d2a5fe)
  Module name: A.B:C.D

I think it would be wrong if that printed:
Module name: A.B-C.D

The primary module name is A.B (_not_ A) and there is no implication that we would see
A/B/...

What would happen if we ported clang to some (unknown, unlikely) platform where '-' was not a legal pathname character - we should not expect to have to change Sema for this, right?

---

In summary my understanding is:

• The compiler (Parser/Sema) should know the C++20 module name as it is written by the user.

• You can achieve your objective of using the prebuilt-module-path, but the translation from C++20 module name to on-disk name needs to happen in the lookup, not in artificially changing the module name internally to the compiler.

---

As a general point, there are already (too) many modules command line flags, and we will probably need to add a few more for C++20. I have tried in my implementation to keep separate those for C++20 and those for implicit / hierarchical and modules ts.

IMHO it would be better for the user if modules were modal in the driver - and the driver rejected options that do not apply to the mode in action - but that is not for this patch!

[urnathan]

clang/lib/Sema/SemaModule.cpp

171

Information should be presented to the user in the form the user understands -- M-P is not such a format. As Iain says, the association from module names to file names is arbitrary, and baking (a default?) into Sema is not the right thing.

In some cases the compiler may need to tell the user both. For instance

cannot find module 'M:P' (random/path/M-P.pcm)

Let's not derail the initial pieces of partitions with this complex mapping problem.

[ChuanqiXu]

clang/lib/Sema/SemaModule.cpp

171

I still think the compiler should have a -fmodule-partition-separator option to tell the partition seperator in filesystem to ease the use of C++20 modules. It looks like that the current implementation couldn't handle the case that a module unit imports multiple partitions.

But I agree with there already too many options about clang modules. And I agree with we could handle the problem in later patches. I have no objection to stop the patch landing.

[ChuanqiXu]

I think it is helpful to add some tests from https://reviews.llvm.org/D113972

[iains]

In D118586#3335167, @ChuanqiXu wrote:

I think it is helpful to add some tests from https://reviews.llvm.org/D113972

Actually, in response to your concerns, this morning I wrote a multi-partition test case to add to this (which does pass).

Would you be able to split out the important test cases from D113972 as a separate patch ?

[ChuanqiXu]

In D118586#3335191, @iains wrote:

In D118586#3335167, @ChuanqiXu wrote:

I think it is helpful to add some tests from https://reviews.llvm.org/D113972

Actually, in response to your concerns, this morning I wrote a multi-partition test case to add to this (which does pass).

Thanks. I think it would be helpful to add it here.

Would you be able to split out the important test cases from D113972 as a separate patch ?

I found the meaningful test case mainly comes from the example in spec. e.g., http://eel.is/c++draft/module.unit#4, http://eel.is/c++draft/module.unit#4, http://eel.is/c++draft/module.unit#4

[iains]

In D118586#3335235, @ChuanqiXu wrote:

In D118586#3335191, @iains wrote:

In D118586#3335167, @ChuanqiXu wrote:

I think it is helpful to add some tests from https://reviews.llvm.org/D113972

Actually, in response to your concerns, this morning I wrote a multi-partition test case to add to this (which does pass).

Thanks. I think it would be helpful to add it here.

Would you be able to split out the important test cases from D113972 as a separate patch ?

I found the meaningful test case mainly comes from the example in spec. e.g., http://eel.is/c++draft/module.unit#4, http://eel.is/c++draft/module.unit#4, http://eel.is/c++draft/module.unit#4

Right, actually it is my intention to have each of the examples in the std as a test case (this is what I have in my working branch) but, obviously, we can only introduce them when the features they show are implemented. So (for example) 4/8 adds 10.1 examples 1 and 2. 5/8 adds in 10.3 ex1 and ex2 .. and so on ..

However, maybe I should review if some of the others should be accepted already :)

[iains]

rebased, added a multi-partition testcase.

[ChuanqiXu]

(May off the patch)

BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between clang and gcc are basically compatible so that user who decide to use clang could turn back to use gcc whenever he want. But I am worrying that it would be a harder decision for users if them are going to use clang. Since the cost to use GCC again would be higher.

[iains]

In D118586#3336612, @ChuanqiXu wrote:

(May off the patch)

I am not sure what you mean here.

BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between clang and gcc are basically compatible so that user who decide to use clang could turn back to use gcc whenever he want. But I am worrying that it would be a harder decision for users if them are going to use clang. Since the cost to use GCC again would be higher.

We have the same objective: to keep user-facing options and functionality the same between clang and GCC (easier for both user and for build systems).
The content of this patch does not alter that. BTW GCC also keeps the module name as "Primary:Parition" internally and user-facing output. The module name in C++20 modules has no fixed mapping to the BMI/CMI filename on disk,.

[ChuanqiXu]

In D118586#3336865, @iains wrote:

In D118586#3336612, @ChuanqiXu wrote:

(May off the patch)

I am not sure what you mean here.

I mean this comment is not related to the patch. So it doesn't mean to block the patch landing.

BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between clang and gcc are basically compatible so that user who decide to use clang could turn back to use gcc whenever he want. But I am worrying that it would be a harder decision for users if them are going to use clang. Since the cost to use GCC again would be higher.

We have the same objective: to keep user-facing options and functionality the same between clang and GCC (easier for both user and for build systems).
The content of this patch does not alter that. BTW GCC also keeps the module name as "Primary:Parition" internally and user-facing output. The module name in C++20 modules has no fixed mapping to the BMI/CMI filename on disk,.

I just tested the behavior of GCC. When I compile a partition interface unit (let's name is mod:part), it would generate a file named mod-part,gcm under the directory gcm.cache. So I feel what GCC does here is more aggressive, it doesn't only choose the separator '-'. It would create an cache directory by default. (I am not blaming it. I like the style too. Otherwise, we need to sore CMI to a place by hand). What I want to say here is that GCC would replace '-' to ':' when maps to the filesystem. I am OK to not implement this in the series patch. But I hope we could get in consensus that we should implement this.

[iains]

In D118586#3336892, @ChuanqiXu wrote:

In D118586#3336865, @iains wrote:

In D118586#3336612, @ChuanqiXu wrote:

(May off the patch)

I am not sure what you mean here.

I mean this comment is not related to the patch. So it doesn't mean to block the patch landing.

BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between clang and gcc are basically compatible so that user who decide to use clang could turn back to use gcc whenever he want. But I am worrying that it would be a harder decision for users if them are going to use clang. Since the cost to use GCC again would be higher.

We have the same objective: to keep user-facing options and functionality the same between clang and GCC (easier for both user and for build systems).
The content of this patch does not alter that. BTW GCC also keeps the module name as "Primary:Parition" internally and user-facing output. The module name in C++20 modules has no fixed mapping to the BMI/CMI filename on disk,.

I just tested the behavior of GCC. When I compile a partition interface unit (let's name is mod:part), it would generate a file named mod-part,gcm under the directory gcm.cache. So I feel what GCC does here is more aggressive, it doesn't only choose the separator '-'. It would create an cache directory by default. (I am not blaming it. I like the style too. Otherwise, we need to sore CMI to a place by hand). What I want to say here is that GCC would replace '-' to ':' when maps to the filesystem. I am OK to not implement this in the series patch. But I hope we could get in consensus that we should implement this.

I think I am not explaining well enough ...

Please execute "strings gcm.cache/mod-part,gcm" - you will see that the Module name is "mod:part" in the module file (GCC knows the module as mod:part internally, it is only the on-disk name that is changed).

We could choose to use "mod-part.pcm" in our example test cases here, but it would not make any difference to the requirement - the mapping between module name and filename has to be determined by the mechanism that interfaces with the filesystem (we cannot impose that mapping on the module names internally to the compiler - since the mapping is unknown in the general case).

So we would implement a basic interface (in the module loader, I suppose since that is available for jobs without a preprocessor) that would give pathNameForModuleName(ModuleName) ,... and probably moduleNameForPathName(PathName) ... those would interface to whatever means was used (e.g. ,module mapper, P1184 etc). You could arrange for the default implementation to do the translation you see in GCC (mod:part <=> mod-part.pcm).

[ChuanqiXu]

In D118586#3336933, @iains wrote:

In D118586#3336892, @ChuanqiXu wrote:

In D118586#3336865, @iains wrote:

In D118586#3336612, @ChuanqiXu wrote:

(May off the patch)

I am not sure what you mean here.

I mean this comment is not related to the patch. So it doesn't mean to block the patch landing.

BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between clang and gcc are basically compatible so that user who decide to use clang could turn back to use gcc whenever he want. But I am worrying that it would be a harder decision for users if them are going to use clang. Since the cost to use GCC again would be higher.

We have the same objective: to keep user-facing options and functionality the same between clang and GCC (easier for both user and for build systems).
The content of this patch does not alter that. BTW GCC also keeps the module name as "Primary:Parition" internally and user-facing output. The module name in C++20 modules has no fixed mapping to the BMI/CMI filename on disk,.

I just tested the behavior of GCC. When I compile a partition interface unit (let's name is mod:part), it would generate a file named mod-part,gcm under the directory gcm.cache. So I feel what GCC does here is more aggressive, it doesn't only choose the separator '-'. It would create an cache directory by default. (I am not blaming it. I like the style too. Otherwise, we need to sore CMI to a place by hand). What I want to say here is that GCC would replace '-' to ':' when maps to the filesystem. I am OK to not implement this in the series patch. But I hope we could get in consensus that we should implement this.

I think I am not explaining well enough ...

Please execute "strings gcm.cache/mod-part,gcm" - you will see that the Module name is "mod:part" in the module file (GCC knows the module as mod:part internally, it is only the on-disk name that is changed).

We could choose to use "mod-part.pcm" in our example test cases here, but it would not make any difference to the requirement - the mapping between module name and filename has to be determined by the mechanism that interfaces with the filesystem (we cannot impose that mapping on the module names internally to the compiler - since the mapping is unknown in the general case).

So we would implement a basic interface (in the module loader, I suppose since that is available for jobs without a preprocessor) that would give pathNameForModuleName(ModuleName) ,... and probably moduleNameForPathName(PathName) ... those would interface to whatever means was used (e.g. ,module mapper, P1184 etc). You could arrange for the default implementation to do the translation you see in GCC (mod:part <=> mod-part.pcm).

Thanks for the detailed explanation. I think we're in consensus now.

[jansvoboda11]

clang/test/Modules/cxx20-multiple-partitions.cpp

48 ↗

(On Diff #410320)

Instead of splitting the file with preprocessor, you could use the split-file utility (introduced in D83834).

[iains]

rebased, update testcases to use split-file.

[iains]

clang/test/Modules/cxx20-multiple-partitions.cpp

48 ↗

(On Diff #410320)

thanks!
this is exactly what is needed here.
I've converted the added cases here (and will do for the follow-on patches).

clang/include/clang/AST/Decl.h

 /// Describes a module import declaration, which makes the contents
 /// of the named module visible in the current translation unit.
 ///
 /// An import declaration imports the named module (or submodule). For example:
 /// \code
 ///   @import std.vector;
 /// \endcode
 ///
+/// A C++20 module import declaration imports the named module or partition.
+/// Periods are permitted in C++20 module names, but have no semantic meaning.
+/// For example:
+/// \code
+///   import NamedModule;
+///   import :SomePartition; // Must be a partition of the current module.
+///   import Names.Like.this; // Allowed.
+///   import :and.Also.Partition.names;
+/// \endcode
+///
 /// Import declarations can also be implicitly generated from
 /// \#include/\#import directives.
 class ImportDecl final : public Decl,
                          llvm::TrailingObjects<ImportDecl, SourceLocation> {
   friend class ASTContext;
   friend class ASTDeclReader;
   friend class ASTReader;
   friend TrailingObjects;

clang/include/clang/Basic/DiagnosticParseKinds.td

def err_global_module_introducer_not_at_start : Error<
   "'module;' introducing a global module fragment can appear only "
   "at the start of the translation unit">;
 def err_module_fragment_exported : Error<
   "%select{global|private}0 module fragment cannot be exported">;
 def err_private_module_fragment_expected_semi : Error<
   "expected ';' after private module fragment declaration">;
 def err_missing_before_module_end : Error<"expected %0 at end of module">;
 def err_unsupported_module_partition : Error<
-  "sorry, module partitions are not yet supported">;
+  "module partitions are only supported for C++20">;
 
 def err_export_empty : Error<"export declaration cannot be empty">;
 }
 
 let CategoryName = "Generics Issue" in {
 
 def err_objc_expected_type_parameter : Error<
   "expected type parameter name">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def ext_module_import_not_at_top_level_noop : ExtWarn<
   "redundant #include of module '%0' appears within %1">, DefaultError,
   InGroup<DiagGroup<"modules-import-nested-redundant">>;
 def note_module_import_not_at_top_level : Note<"%0 begins here">;
 def err_module_self_import : Error<
   "import of module '%0' appears within same top-level module '%1'">;
 def err_module_import_in_implementation : Error<
   "@import of module '%0' in implementation of '%1'; use #import">;
+def err_module_partition_import_global : Error<
+  "module partition '%0' cannot be imported into the global module">;
 
 // C++ Modules
 def err_module_decl_not_at_start : Error<
   "module declaration must occur at the start of the translation unit">;
 def note_global_module_introducer_missing : Note<
   "add 'module;' to the start of the file to introduce a "
   "global module fragment">;
 def err_export_within_anonymous_namespace : Error<

clang/include/clang/Sema/Sema.h

private:
 
   bool RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
                                CompleteTypeKind Kind, TypeDiagnoser *Diagnoser);
 
   struct ModuleScope {
     SourceLocation BeginLoc;
     clang::Module *Module = nullptr;
     bool ModuleInterface = false;
+    bool IsPartition = false;
     bool ImplicitGlobalModuleFragment = false;
     VisibleModuleSet OuterVisibleModules;
   };
   /// The modules we're currently parsing.
   llvm::SmallVector<ModuleScope, 16> ModuleScopes;
 
   /// Namespace definitions that we will export when they finish.
   llvm::SmallPtrSet<const NamespaceDecl*, 8> DeferredExportedNamespaces;
enum class ModuleDeclKind {
     Interface,      ///< 'export module X;'
     Implementation, ///< 'module X;'
   };
 
   /// The parser has processed a module-declaration that begins the definition
   /// of a module interface or implementation.
   DeclGroupPtrTy ActOnModuleDecl(SourceLocation StartLoc,
                                  SourceLocation ModuleLoc, ModuleDeclKind MDK,
-                                 ModuleIdPath Path, bool IsFirstDecl);
+                                 ModuleIdPath Path, ModuleIdPath Partition,
+                                 bool IsFirstDecl);
 
   /// The parser has processed a global-module-fragment declaration that begins
   /// the definition of the global module fragment of the current module unit.
   /// \param ModuleLoc The location of the 'module' keyword.
   DeclGroupPtrTy ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc);
 
   /// The parser has processed a private-module-fragment declaration that begins
   /// the definition of the private module fragment of the current module unit.
   /// \param ModuleLoc The location of the 'module' keyword.
   /// \param PrivateLoc The location of the 'private' keyword.
   DeclGroupPtrTy ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
                                                 SourceLocation PrivateLoc);
 
   /// The parser has processed a module import declaration.
   ///
   /// \param StartLoc The location of the first token in the declaration. This
   ///        could be the location of an '@', 'export', or 'import'.
   /// \param ExportLoc The location of the 'export' keyword, if any.
   /// \param ImportLoc The location of the 'import' keyword.
-  /// \param Path The module access path.
+  /// \param NamePath The module toplevel name as an access path.
+  /// \param Partition The module partition name as an access path.
   DeclResult ActOnModuleImport(SourceLocation StartLoc,
                                SourceLocation ExportLoc,
-                               SourceLocation ImportLoc, ModuleIdPath Path);
+                               SourceLocation ImportLoc,
+                               ModuleIdPath NamePath,
+                               ModuleIdPath Partition = {});
   DeclResult ActOnModuleImport(SourceLocation StartLoc,
                                SourceLocation ExportLoc,
                                SourceLocation ImportLoc, Module *M,
-                               ModuleIdPath Path = {});
+                               ModuleIdPath NamePath = {});
 
   /// The parser has processed a module import translated from a
   /// #include or similar preprocessing directive.
   void ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
   void BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
 
   /// The parsed has entered a submodule.
   void ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod);

clang/lib/AST/Decl.cpp

if (!M)
     return nullptr;
 
   switch (M->Kind) {
   case Module::ModuleMapModule:
     // Module map modules have no special linkage semantics.
     return nullptr;
 
   case Module::ModuleInterfaceUnit:
+    // For decls in partitions, inn principle, the owning module is really
+    // the named module, but that might not be available here.
   case Module::ModulePartitionInterface:
   case Module::ModulePartitionImplementation:
     return M;
 
   case Module::GlobalModuleFragment: {
     // External linkage declarations in the global module have no owning module
     // for linkage purposes. But internal linkage declarations in the global
     // module fragment of a particular module are owned by that module for

clang/lib/Parse/Parser.cpp

if (getLangOpts().CPlusPlusModules && Tok.is(tok::colon) &&
     return Actions.ActOnPrivateModuleFragmentDecl(ModuleLoc, PrivateLoc);
   }
 
   SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
   if (ParseModuleName(ModuleLoc, Path, /*IsImport*/false))
     return nullptr;
 
   // Parse the optional module-partition.
+  SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Partition;
   if (Tok.is(tok::colon)) {
     SourceLocation ColonLoc = ConsumeToken();
-    SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Partition;
-    if (ParseModuleName(ModuleLoc, Partition, /*IsImport*/false))
-      return nullptr;
-
-    // FIXME: Support module partition declarations.
+    if (!getLangOpts().CPlusPlusModules) 
       Diag(ColonLoc, diag::err_unsupported_module_partition)
            << SourceRange(ColonLoc, Partition.back().second);
       // Recover by parsing as a non-partition.
+    else if (ParseModuleName(ModuleLoc, Partition, /*IsImport*/false))
+      return nullptr;
    }
 
   // We don't support any module attributes yet; just parse them and diagnose.
   ParsedAttributesWithRange Attrs(AttrFactory);
   MaybeParseCXX11Attributes(Attrs);
   ProhibitCXX11Attributes(Attrs, diag::err_attribute_not_module_attr);
 
   ExpectAndConsumeSemi(diag::err_module_expected_semi);
 
-  return Actions.ActOnModuleDecl(StartLoc, ModuleLoc, MDK, Path, IsFirstDecl);
+  return Actions.ActOnModuleDecl(StartLoc, ModuleLoc, MDK, Path,
+                                 Partition, IsFirstDecl);
 }
 
 /// Parse a module import declaration. This is essentially the same for
 /// Objective-C and the C++ Modules TS, except for the leading '@' (in ObjC)
 /// and the trailing optional attributes (in C++).
 ///
 /// [ObjC]  @import declaration:
 ///           '@' 'import' module-name ';'
 /// [ModTS] module-import-declaration:
 ///           'import' module-name attribute-specifier-seq[opt] ';'
 /// [C++2a] module-import-declaration:
 ///           'export'[opt] 'import' module-name
Decl *Parser::ParseModuleImport(SourceLocation AtLoc) {
   TryConsumeToken(tok::kw_export, ExportLoc);
 
   assert((AtLoc.isInvalid() ? Tok.isOneOf(tok::kw_import, tok::identifier)
                             : Tok.isObjCAtKeyword(tok::objc_import)) &&
          "Improper start to module import");
   bool IsObjCAtImport = Tok.isObjCAtKeyword(tok::objc_import);
   SourceLocation ImportLoc = ConsumeToken();
 
-  SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
+  SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> NamePath;
+  SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Partition;
   Module *HeaderUnit = nullptr;
+  bool IsPartition = false;
 
   if (Tok.is(tok::header_name)) {
     // This is a header import that the preprocessor decided we should skip
     // because it was malformed in some way. Parse and ignore it; it's already
     // been diagnosed.
     ConsumeToken();
   } else if (Tok.is(tok::annot_header_unit)) {
     // This is a header import that the preprocessor mapped to a module import.
     HeaderUnit = reinterpret_cast<Module *>(Tok.getAnnotationValue());
     ConsumeAnnotationToken();
-  } else if (getLangOpts().CPlusPlusModules && Tok.is(tok::colon)) {
+  } else if (Tok.is(tok::colon)) {
     SourceLocation ColonLoc = ConsumeToken();
-    if (ParseModuleName(ImportLoc, Path, /*IsImport*/true))
-      return nullptr;
-
-    // FIXME: Support module partition import.
+    bool Err = ParseModuleName(ImportLoc, Partition, /*IsImport*/true);
+    if (!getLangOpts().CPlusPlusModules) {
       Diag(ColonLoc, diag::err_unsupported_module_partition)
-      << SourceRange(ColonLoc, Path.back().second);
+        << SourceRange(ColonLoc, Partition.back().second);
+      return nullptr;
+    } else if (Err)
       return nullptr;
+    IsPartition = true;
   } else {
-    if (ParseModuleName(ImportLoc, Path, /*IsImport*/true))
+    if (ParseModuleName(ImportLoc, NamePath, /*IsImport*/true))
       return nullptr;
   }
 
   ParsedAttributesWithRange Attrs(AttrFactory);
   MaybeParseCXX11Attributes(Attrs);
   // We don't support any module import attributes yet.
   ProhibitCXX11Attributes(Attrs, diag::err_attribute_not_import_attr);
 
   if (PP.hadModuleLoaderFatalFailure()) {
     // With a fatal failure in the module loader, we abort parsing.
     cutOffParsing();
     return nullptr;
   }
 
   DeclResult Import;
   if (HeaderUnit)
     Import =
         Actions.ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, HeaderUnit);
-  else if (!Path.empty())
-    Import = Actions.ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, Path);
+  else if (!NamePath.empty() || !Partition.empty())
+    Import = Actions.ActOnModuleImport(StartLoc, ExportLoc, ImportLoc,
+                                       NamePath, Partition);
   ExpectAndConsumeSemi(diag::err_module_expected_semi);
   if (Import.isInvalid())
     return nullptr;
 
   // Using '@import' in framework headers requires modules to be enabled so that
   // the header is parseable. Emit a warning to make the user aware.
   if (IsObjCAtImport && AtLoc.isValid()) {
     auto &SrcMgr = PP.getSourceManager();

clang/lib/Sema/SemaDecl.cpp

bool Sema::CheckRedeclarationModuleOwnership(NamedDecl *New, NamedDecl *Old) {
   Module *NewM = New->getOwningModule();
   Module *OldM = Old->getOwningModule();
 
   if (NewM && NewM->Kind == Module::PrivateModuleFragment)
     NewM = NewM->Parent;
   if (OldM && OldM->Kind == Module::PrivateModuleFragment)
     OldM = OldM->Parent;
 
+  // If we have a decl in a module partition it is part of the containing
+  // module (which is the only thing that can be importing it).
+  if (NewM && OldM && (OldM->Kind == Module::ModulePartitionInterface ||
+                       OldM->Kind == Module::ModulePartitionImplementation)) {
+    return false;
+  }
+
   if (NewM == OldM)
     return false;
 
   bool NewIsModuleInterface = NewM && NewM->isModulePurview();
   bool OldIsModuleInterface = OldM && OldM->isModulePurview();
   if (NewIsModuleInterface || OldIsModuleInterface) {
     // C++ Modules TS [basic.def.odr] 6.2/6.7 [sic]:
     //   if a declaration of D [...] appears in the purview of a module, all

clang/lib/Sema/SemaLookup.cpp

llvm::DenseSet<Module*> &Sema::getLookupModules() {
   return LookupModulesCache;
 }
 
 /// Determine whether the module M is part of the current module from the
 /// perspective of a module-private visibility check.
 static bool isInCurrentModule(const Module *M, const LangOptions &LangOpts) {
   // If M is the global module fragment of a module that we've not yet finished
   // parsing, then it must be part of the current module.
+  // If it's a partition, then it must be visible to an importer (since only
+  // another partition or the named module can import it).
   return M->getTopLevelModuleName() == LangOpts.CurrentModule ||
-         (M->Kind == Module::GlobalModuleFragment && !M->Parent);
+         (M->Kind == Module::GlobalModuleFragment && !M->Parent) ||
+         M->isPartition();
 }
 
 bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
   for (const Module *Merged : Context.getModulesWithMergedDefinition(Def))
     if (isModuleVisible(Merged))
       return true;
   return false;
 }

clang/lib/Sema/SemaModule.cpp

S.Diag(cast<Decl>(DC)->getBeginLoc(),
         << DC;
   } else if (!M->IsExternC && ExternCLoc.isValid()) {
     S.Diag(ImportLoc, diag::ext_module_import_in_extern_c)
       << M->getFullModuleName();
     S.Diag(ExternCLoc, diag::note_extern_c_begins_here);
   }
 }
 
+static std::string stringFromPath(ModuleIdPath Path) {
+  std::string Name;
+  if (Path.empty())
+    return Name;
+
+  for (auto &Piece : Path) {
+    if (!Name.empty())
+      Name += ".";
+    Name += Piece.first->getName();
+  }
+  return Name;
+}
+
 Sema::DeclGroupPtrTy
 Sema::ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc) {
   if (!ModuleScopes.empty() &&
       ModuleScopes.back().Module->Kind == Module::GlobalModuleFragment) {
     // Under -std=c++2a -fmodules-ts, we can find an explicit 'module;' after
     // already implicitly entering the global module fragment. That's OK.
     assert(getLangOpts().CPlusPlusModules && getLangOpts().ModulesTS &&
            "unexpectedly encountered multiple global module fragment decls");
Sema::ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc) {
   TU->setLocalOwningModule(GlobalModule);
 
   // FIXME: Consider creating an explicit representation of this declaration.
   return nullptr;
 }
 
 Sema::DeclGroupPtrTy
 Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc,
-                      ModuleDeclKind MDK, ModuleIdPath Path, bool IsFirstDecl) {
+                      ModuleDeclKind MDK, ModuleIdPath NamePath,
+                      ModuleIdPath Partition, bool IsFirstDecl) {
   assert((getLangOpts().ModulesTS || getLangOpts().CPlusPlusModules) &&
          "should only have module decl in Modules TS or C++20");
 
   // A module implementation unit requires that we are not compiling a module
   // of any kind. A module interface unit requires that we are not compiling a
   // module map.
   switch (getLangOpts().getCompilingModule()) {
   case LangOptions::CMK_None:
if (BeginLoc.isValid()) {
       Diag(BeginLoc, diag::note_global_module_introducer_missing)
           << FixItHint::CreateInsertion(BeginLoc, "module;\n");
     }
   }
 
   // Flatten the dots in a module name. Unlike Clang's hierarchical module map
   // modules, the dots here are just another character that can appear in a
   // module name.
-  std::string ModuleName;
-  for (auto &Piece : Path) {
-    if (!ModuleName.empty())
-      ModuleName += ".";
-    ModuleName += Piece.first->getName();
+  std::string ModuleName = stringFromPath(NamePath);
+  bool IsPartition = !Partition.empty();
+  if (IsPartition) {
+    ModuleName += ":";
+    ModuleName += stringFromPath(Partition);
   }
-
   // If a module name was explicitly specified on the command line, it must be
   // correct.
   if (!getLangOpts().CurrentModule.empty() &&
       getLangOpts().CurrentModule != ModuleName) {
-    Diag(Path.front().second, diag::err_current_module_name_mismatch)
-        << SourceRange(Path.front().second, Path.back().second)
+    Diag(NamePath.front().second, diag::err_current_module_name_mismatch)
+        << SourceRange(NamePath.front().second,
+                       IsPartition ? Partition.back().second
+                                   : NamePath.back().second)
         << getLangOpts().CurrentModule;
     return nullptr;
   }
   const_cast<LangOptions&>(getLangOpts()).CurrentModule = ModuleName;
 
   auto &Map = PP.getHeaderSearchInfo().getModuleMap();
   Module *Mod;
 
   switch (MDK) {
   case ModuleDeclKind::Interface: {
     // We can't have parsed or imported a definition of this module or parsed a
     // module map defining it already.
     if (auto *M = Map.findModule(ModuleName)) {
-      Diag(Path[0].second, diag::err_module_redefinition) << ModuleName;
+      Diag(NamePath[0].second, diag::err_module_redefinition) << ModuleName;
       if (M->DefinitionLoc.isValid())
         Diag(M->DefinitionLoc, diag::note_prev_module_definition);
       else if (Optional<FileEntryRef> FE = M->getASTFile())
         Diag(M->DefinitionLoc, diag::note_prev_module_definition_from_ast_file)
             << FE->getName();
       Mod = M;
       break;
     }
 
     // Create a Module for the module that we're defining.
     Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
                                            GlobalModuleFragment);
+    if (IsPartition)
+      Mod->Kind = Module::ModulePartitionInterface;
     assert(Mod && "module creation should not fail");
     break;
   }
 
   case ModuleDeclKind::Implementation:
     std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc(
-        PP.getIdentifierInfo(ModuleName), Path[0].second);
+        PP.getIdentifierInfo(ModuleName), NamePath[0].second);
+    if (IsPartition) {
+      // Create an interface, but note that it is an implementation
+      // unit.
+      Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
+                                             GlobalModuleFragment);
+      Mod->Kind = Module::ModulePartitionImplementation;
+    } else {
+      // C++20 A module-declaration that contains neither an export-
+      // keyword nor a module-partition implicitly imports the primary
+      // module interface unit of the module as if by a module-import-
+      // declaration.
       Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
                                          Module::AllVisible,
                                          /*IsInclusionDirective=*/false);
       if (!Mod) {
         Diag(ModuleLoc, diag::err_module_not_defined) << ModuleName;
         // Create an empty module interface unit for error recovery.
         Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
                                                GlobalModuleFragment);
       }
+    }
     break;
   }
 
   if (!GlobalModuleFragment) {
     ModuleScopes.push_back({});
     if (getLangOpts().ModulesLocalVisibility)
       ModuleScopes.back().OuterVisibleModules = std::move(VisibleModules);
   } else {
     // We're done with the global module fragment now.
     ActOnEndOfTranslationUnitFragment(TUFragmentKind::Global);
   }
 
   // Switch from the global module fragment (if any) to the named module.
   ModuleScopes.back().BeginLoc = StartLoc;
   ModuleScopes.back().Module = Mod;
-  ModuleScopes.back().ModuleInterface = MDK != ModuleDeclKind::Implementation;
+  ModuleScopes.back().ModuleInterface =
+    (MDK != ModuleDeclKind::Implementation || IsPartition);
+  ModuleScopes.back().IsPartition = IsPartition;
   VisibleModules.setVisible(Mod, ModuleLoc);
 
   // From now on, we have an owning module for all declarations we see.
   // However, those declarations are module-private unless explicitly
   // exported.
   auto *TU = Context.getTranslationUnitDecl();
   TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModulePrivate);
   TU->setLocalOwningModule(Mod);
Sema::ActOnPrivateModuleFragmentDecl(SourceLocation ModuleLoc,
 
   // FIXME: Consider creating an explicit representation of this declaration.
   return nullptr;
 }
 
 DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
                                    SourceLocation ExportLoc,
                                    SourceLocation ImportLoc,
-                                   ModuleIdPath Path) {
-  // Flatten the module path for a Modules TS module name.
+                                   ModuleIdPath NamePath,
+                                   ModuleIdPath Partition) {
+
+  bool IsPartition = !Partition.empty();
+  bool Cxx20Mode = getLangOpts().CPlusPlusModules || getLangOpts().ModulesTS;
+  assert ((!IsPartition || Cxx20Mode) &&
+          "partition seen in non-C++20 code?");
+  assert ((!IsPartition || NamePath.empty()) &&
+          "trying to import a fully qualified partition name?");
+
+  // For a C++20/Modules TS module name, flatten into a  single identifier
+  // with the source location of the first component.
   std::pair<IdentifierInfo *, SourceLocation> ModuleNameLoc;
-  if (getLangOpts().ModulesTS) {
+
+  if (IsPartition) {
     std::string ModuleName;
-    for (auto &Piece : Path) {
-      if (!ModuleName.empty())
-        ModuleName += ".";
-      ModuleName += Piece.first->getName();
+    if (ModuleScopes.empty() ||
+        ModuleScopes.back().Module->Kind == Module::GlobalModuleFragment) {
+      // This is an attempt to import a partition without naming the module to
+      // which it belongs.
+      ModuleName = ":";
+      ModuleName += stringFromPath(Partition);
+      Diag(ImportLoc, diag::err_module_partition_import_global) << ModuleName;
+      return true;
     }
-    ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Path[0].second};
-    Path = ModuleIdPath(ModuleNameLoc);
+    Module *NamedMod = ModuleScopes.back().Module;
+    if (ModuleScopes.back().IsPartition) {
+      // We're importing a partition into a partition, find the name of the
+      // owning named module.
+      size_t P = NamedMod->Name.find_first_of(":");
+      ModuleName = NamedMod->Name.substr(0, P+1);
+    } else {
+      // We're importing a partition into the named module itself (either the
+      // interface or an implementation TU).
+      ModuleName =  NamedMod->Name;
+      ModuleName += ":";
+    }
+    ModuleName += stringFromPath(Partition);
+    ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), Partition[0].second};
+    Partition = ModuleIdPath(ModuleNameLoc);
+  } else if (Cxx20Mode) {
+    std::string ModuleName = stringFromPath(NamePath);
+    ModuleNameLoc = {PP.getIdentifierInfo(ModuleName), NamePath[0].second};
+    NamePath = ModuleIdPath(ModuleNameLoc);
   }
 
+  Module::NameVisibilityKind Vis = IsPartition ? Module::Hidden
+                                               : Module::AllVisible;
   Module *Mod =
-      getModuleLoader().loadModule(ImportLoc, Path, Module::AllVisible,
-                                   /*IsInclusionDirective=*/false);
+      getModuleLoader().loadModule(ImportLoc,
+                                   IsPartition ? Partition : NamePath,
+                                   Vis, /*IsInclusionDirective=*/false);
   if (!Mod)
     return true;
 
-  return ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, Mod, Path);
+  return ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, Mod,
+                           IsPartition ? Partition : NamePath);
 }
 
 /// Determine whether \p D is lexically within an export-declaration.
 static const ExportDecl *getEnclosingExportDecl(const Decl *D) {
   for (auto *DC = D->getLexicalDeclContext(); DC; DC = DC->getLexicalParent())
     if (auto *ED = dyn_cast<ExportDecl>(DC))
       return ED;
   return nullptr;
 }
 
 DeclResult Sema::ActOnModuleImport(SourceLocation StartLoc,
                                    SourceLocation ExportLoc,
                                    SourceLocation ImportLoc,
-                                   Module *Mod, ModuleIdPath Path) {
+                                   Module *Mod, ModuleIdPath NamePath) {
   VisibleModules.setVisible(Mod, ImportLoc);
 
   checkModuleImportContext(*this, Mod, ImportLoc, CurContext);
 
   // FIXME: we should support importing a submodule within a different submodule
   // of the same top-level module. Until we do, make it an error rather than
   // silently ignoring the import.
   // Import-from-implementation is valid in the Modules TS. FIXME: Should we
   // warn on a redundant import of the current module?
   // FIXME: Import of a module from an implementation partition of the same
   // module is permitted.
   if (Mod->getTopLevelModuleName() == getLangOpts().CurrentModule &&
       (getLangOpts().isCompilingModule() || !getLangOpts().ModulesTS)) {
     Diag(ImportLoc, getLangOpts().isCompilingModule()
                         ? diag::err_module_self_import
                         : diag::err_module_import_in_implementation)
         << Mod->getFullModuleName() << getLangOpts().CurrentModule;
   }
 
+//  CXX/module/module.interface/p2.cpp
+// assert ((!getLangOpts().CPlusPlusModules || !Mod->Parent) &&
+//          "unexpected module hierarchy in C++20 mode");
   SmallVector<SourceLocation, 2> IdentifierLocs;
+
+  if (NamePath.empty()) {
+    // If this was a header import, pad out with dummy locations.
+    // FIXME: Pass in and use the location of the header-name token in this
+    // case.
+    for (Module *ModCheck = Mod; ModCheck; ModCheck = ModCheck->Parent)
+      IdentifierLocs.push_back(SourceLocation());
+  } else if (getLangOpts().CPlusPlusModules && !Mod->Parent) {
+    // A single identifier for the whole name.
+    IdentifierLocs.push_back(NamePath[0].second);
+  } else {
     Module *ModCheck = Mod;
-  for (unsigned I = 0, N = Path.size(); I != N; ++I) {
+    for (unsigned I = 0, N = NamePath.size(); I != N; ++I) {
       // If we've run out of module parents, just drop the remaining identifiers.
       // We need the length to be consistent.
       if (!ModCheck)
         break;
       ModCheck = ModCheck->Parent;
 
-    IdentifierLocs.push_back(Path[I].second);
-  }
-
-  // If this was a header import, pad out with dummy locations.
-  // FIXME: Pass in and use the location of the header-name token in this case.
-  if (Path.empty()) {
-    for (; ModCheck; ModCheck = ModCheck->Parent) {
-      IdentifierLocs.push_back(SourceLocation());
+      IdentifierLocs.push_back(NamePath[I].second);
     }
   }
 
   ImportDecl *Import = ImportDecl::Create(Context, CurContext, StartLoc,
                                           Mod, IdentifierLocs);
   CurContext->addDecl(Import);
 
   // Sequence initialization of the imported module before that of the current
   // module, if any.
   if (!ModuleScopes.empty())
     Context.addModuleInitializer(ModuleScopes.back().Module, Import);
 
   if (!ModuleScopes.empty() && ModuleScopes.back().ModuleInterface) {
     // Re-export the module if the imported module is exported.
     // Note that we don't need to add re-exported module to Imports field
     // since `Exports` implies the module is imported already.
     if (ExportLoc.isValid() || getEnclosingExportDecl(Import))
       getCurrentModule()->Exports.emplace_back(Mod, false);
     else
       getCurrentModule()->Imports.insert(Mod);
   } else if (ExportLoc.isValid()) {
     // [module.interface]p1:
     // An export-declaration shall inhabit a namespace scope and appear in the
     // purview of a module interface unit.
     Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
+  } else if (getLangOpts().isCompilingModule()) {
+    Module *ThisModule = PP.getHeaderSearchInfo()
+                           .lookupModule(getLangOpts().CurrentModule,
+                                         ExportLoc, false, false);
+    assert (ThisModule && "was expecting a module if building one");
   }
 
   return Import;
 }
 
 void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
   checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext, true);
   BuildModuleInclude(DirectiveLoc, Mod);
if (ShouldAddImport) {
     if (!ModuleScopes.empty())
       Context.addModuleInitializer(ModuleScopes.back().Module, ImportD);
     TU->addDecl(ImportD);
     Consumer.HandleImplicitImportDecl(ImportD);
   }
 
   getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, DirectiveLoc);
   VisibleModules.setVisible(Mod, DirectiveLoc);
+
+  if (getLangOpts().isCompilingModule()) {
+    Module *ThisModule = PP.getHeaderSearchInfo()
+                           .lookupModule(getLangOpts().CurrentModule,
+                                         DirectiveLoc, false,false);
+    assert (ThisModule && "was expecting a module if building one");
+  }
 }
 
 void Sema::ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod) {
   checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext, true);
 
   ModuleScopes.push_back({});
   ModuleScopes.back().Module = Mod;
   if (getLangOpts().ModulesLocalVisibility)
Module *Sema::PushGlobalModuleFragment(SourceLocation BeginLoc,
   ModuleMap &Map = PP.getHeaderSearchInfo().getModuleMap();
   Module *GlobalModule =
       Map.createGlobalModuleFragmentForModuleUnit(BeginLoc, getCurrentModule());
   assert(GlobalModule && "module creation should not fail");
 
   // Enter the scope of the global module.
   ModuleScopes.push_back({BeginLoc, GlobalModule,
                           /*ModuleInterface=*/false,
+                          /*IsPartition=*/false,
                           /*ImplicitGlobalModuleFragment=*/IsImplicit,
-                          /*VisibleModuleSet*/{}});
+                          /*OuterVisibleModules=*/{}});
   VisibleModules.setVisible(GlobalModule, BeginLoc);
 
   return GlobalModule;
 }
 
 void Sema::PopGlobalModuleFragment() {
   assert(!ModuleScopes.empty() && getCurrentModule()->isGlobalModule() &&
          "left the wrong module scope, which is not global module fragment");
   ModuleScopes.pop_back();
 }

clang/test/CXX/module/module.unit/p3.cpp

 // RUN: %clang_cc1 -std=c++2a -verify %s
 
-export module foo:bar; // expected-error {{sorry, module partitions are not yet supported}}
-import :baz; // expected-error {{sorry, module partitions are not yet supported}}
+export module foo:bar;
+import :baz; // expected-error {{module 'foo:baz' not found}}

clang/test/CXX/module/module.unit/p8.cpp

 // RUN: echo 'export module foo; export int n;' > %t.cppm
 // RUN: %clang_cc1 -std=c++2a %t.cppm -emit-module-interface -o %t.pcm
 // RUN: %clang_cc1 -std=c++2a -fmodule-file=%t.pcm -verify -DMODE=0 %s
 // RUN: %clang_cc1 -std=c++2a -fmodule-file=%t.pcm -verify -DMODE=1 %s
 // RUN: %clang_cc1 -std=c++2a -fmodule-file=%t.pcm -verify -DMODE=2 %s
 // RUN: %clang_cc1 -std=c++2a -fmodule-file=%t.pcm -verify -DMODE=3 %s
 // RUN: %clang_cc1 -std=c++2a -fmodule-file=%t.pcm -verify -DMODE=4 %s
 // RUN: %clang_cc1 -std=c++2a -fmodule-file=%t.pcm -verify -DMODE=5 %s
 
 #if MODE == 0
 // no module declaration
 
 #elif MODE == 1
 // expected-no-diagnostics
-module foo;
+module foo; // Implementation, implicitly imports foo.
 #define IMPORTED
 
 #elif MODE == 2
 export module foo; // expected-error {{redefinition of module 'foo'}}
 // expected-note-re@* {{module loaded from '{{.*}}.pcm'}}
 #define IMPORTED
 
 #elif MODE == 3
-export module bar;
+export module bar; // A different module
 
 #elif MODE == 4
-module foo:bar; // expected-error {{not yet supported}}
-#define IMPORTED // FIXME
+module foo:bar; // Partition implementation
+//#define IMPORTED (we don't import foo here)
 
 #elif MODE == 5
-export module foo:bar; // expected-error {{not yet supported}} expected-error {{redefinition}} expected-note@* {{loaded from}}
-#define IMPORTED // FIXME
+export module foo:bar; // Partition interface
+//#define IMPORTED  (we don't import foo here)
 
 #endif
 
 int k = n;
 #ifndef IMPORTED
 // expected-error@-2 {{declaration of 'n' must be imported from module 'foo' before it is required}}
 // expected-note@* {{not visible}}
 #endif

clang/test/Modules/cxx20-10-1-ex1.cpp

+// The example in the standard is not in required build order.
+// revised here
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/A_Internals.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/A_Internals.pcm -o %t/A_Foo.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:  -fmodule-file=%t/A_Foo.pcm -o %t/A.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj  -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/A.pcm -o %t/ex1.o
+
+// expected-no-diagnostics
+
+#if TU == 1
+
+module A:Internals;
+int bar();
+
+#elif TU == 2
+
+export module A:Foo;
+
+import :Internals;
+
+export int foo() { return 2 * (bar() + 1); }
+
+#elif TU == 3
+
+export module A;
+export import :Foo;
+export int baz();
+
+#elif TU == 4
+module A;
+
+import :Internals;
+
+int bar() { return baz() - 10; }
+int baz() { return 30; }
+
+#else
+#error "no TU set"
+#endif

clang/test/Modules/cxx20-10-1-ex2.cpp

+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \
+// RUN:  -o %t/B_Y.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=2 -x c++ %s \
+// RUN:  -fmodule-file=%t/B_Y.pcm -o %t/B.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=3 -x c++ %s \
+// RUN:   -o %t/B_X1.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=4 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/B_X2.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -D TU=5 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu5.s
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=6 -x c++ %s \
+// RUN:  -fmodule-file=%t/B.pcm  -o %t/b_tu6.s -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=7 -x c++ %s \
+// RUN:  -fmodule-file=%t/B_X2.pcm  -o %t/B_X3.pcm -verify
+
+#if TU == 1
+  module B:Y;
+  int y();
+// expected-no-diagnostics
+#elif TU == 2
+  export module B;
+  import :Y;
+  int n = y();
+// expected-no-diagnostics
+#elif TU == 3
+  module B:X1; // does not implicitly import B
+  int &a = n;  // expected-error {{use of undeclared identifier }}
+#elif TU == 4
+  module B:X2; // does not implicitly import B
+  import B;
+  int &b = n; // OK
+// expected-no-diagnostics
+#elif TU == 5
+  module B; // implicitly imports B
+  int &c = n; // OK
+// expected-no-diagnostics
+#elif TU == 6
+  import B;
+              // error, n is module-local and this is not a module.
+  int &c = n; // expected-error {{use of undeclared identifier}}
+#elif TU == 7
+  module B:X3; // does not implicitly import B
+  import :X2; // X2 is an implementation so exports nothing.
+              // error: n not visible here.
+  int &c = n; // expected-error {{use of undeclared identifier }}
+#else
+#error "no TU set"
+#endif

clang/test/Modules/cxx20-module-partition-diagnostics-a.cpp

+// Module Partition diagnostics
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=1 -x c++ %s -o /dev/null -verify
+
+// RUN: %clang_cc1 -std=c++20 -S -D TU=2 -x c++ %s -o /dev/null -verify
+
+#if TU == 1
+
+import :B; // expected-error {{module partition ':B' cannot be imported into the global module}}
+
+#elif TU == 2
+module; // expected-error {{missing 'module' declaration at end of global module fragment introduced here}}
+
+import :Part; // expected-error {{module partition ':Part' cannot be imported into the global module}}
+
+#else
+#error "no TU set"
+#endif