[Modules] Add ability to specify module name to module file mapping

[boris]

Summary

Extend the -fmodule-file option to support the [<name>=]<file> value format.
If the name is omitted, then the old semantics is preserved (the module file
is loaded whether needed or not). If the name is specified, then the mapping
is treated as just another prebuilt module search mechanism, similar to
-fprebuilt-module-path, and the module file is only loaded if actually
used (e.g., via import). With one exception: this mapping also overrides
module file references embedded in other modules (which can be useful if
module files are moved/renamed as often happens during remote compilation).

This override semantics requires some extra work: we now store the module
name in addition to the file name in the serialized AST representation as
well as keep track of already loaded prebuilt modules by-name in addition
to by-file.

Patch by Boris Kolpackov

---

Additional notes:

1. Based on this mailing list discussion:
   
   http://lists.llvm.org/pipermail/cfe-dev/2017-June/054431.html

2. The need to change the serialized AST representation was a bit more than what I hoped for my first patch but based on this FIXME comment I recon this is moving in right direction:
   
   ASTReader::ReadModuleOffsetMap():
   
   // FIXME: Looking up dependency modules by filename is horrible.
   
   So now, at least for prebuilt modules, we look up by the module name.

3. Overloading -fmodule-file for this admittedly fairly different semantics might look like a bad idea and source of some unnecessary complexity. The problem with a separate option approach is the difficulty of finding a decent name that is not already used (e.g., -fmodule-map is out because of -fmodule-map-file; more details in the mailing list thread). But I am still open to changing this to a separate option if there are strong feelings (and good name suggestions ;-)).

4. I plan to implement a companion option that will read this mapping from a file. I will submit it as a separate patch once the general validity of the approach is confirmed.

[boris]

Rebase on latest HEAD.

[boris]

Ping.

[boris]

Rebase on latest HEAD.

[boris]

Ping.

I am holding off on proposing the same functionality to GCC because I want to make sure the command line interface is the same for both compilers (GCC has less baggage in this area, option-name-wise). So confirming that at least the naming/semantics are acceptable would be very helpful.

[boris]

Ping.

FWIW, I went ahead and implemented this functionality in GCC. It has been merged into its c++-modules branch.

[boris]

Ping.

[boris]

Ping.

[rsmith]

docs/ClangCommandLineReference.rst

3–4 ↗

(On Diff #107017)

Please revert the change to this file; we can regenerate it after this lands (as a separate commit).

include/clang/Driver/Options.td

1105 ↗

(On Diff #107017)

How about "Specify the mapping of module name to precompiled module file, or load a module file if name is omitted."?

include/clang/Serialization/ModuleManager.h

52–53 ↗

(On Diff #107017)

llvm::StringMap would be preferable here.

lib/Driver/ToolChains/Clang.cpp

3613–3622 ↗

(On Diff #107017)

What's the purpose of splitting this into two separate loops? I see that you're passing on all -fmodule-file=name=value flags even if modules is not enabled. Is that useful?

lib/Frontend/CompilerInstance.cpp

1623–1630 ↗

(On Diff #107017)

This doesn't make sense for header modules (defined by a module map). For the case of a Modules TS module implementation unit, we should skip this CurrentModule check, but let's leave that for a separate patch.

lib/Frontend/CompilerInvocation.cpp

982 ↗

(On Diff #107017)

There should be some way to specify a module file that contains a = in its file name. Ideally, that way would be compatible with our currently-supported form of -fmodule-name=filename. I don't see a way to support that other than stating every value we're given and checking to see whether it exists before attempting to split on a =... thoughts?

One somewhat hackish alternative would be to only recognize an = if there is no preceding /. (So -fmodule-file=foo=bar.pcm specifies a mapping, and -fmodule-file=./foo=bar.pcm specifies the file ./foo=bar.pcm.)

(FWIW, we also support module names containing = characters.)

lib/Lex/HeaderSearch.cpp

131 ↗

(On Diff #107017)

I like the renaming you've done here. Should this one also be renamed to getCachedModuleFileName?

lib/Serialization/ASTReader.cpp

4145–4146 ↗

(On Diff #107017)

I'm worried by this: the Type can be different for different imports of the same .pcm file, and you'll only get here for the *first* import edge. So you can fail to add the module to the "prebuilt modules" map here, and then later attempt to look it up there (when processing the module offset map) and fail to find it.

Instead of keeping a separate lookup structure on the module manager, could you look up the Module, and then use its ASTFile to find the corresponding ModuleFile?

(If you go that way, the changes to module lookup when reading the module offset map could be generalized to apply to all MK_*Module types.)

[boris]

I've marked as "done" items that I have resolved in my local revision (not yet uploaded) and have added one comment for further feedback.

lib/Frontend/CompilerInvocation.cpp

982 ↗

(On Diff #107017)

A couple of thoughts:

1. Using stat() is also not without issues: I may have an unrelated file with a name that matches the whole value -- this will be fun to debug.

2. GCC seems to have given up on trying to support paths with '=' since AFAIK, none of their key=value options (e.g., -fdebug-prefix-map) support any kind of escaping. I think the implicit assumption is that if you are using paths with '=' in them, then you've asked for it.

3. The '/' idea will work but will get a bit hairier if we also want to support Windows (will need to check for both slashes).

4. Another option is to reserve empty module name as a way to escape '=': -fmodule-file==foo=bar.pcm. Not backwards compatible (but neither is ./) and looks a bit weird but is the simplest to implement.

My preference order is (2), (4), (3), (1). Let me know which way you want to go.

[boris]

lib/Serialization/ASTReader.cpp

4145–4146 ↗

(On Diff #107017)

Yes, I was not entirely happy with this register prebuilt business so thanks for the hint. I've implemented this and all tests pass (both Clang's and my own). Though I cannot say I fully understand all the possible scenarios (like when can ASTFile be NULL).

(If you go that way, the changes to module lookup when reading the module offset map could be generalized to apply to all MK_*Module types.)

Are you suggesting that we switch to storing module names instead of module files for all the module types in the offset map? If so, I think I've tried that at some point but it didn't go well (existing tests were failing if I remember correctly). I can try again if you think this should work.

[boris]

New revision of the patch that I believe addresses all the issues except for the '=' escaping.

[boris]

Ping.

[rsmith]

Only typographical nits and post-commit suggestions, so please go ahead. Thanks!

include/clang/Serialization/ModuleManager.h

63 ↗

(On Diff #111826)

&, not & , please.

lib/Frontend/CompilerInvocation.cpp

986 ↗

(On Diff #111826)

Does a module file specified via -fmodule-file=foo=foo.pcm get included in the dep file if it is used? (If not, I'm happy for that to be fixed in a separate change, but it does need to be fixed for depfile-based build systems.)

982 ↗

(On Diff #107017)

We've had a while to think about this, and haven't found a completely satisfactory answer, so let's go with (2) for now, without prejudice to future alternatives. It's fully GCC-compatible, and happens to be what this patch already does :)

lib/Serialization/ASTReader.cpp

10180 ↗

(On Diff #111826)

No space before ().

4145–4146 ↗

(On Diff #107017)

Yes, that's what I was suggesting, but don't block this commit on it. Ultimately, we should probably be using something like an index into the IMPORTS list instead (but it's not quite as easy as that since we also have module offsets for imports of imports and so on).

[boris]

lib/Frontend/CompilerInvocation.cpp

986 ↗

(On Diff #111826)

No, currently it does not and neither a module file that is discovered via -fprebuilt-module-path. I think if we are doing this, then we should do for both cases. What do you think?

cfe/trunk/docs/Modules.rst

``-fmodule-map-file=<file>``
   Load the given module map file if a header from its directory or one of its subdirectories is loaded.
 
 ``-fmodules-search-all``
   If a symbol is not found, search modules referenced in the current module maps but not imported for symbols, so the error message can reference the module by name.  Note that if the global module index has not been built before, this might take some time as it needs to build all the modules.  Note that this option doesn't apply in module builds, to avoid the recursion.
 
 ``-fno-implicit-modules``
   All modules used by the build must be specified with ``-fmodule-file``.
 
-``-fmodule-file=<file>``
-  Load the given precompiled module file.
+``-fmodule-file=[<name>=]<file>``
+  Specify the mapping of module names to precompiled module files. If the
+  name is omitted, then the module file is loaded whether actually required
+  or not. If the name is specified, then the mapping is treated as another
+  prebuilt module search mechanism (in addition to ``-fprebuilt-module-path``)
+  and the module is only loaded if required. Note that in this case the
+  specified file also overrides this module's paths that might be embedded
+  in other precompiled module files.
 
 ``-fprebuilt-module-path=<directory>``
   Specify the path to the prebuilt modules. If specified, we will look for modules in this directory for a given top-level module name. We don't need a module map for loading prebuilt modules in this directory and the compiler will not try to rebuild these modules. This can be specified multiple times.
 
 Module Semantics
 ================
 
 Modules are modeled as if each submodule were a separate translation unit, and a module import makes names from the other translation unit visible. Each submodule starts with a new preprocessor state and an empty translation unit.

 
 .. [#] There are certain anti-patterns that occur in headers, particularly system headers, that cause problems for modules. The section `Modularizing a Platform`_ describes some of them.
 
 .. [#] The second instance is actually a new thread within the current process, not a separate process. However, the original compiler instance is blocked on the execution of this thread.
 
 .. [#] The preprocessing context in which the modules are parsed is actually dependent on the command-line options provided to the compiler, including the language dialect and any ``-D`` options. However, the compiled modules for different command-line options are kept distinct, and any preprocessor directives that occur within the translation unit are ignored. See the section on the `Configuration macros declaration`_ for more information.
 
 .. _PCHInternals: PCHInternals.html
- 

cfe/trunk/include/clang/Driver/Options.td

def fmodule_name_EQ : Joined<["-"], "fmodule-name=">, Group<f_Group>,
   HelpText<"Specify the name of the module to build">;
 def fmodule_name : Separate<["-"], "fmodule-name">, Alias<fmodule_name_EQ>;
 def fmodule_implementation_of : Separate<["-"], "fmodule-implementation-of">,
   Flags<[CC1Option]>, Alias<fmodule_name_EQ>;
 def fmodule_map_file : Joined<["-"], "fmodule-map-file=">,
   Group<f_Group>, Flags<[DriverOption,CC1Option]>, MetaVarName<"<file>">,
   HelpText<"Load this module map file">;
 def fmodule_file : Joined<["-"], "fmodule-file=">,
-  Group<f_Group>, Flags<[DriverOption,CC1Option]>,
-  HelpText<"Load this precompiled module file">, MetaVarName<"<file>">;
+  Group<i_Group>, Flags<[DriverOption,CC1Option]>, MetaVarName<"[<name>=]<file>">,
+  HelpText<"Specify the mapping of module name to precompiled module file, or load a module file if name is omitted.">;
 def fmodules_ignore_macro : Joined<["-"], "fmodules-ignore-macro=">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Ignore the definition of the given macro when building and loading modules">;
 def fmodules_decluse : Flag <["-"], "fmodules-decluse">, Group<f_Group>,
   Flags<[DriverOption,CC1Option]>,
   HelpText<"Require declaration of modules used within a module">;
 def fmodules_strict_decluse : Flag <["-"], "fmodules-strict-decluse">, Group<f_Group>,
   Flags<[DriverOption,CC1Option]>,
   HelpText<"Like -fmodules-decluse but requires all headers to be in modules">;

cfe/trunk/include/clang/Lex/HeaderSearch.h

public:
 
   /// CreateHeaderMap - This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHeaderMap(const FileEntry *FE);
 
   /// \brief Get filenames for all registered header maps.
   void getHeaderMapFileNames(SmallVectorImpl<std::string> &Names) const;
 
-  /// \brief Retrieve the name of the module file that should be used to 
-  /// load the given module.
+  /// \brief Retrieve the name of the cached module file that should be used
+  /// to load the given module.
   ///
   /// \param Module The module whose module file name will be returned.
   ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getModuleFileName(Module *Module);
+  std::string getCachedModuleFileName(Module *Module);
 
-  /// \brief Retrieve the name of the module file that should be used to 
-  /// load a module with the given name.
+  /// \brief Retrieve the name of the prebuilt module file that should be used
+  /// to load a module with the given name.
+  ///
+  /// \param ModuleName The module whose module file name will be returned.
+  ///
+  /// \param FileMapOnly If true, then only look in the explicit module name
+  //  to file name map and skip the directory search.
+  ///
+  /// \returns The name of the module file that corresponds to this module,
+  /// or an empty string if this module does not correspond to any module file.
+  std::string getPrebuiltModuleFileName(StringRef ModuleName,
+                                        bool FileMapOnly = false);
+
+
+  /// \brief Retrieve the name of the (to-be-)cached module file that should
+  /// be used to load a module with the given name.
   ///
   /// \param ModuleName The module whose module file name will be returned.
   ///
   /// \param ModuleMapPath A path that when combined with \c ModuleName
   /// uniquely identifies this module. See Module::ModuleMap.
   ///
-  /// \param UsePrebuiltPath Whether we should use the prebuilt module path.
-  ///
   /// \returns The name of the module file that corresponds to this module,
   /// or an empty string if this module does not correspond to any module file.
-  std::string getModuleFileName(StringRef ModuleName, StringRef ModuleMapPath,
-                                bool UsePrebuiltPath);
+  std::string getCachedModuleFileName(StringRef ModuleName,
+                                      StringRef ModuleMapPath);
 
   /// \brief Lookup a module Search for a module with the given name.
   ///
   /// \param ModuleName The name of the module we're looking for.
   ///
   /// \param AllowSearch Whether we are allowed to search in the various
   /// search directories to produce a module definition. If not, this lookup
   /// will only return an already-known module.

cfe/trunk/include/clang/Lex/HeaderSearchOptions.h

 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/CachedHashString.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringRef.h"
 #include <string>
 #include <vector>
+#include <map>
 
 namespace clang {
 
 namespace frontend {
   /// IncludeDirGroup - Identifies the group an include Entry belongs to,
   /// representing its relative positive in the search list.
   /// \#include directives whose paths are enclosed by string quotes ("")
   /// start searching at the Quoted group (specified by '-iquote'),
public:
   std::string ResourceDir;
 
   /// \brief The directory used for the module cache.
   std::string ModuleCachePath;
 
   /// \brief The directory used for a user build.
   std::string ModuleUserBuildPath;
 
+  /// \brief The mapping of module names to prebuilt module files.
+  std::map<std::string, std::string> PrebuiltModuleFiles;
+
   /// \brief The directories used to load prebuilt module files.
   std::vector<std::string> PrebuiltModulePaths;
 
   /// The module/pch container format.
   std::string ModuleFormat;
 
   /// \brief Whether we should disable the use of the hash string within the
   /// module cache.

cfe/trunk/include/clang/Serialization/ASTReader.h

public:
 
   /// \brief Read a floating-point value
   llvm::APFloat ReadAPFloat(const RecordData &Record,
                             const llvm::fltSemantics &Sem, unsigned &Idx);
 
   // \brief Read a string
   static std::string ReadString(const RecordData &Record, unsigned &Idx);
 
+  // \brief Skip a string
+  static void SkipString(const RecordData &Record, unsigned &Idx) {
+    Idx += Record[Idx] + 1;
+  }
+
   // \brief Read a path
   std::string ReadPath(ModuleFile &F, const RecordData &Record, unsigned &Idx);
 
+  // \brief Skip a path
+  static void SkipPath(const RecordData &Record, unsigned &Idx) {
+    SkipString(Record, Idx);
+  }
+
   /// \brief Read a version tuple.
   static VersionTuple ReadVersionTuple(const RecordData &Record, unsigned &Idx);
 
   CXXTemporary *ReadCXXTemporary(ModuleFile &F, const RecordData &Record,
                                  unsigned &Idx);
 
   /// \brief Reads attributes from the current stream position.
   void ReadAttributes(ASTRecordReader &Record, AttrVec &Attrs);

cfe/trunk/include/clang/Serialization/ModuleManager.h

 #include "llvm/ADT/iterator.h"
 
 namespace clang { 
 
 class GlobalModuleIndex;
 class MemoryBufferCache;
 class ModuleMap;
 class PCHContainerReader;
+class HeaderSearch;
 
 namespace serialization {
 
 /// \brief Manages the set of modules loaded by an AST reader.
 class ModuleManager {
   /// \brief The chain of AST files, in the order in which we started to load
   /// them (this order isn't really useful for anything).
   SmallVector<std::unique_ptr<ModuleFile>, 2> Chain;
class ModuleManager {
   FileManager &FileMgr;
 
   /// Cache of PCM files.
   IntrusiveRefCntPtr<MemoryBufferCache> PCMCache;
 
   /// \brief Knows how to unwrap module containers.
   const PCHContainerReader &PCHContainerRdr;
 
+  /// \brief Preprocessor's HeaderSearchInfo containing the module map.
+  const HeaderSearch &HeaderSearchInfo;
+
   /// \brief A lookup of in-memory (virtual file) buffers
   llvm::DenseMap<const FileEntry *, std::unique_ptr<llvm::MemoryBuffer>>
       InMemoryBuffers;
 
   /// \brief The visitation order.
   SmallVector<ModuleFile *, 4> VisitOrder;
       
   /// \brief The list of module files that both we and the global module index
typedef llvm::pointee_iterator<
       SmallVectorImpl<std::unique_ptr<ModuleFile>>::const_iterator>
       ModuleConstIterator;
   typedef llvm::pointee_iterator<
       SmallVectorImpl<std::unique_ptr<ModuleFile>>::reverse_iterator>
       ModuleReverseIterator;
   typedef std::pair<uint32_t, StringRef> ModuleOffset;
 
   explicit ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache,
-                         const PCHContainerReader &PCHContainerRdr);
+                         const PCHContainerReader &PCHContainerRdr,
+                         const HeaderSearch &HeaderSearchInfo);
   ~ModuleManager();
 
   /// \brief Forward iterator to traverse all loaded modules.
   ModuleIterator begin() { return Chain.begin(); }
   /// \brief Forward iterator end-point to traverse all loaded modules
   ModuleIterator end() { return Chain.end(); }
   
   /// \brief Const forward iterator to traverse all loaded modules.
public:
   
   /// \brief Returns the primary module associated with the manager, that is,
   /// the first module loaded.
   ModuleFile &getPrimaryModule() const { return *Chain[0]; }
   
   /// \brief Returns the module associated with the given index
   ModuleFile &operator[](unsigned Index) const { return *Chain[Index]; }
   
-  /// \brief Returns the module associated with the given name
-  ModuleFile *lookup(StringRef Name) const;
+  /// \brief Returns the module associated with the given file name.
+  ModuleFile *lookupByFileName(StringRef FileName) const;
+
+  /// \brief Returns the module associated with the given module name.
+  ModuleFile *lookupByModuleName(StringRef ModName) const;
 
   /// \brief Returns the module associated with the given module file.
   ModuleFile *lookup(const FileEntry *File) const;
 
   /// \brief Returns the in-memory (virtual file) buffer with the given name
   std::unique_ptr<llvm::MemoryBuffer> lookupBuffer(StringRef Name);
   
   /// \brief Number of modules loaded

cfe/trunk/lib/Driver/ToolChains/Clang.cpp

if (!Args.hasFlag(options::OPT_fimplicit_modules,
     }
     const char Arg[] = "-fmodules-cache-path=";
     Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
     CmdArgs.push_back(Args.MakeArgString(Path));
   }
 
   if (HaveAnyModules) {
     // -fprebuilt-module-path specifies where to load the prebuilt module files.
-    for (const Arg *A : Args.filtered(options::OPT_fprebuilt_module_path))
+    for (const Arg *A : Args.filtered(options::OPT_fprebuilt_module_path)) {
       CmdArgs.push_back(Args.MakeArgString(
           std::string("-fprebuilt-module-path=") + A->getValue()));
+      A->claim();
+    }
   }
 
   // -fmodule-name specifies the module that is currently being built (or
   // used for header checking by -fmodule-maps).
   Args.AddLastArg(CmdArgs, options::OPT_fmodule_name_EQ);
 
   // -fmodule-map-file can be used to specify files containing module
   // definitions.
   Args.AddAllArgs(CmdArgs, options::OPT_fmodule_map_file);
 
   // -fbuiltin-module-map can be used to load the clang
   // builtin headers modulemap file.
   if (Args.hasArg(options::OPT_fbuiltin_module_map)) {
     SmallString<128> BuiltinModuleMap(D.ResourceDir);
     llvm::sys::path::append(BuiltinModuleMap, "include");
     llvm::sys::path::append(BuiltinModuleMap, "module.modulemap");
     if (llvm::sys::fs::exists(BuiltinModuleMap)) {
       CmdArgs.push_back(Args.MakeArgString("-fmodule-map-file=" +
                                            BuiltinModuleMap));
     }
   }
 
-  // -fmodule-file can be used to specify files containing precompiled modules.
+  // The -fmodule-file=<name>=<file> form specifies the mapping of module
+  // names to precompiled module files (the module is loaded only if used).
+  // The -fmodule-file=<file> form can be used to unconditionally load
+  // precompiled module files (whether used or not).
   if (HaveAnyModules)
     Args.AddAllArgs(CmdArgs, options::OPT_fmodule_file);
   else
     Args.ClaimAllArgs(options::OPT_fmodule_file);
 
   // When building modules and generating crashdumps, we need to dump a module
   // dependency VFS alongside the output.
   if (HaveClangModules && C.isForDiagnostics()) {

cfe/trunk/lib/Frontend/CompilerInstance.cpp

CompilerInstance::loadModule(SourceLocation ImportLoc,
   llvm::DenseMap<const IdentifierInfo *, clang::Module *>::iterator Known
     = KnownModules.find(Path[0].first);
   if (Known != KnownModules.end()) {
     // Retrieve the cached top-level module.
     Module = Known->second;    
   } else if (ModuleName == getLangOpts().CurrentModule) {
     // This is the module we're building. 
     Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+    /// FIXME: perhaps we should (a) look for a module using the module name
+    //  to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
+    //if (Module == nullptr) {
+    //  getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
+    //    << ModuleName;
+    //  ModuleBuildFailed = true;
+    //  return ModuleLoadResult();
+    //}
     Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
   } else {
     // Search for a module with the given name.
     Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
     HeaderSearchOptions &HSOpts =
         PP->getHeaderSearchInfo().getHeaderSearchOpts();
 
     std::string ModuleFileName;
     enum ModuleSource {
       ModuleNotFound, ModuleCache, PrebuiltModulePath, ModuleBuildPragma
     } Source = ModuleNotFound;
 
     // Check to see if the module has been built as part of this compilation
     // via a module build pragma.
     auto BuiltModuleIt = BuiltModules.find(ModuleName);
     if (BuiltModuleIt != BuiltModules.end()) {
       ModuleFileName = BuiltModuleIt->second;
       Source = ModuleBuildPragma;
     }
 
     // Try to load the module from the prebuilt module path.
-    if (Source == ModuleNotFound && !HSOpts.PrebuiltModulePaths.empty()) {
-      ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(
-          ModuleName, "", /*UsePrebuiltPath*/ true);
+    if (Source == ModuleNotFound && (!HSOpts.PrebuiltModuleFiles.empty() ||
+                                     !HSOpts.PrebuiltModulePaths.empty())) {
+      ModuleFileName =
+        PP->getHeaderSearchInfo().getPrebuiltModuleFileName(ModuleName);
       if (!ModuleFileName.empty())
         Source = PrebuiltModulePath;
     }
 
     // Try to load the module from the module cache.
     if (Source == ModuleNotFound && Module) {
-      ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(Module);
+      ModuleFileName = PP->getHeaderSearchInfo().getCachedModuleFileName(Module);
       Source = ModuleCache;
     }
 
     if (Source == ModuleNotFound) {
       // We can't find a module, error out here.
       getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
           << ModuleName << SourceRange(ImportLoc, ModuleNameLoc);
       ModuleBuildFailed = true;

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
   Opts.ModuleDependencyOutputDir =
       Args.getLastArgValue(OPT_module_dependency_dir);
   if (Args.hasArg(OPT_MV))
     Opts.OutputFormat = DependencyOutputFormat::NMake;
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
   Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry);
-  auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
-  Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(),
-                        ModuleFiles.end());
+  // Only the -fmodule-file=<file> form.
+  for (const Arg *A : Args.filtered(OPT_fmodule_file)) {
+    StringRef Val = A->getValue();
+    if (Val.find('=') == StringRef::npos)
+      Opts.ExtraDeps.push_back(Val);
+  }
 }
 
 static bool parseShowColorsArgs(const ArgList &Args, bool DefaultColor) {
   // Color diagnostics default to auto ("on" if terminal supports) in the driver
   // but default to off in cc1, needing an explicit OPT_fdiagnostics_color.
   // Support both clang's -f[no-]color-diagnostics and gcc's
   // -f[no-]diagnostics-colors[=never|always|auto].
   enum {
static InputKind ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
   Opts.FixToTemporaries = Args.hasArg(OPT_fixit_to_temp);
   Opts.ASTDumpDecls = Args.hasArg(OPT_ast_dump);
   Opts.ASTDumpAll = Args.hasArg(OPT_ast_dump_all);
   Opts.ASTDumpFilter = Args.getLastArgValue(OPT_ast_dump_filter);
   Opts.ASTDumpLookups = Args.hasArg(OPT_ast_dump_lookups);
   Opts.UseGlobalModuleIndex = !Args.hasArg(OPT_fno_modules_global_index);
   Opts.GenerateGlobalModuleIndex = Opts.UseGlobalModuleIndex;
   Opts.ModuleMapFiles = Args.getAllArgValues(OPT_fmodule_map_file);
-  Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
+  // Only the -fmodule-file=<file> form.
+  for (const Arg *A : Args.filtered(OPT_fmodule_file)) {
+    StringRef Val = A->getValue();
+    if (Val.find('=') == StringRef::npos)
+      Opts.ModuleFiles.push_back(Val);
+  }
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
     = Args.hasArg(OPT_code_completion_macros);
   Opts.CodeCompleteOpts.IncludeCodePatterns
     = Args.hasArg(OPT_code_completion_patterns);
if (WorkingDir.empty())
       llvm::sys::fs::make_absolute(P);
     else
       llvm::sys::fs::make_absolute(WorkingDir, P);
   }
   llvm::sys::path::remove_dots(P);
   Opts.ModuleCachePath = P.str();
 
   Opts.ModuleUserBuildPath = Args.getLastArgValue(OPT_fmodules_user_build_path);
+  // Only the -fmodule-file=<name>=<file> form.
+  for (const Arg *A : Args.filtered(OPT_fmodule_file)) {
+    StringRef Val = A->getValue();
+    if (Val.find('=') != StringRef::npos)
+      Opts.PrebuiltModuleFiles.insert(Val.split('='));
+  }
   for (const Arg *A : Args.filtered(OPT_fprebuilt_module_path))
     Opts.AddPrebuiltModulePath(A->getValue());
   Opts.DisableModuleHash = Args.hasArg(OPT_fdisable_module_hash);
   Opts.ModulesHashContent = Args.hasArg(OPT_fmodules_hash_content);
   Opts.ModulesValidateDiagnosticOptions =
       !Args.hasArg(OPT_fmodules_disable_diagnostic_validation);
   Opts.ImplicitModuleMaps = Args.hasArg(OPT_fimplicit_module_maps);
   Opts.ModuleMapFileHomeIsCwd = Args.hasArg(OPT_fmodule_map_file_home_is_cwd);

cfe/trunk/lib/Frontend/FrontendActions.cpp

GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance &CI,
   // in the module cache.
   if (CI.getFrontendOpts().OutputFile.empty()) {
     StringRef ModuleMapFile = CI.getFrontendOpts().OriginalModuleMap;
     if (ModuleMapFile.empty())
       ModuleMapFile = InFile;
 
     HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
     CI.getFrontendOpts().OutputFile =
-        HS.getModuleFileName(CI.getLangOpts().CurrentModule, ModuleMapFile,
-                             /*UsePrebuiltPath=*/false);
+        HS.getCachedModuleFileName(CI.getLangOpts().CurrentModule,
+                                   ModuleMapFile);
   }
 
   // We use createOutputFile here because this is exposed via libclang, and we
   // must disable the RemoveFileOnSignal behavior.
   // We use a temporary to avoid race conditions.
   return CI.createOutputFile(CI.getFrontendOpts().OutputFile, /*Binary=*/true,
                              /*RemoveFileOnSignal=*/false, InFile,
                              /*Extension=*/"", /*useTemporary=*/true,

cfe/trunk/lib/Lex/HeaderSearch.cpp

 
 /// \brief Get filenames for all registered header maps.
 void HeaderSearch::getHeaderMapFileNames(
     SmallVectorImpl<std::string> &Names) const {
   for (auto &HM : HeaderMaps)
     Names.push_back(HM.first->getName());
 }
 
-std::string HeaderSearch::getModuleFileName(Module *Module) {
+std::string HeaderSearch::getCachedModuleFileName(Module *Module) {
   const FileEntry *ModuleMap =
       getModuleMap().getModuleMapFileForUniquing(Module);
-  return getModuleFileName(Module->Name, ModuleMap->getName(),
-                           /*UsePrebuiltPath*/false);
+  return getCachedModuleFileName(Module->Name, ModuleMap->getName());
 }
 
-std::string HeaderSearch::getModuleFileName(StringRef ModuleName,
-                                            StringRef ModuleMapPath,
-                                            bool UsePrebuiltPath) {
-  if (UsePrebuiltPath) {
-    if (HSOpts->PrebuiltModulePaths.empty())
+std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName,
+                                                    bool FileMapOnly) {
+  // First check the module name to pcm file map.
+  auto i (HSOpts->PrebuiltModuleFiles.find(ModuleName));
+  if (i != HSOpts->PrebuiltModuleFiles.end())
+    return i->second;
+
+  if (FileMapOnly || HSOpts->PrebuiltModulePaths.empty())
       return std::string();
 
-    // Go though each prebuilt module path and try to find the pcm file.
+  // Then go through each prebuilt module directory and try to find the pcm
+  // file.
     for (const std::string &Dir : HSOpts->PrebuiltModulePaths) {
       SmallString<256> Result(Dir);
       llvm::sys::fs::make_absolute(Result);
 
       llvm::sys::path::append(Result, ModuleName + ".pcm");
       if (getFileMgr().getFile(Result.str()))
         return Result.str().str();
     }
     return std::string();
   }
 
+std::string HeaderSearch::getCachedModuleFileName(StringRef ModuleName,
+                                                  StringRef ModuleMapPath) {
   // If we don't have a module cache path or aren't supposed to use one, we
   // can't do anything.
   if (getModuleCachePath().empty())
     return std::string();
 
   SmallString<256> Result(getModuleCachePath());
   llvm::sys::fs::make_absolute(Result);
 

cfe/trunk/lib/Serialization/ASTReader.cpp

case IMPORTS: {
         SourceLocation ImportLoc =
             ReadUntranslatedSourceLocation(Record[Idx++]);
         off_t StoredSize = (off_t)Record[Idx++];
         time_t StoredModTime = (time_t)Record[Idx++];
         ASTFileSignature StoredSignature = {
             {{(uint32_t)Record[Idx++], (uint32_t)Record[Idx++],
               (uint32_t)Record[Idx++], (uint32_t)Record[Idx++],
               (uint32_t)Record[Idx++]}}};
-        auto ImportedFile = ReadPath(F, Record, Idx);
+
+        std::string ImportedName = ReadString(Record, Idx);
+        std::string ImportedFile;
+
+        // For prebuilt and explicit modules first consult the file map for
+        // an override. Note that here we don't search prebuilt module
+        // directories, only the explicit name to file mappings. Also, we will
+        // still verify the size/signature making sure it is essentially the
+        // same file but perhaps in a different location.
+        if (ImportedKind == MK_PrebuiltModule || ImportedKind == MK_ExplicitModule)
+          ImportedFile = PP.getHeaderSearchInfo().getPrebuiltModuleFileName(
+            ImportedName, /*FileMapOnly*/ true);
+
+        if (ImportedFile.empty())
+          ImportedFile = ReadPath(F, Record, Idx);
+        else
+          SkipPath(Record, Idx);
 
         // If our client can't cope with us being out of date, we can't cope with
         // our dependency being missing.
         unsigned Capabilities = ClientLoadCapabilities;
         if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
           Capabilities &= ~ARR_Missing;
 
         // Load the AST file.
void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const {
   RemapBuilder MacroRemap(F.MacroRemap);
   RemapBuilder PreprocessedEntityRemap(F.PreprocessedEntityRemap);
   RemapBuilder SubmoduleRemap(F.SubmoduleRemap);
   RemapBuilder SelectorRemap(F.SelectorRemap);
   RemapBuilder DeclRemap(F.DeclRemap);
   RemapBuilder TypeRemap(F.TypeRemap);
 
   while (Data < DataEnd) {
-    // FIXME: Looking up dependency modules by filename is horrible.
+    // FIXME: Looking up dependency modules by filename is horrible. Let's
+    // start fixing this with prebuilt and explicit modules and see how it
+    // goes...
     using namespace llvm::support;
+    ModuleKind Kind = static_cast<ModuleKind>(
+      endian::readNext<uint8_t, little, unaligned>(Data));
     uint16_t Len = endian::readNext<uint16_t, little, unaligned>(Data);
     StringRef Name = StringRef((const char*)Data, Len);
     Data += Len;
-    ModuleFile *OM = ModuleMgr.lookup(Name);
+    ModuleFile *OM = (Kind == MK_PrebuiltModule || Kind == MK_ExplicitModule
+                      ? ModuleMgr.lookupByModuleName(Name)
+                      : ModuleMgr.lookupByFileName(Name));
     if (!OM) {
       std::string Msg =
           "SourceLocation remap refers to unknown module, cannot find ";
       Msg.append(Name);
       Error(Msg);
       return;
     }
 
while (!DoneWithControlBlock) {
     case IMPORTS: {
       if (!NeedsImports)
         break;
 
       unsigned Idx = 0, N = Record.size();
       while (Idx < N) {
         // Read information about the AST file.
         Idx += 5; // ImportLoc, Size, ModTime, Signature
+        SkipString(Record, Idx); // Module name; FIXME: pass to listener?
         std::string Filename = ReadString(Record, Idx);
         ResolveImportedPath(Filename, ModuleDir);
         Listener.visitImport(Filename);
       }
       break;
     }
 
     default:
ASTReader::ASTReader(Preprocessor &PP, ASTContext *Context,
                      bool UseGlobalIndex,
                      std::unique_ptr<llvm::Timer> ReadTimer)
     : Listener(DisableValidation
                    ? cast<ASTReaderListener>(new SimpleASTReaderListener(PP))
                    : cast<ASTReaderListener>(new PCHValidator(PP, *this))),
       SourceMgr(PP.getSourceManager()), FileMgr(PP.getFileManager()),
       PCHContainerRdr(PCHContainerRdr), Diags(PP.getDiagnostics()), PP(PP),
       ContextObj(Context),
-      ModuleMgr(PP.getFileManager(), PP.getPCMCache(), PCHContainerRdr),
+      ModuleMgr(PP.getFileManager(), PP.getPCMCache(), PCHContainerRdr,
+                PP.getHeaderSearchInfo()),
       PCMCache(PP.getPCMCache()), DummyIdResolver(PP),
       ReadTimer(std::move(ReadTimer)), isysroot(isysroot),
       DisableValidation(DisableValidation),
       AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
       AllowConfigurationMismatch(AllowConfigurationMismatch),
       ValidateSystemInputs(ValidateSystemInputs),
       UseGlobalIndex(UseGlobalIndex), CurrSwitchCaseStmts(&SwitchCaseStmts) {
   SourceMgr.setExternalSLocEntrySource(this);

cfe/trunk/lib/Serialization/ASTWriter.cpp

for (ModuleFile &M : Mgr) {
       // If we have calculated signature, there is no need to store
       // the size or timestamp.
       Record.push_back(M.Signature ? 0 : M.File->getSize());
       Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File));
 
       for (auto I : M.Signature)
         Record.push_back(I);
 
+      AddString(M.ModuleName, Record);
       AddPath(M.FileName, Record);
     }
     Stream.EmitRecord(IMPORTS, Record);
   }
 
   // Write the options block.
   Stream.EnterSubblock(OPTIONS_BLOCK_ID, 4);
 
ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
   AddTypeRef(Context.ObjCSelRedefinitionType, SpecialTypes);
   AddTypeRef(Context.getucontext_tType(), SpecialTypes);
 
   if (Chain) {
     // Write the mapping information describing our module dependencies and how
     // each of those modules were mapped into our own offset/ID space, so that
     // the reader can build the appropriate mapping to its own offset/ID space.
     // The map consists solely of a blob with the following format:
-    // *(module-name-len:i16 module-name:len*i8
+    // *(module-kind:i8
+    //   module-name-len:i16 module-name:len*i8
     //   source-location-offset:i32
     //   identifier-id:i32
     //   preprocessed-entity-id:i32
     //   macro-definition-id:i32
     //   submodule-id:i32
     //   selector-id:i32
     //   declaration-id:i32
     //   c++-base-specifiers-id:i32
     //   type-id:i32)
     // 
+    // module-kind is the ModuleKind enum value. If it is MK_PrebuiltModule or
+    // MK_ExplicitModule, then the module-name is the module name. Otherwise,
+    // it is the module file name.
+    //
     auto Abbrev = std::make_shared<BitCodeAbbrev>();
     Abbrev->Add(BitCodeAbbrevOp(MODULE_OFFSET_MAP));
     Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
     unsigned ModuleOffsetMapAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
     SmallString<2048> Buffer;
     {
       llvm::raw_svector_ostream Out(Buffer);
       for (ModuleFile &M : Chain->ModuleMgr) {
         using namespace llvm::support;
         endian::Writer<little> LE(Out);
-        StringRef FileName = M.FileName;
-        LE.write<uint16_t>(FileName.size());
-        Out.write(FileName.data(), FileName.size());
+        LE.write<uint8_t>(static_cast<uint8_t>(M.Kind));
+        StringRef Name =
+          M.Kind == MK_PrebuiltModule || M.Kind == MK_ExplicitModule
+          ? M.ModuleName
+          : M.FileName;
+        LE.write<uint16_t>(Name.size());
+        Out.write(Name.data(), Name.size());
 
         // Note: if a base ID was uint max, it would not be possible to load
         // another module after it or have more than one entity inside it.
         uint32_t None = std::numeric_limits<uint32_t>::max();
 
         auto writeBaseIDOrNone = [&](uint32_t BaseID, bool ShouldWrite) {
           assert(BaseID < std::numeric_limits<uint32_t>::max() && "base id too high");
           if (ShouldWrite)

cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp

if (State == ControlBlock && Code == IMPORTS) {
 
         // Skip the stored signature.
         // FIXME: we could read the signature out of the import and validate it.
         ASTFileSignature StoredSignature = {
             {{(uint32_t)Record[Idx++], (uint32_t)Record[Idx++],
               (uint32_t)Record[Idx++], (uint32_t)Record[Idx++],
               (uint32_t)Record[Idx++]}}};
 
+        // Skip the module name (currently this is only used for prebuilt
+        // modules while here we are only dealing with cached).
+        Idx += Record[Idx] + 1;
+
         // Retrieve the imported file name.
         unsigned Length = Record[Idx++];
         SmallString<128> ImportedFile(Record.begin() + Idx,
                                       Record.begin() + Idx + Length);
         Idx += Length;
 
         // Find the imported module file.
         const FileEntry *DependsOnFile

cfe/trunk/lib/Serialization/ModuleManager.cpp

 
 #ifndef NDEBUG
 #include "llvm/Support/GraphWriter.h"
 #endif
 
 using namespace clang;
 using namespace serialization;
 
-ModuleFile *ModuleManager::lookup(StringRef Name) const {
+ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const {
   const FileEntry *Entry = FileMgr.getFile(Name, /*openFile=*/false,
                                            /*cacheFailure=*/false);
   if (Entry)
     return lookup(Entry);
 
   return nullptr;
 }
 
+ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const {
+  if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name))
+    if (const FileEntry *File = Mod->getASTFile())
+      return lookup(File);
+
+  return nullptr;
+}
+
 ModuleFile *ModuleManager::lookup(const FileEntry *File) const {
   auto Known = Modules.find(File);
   if (Known == Modules.end())
     return nullptr;
 
   return Known->second;
 }
 

 void ModuleManager::moduleFileAccepted(ModuleFile *MF) {
   if (!GlobalIndex || GlobalIndex->loadedModuleFile(MF))
     return;
 
   ModulesInCommonWithGlobalIndex.push_back(MF);
 }
 
 ModuleManager::ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache,
-                             const PCHContainerReader &PCHContainerRdr)
+                             const PCHContainerReader &PCHContainerRdr,
+                             const HeaderSearch& HeaderSearchInfo)
     : FileMgr(FileMgr), PCMCache(&PCMCache), PCHContainerRdr(PCHContainerRdr),
-      GlobalIndex(), FirstVisitState(nullptr) {}
+      HeaderSearchInfo (HeaderSearchInfo), GlobalIndex(),
+      FirstVisitState(nullptr) {}
 
 ModuleManager::~ModuleManager() { delete FirstVisitState; }
 
 void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> Visitor,
                           llvm::SmallPtrSetImpl<ModuleFile *> *ModuleFilesHit) {
   // If the visitation order vector is the wrong size, recompute the order.
   if (VisitOrder.size() != Chain.size()) {
     unsigned N = size();

cfe/trunk/test/CXX/modules-ts/basic/basic.search/module-import.cpp

+// Tests for imported module search.
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo 'export module x; int a, b;' > %t/x.cppm
+// RUN: echo 'export module y; import x; int c;' > %t/y.cppm
+// RUN: echo 'export module z; import y; int d;' > %t/z.cppm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface %t/x.cppm -o %t/x.pcm
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=%t/x.pcm %t/y.cppm -o %t/y.pcm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/x.pcm -verify %s \
+// RUN:            -DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/y.pcm -verify %s \
+// RUN:            -DMODULE_NAME=y
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=x=%t/x.pcm -verify %s \
+// RUN:            -DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=y=%t/y.pcm -verify %s \
+// RUN:            -DMODULE_NAME=y
+//
+// RUN: mv %t/x.pcm %t/a.pcm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:            -DMODULE_NAME=x
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:            -DMODULE_NAME=y
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:            -DMODULE_NAME=y
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -emit-module-interface -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm %t/z.cppm -o %t/z.pcm
+//
+// RUN: %clang_cc1 -std=c++1z -fmodules-ts -I%t -fmodule-file=z=%t/z.pcm -fmodule-file=y=%t/y.pcm -fmodule-file=x=%t/a.pcm -verify %s \
+// RUN:            -DMODULE_NAME=z
+//
+
+import MODULE_NAME;
+
+// expected-no-diagnostics