Allow multiple modules with the same name to coexist in the module cache

To differentiate between two modules with the same name, we will
consider the path the module map file that they are defined by* part of
the ‘key’ for looking up the precompiled module (pcm file).
Specifically, this patch renames the precompiled module (pcm) files from
  cache-path/<module hash>/Foo.pcm
to
  cache-path/<module hash>/Foo-<hash of module map path>.pcm

In addition, I’ve taught the ASTReader to re-resolve the names of
imported modules during module loading so that if the header search
context changes between when a module was originally built and when it
is loaded we can rebuild it if necessary.  For example, if module A
imports module B

first time:
clang -I /path/to/A -I /path/to/B ...

second time:
clang -I /path/to/A -I /different/path/to/B ...

will now rebuild A as expected.

* in the case of inferred modules, we use the module map file that
allowed the inference, not the __inferred_module.map file, since the
inferred file path is the same for every inferred module.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@206201 91177308-0d34-0410-b5e6-96231b3b80d8

Author: benlangmuir

Diff for: include/clang/Basic/DiagnosticSerializationKinds.td

@@ -48,7 +48,12 @@ def err_pch_different_branch : Error<
     "PCH file built from a different branch (%0) than the compiler (%1)">;
 def err_pch_with_compiler_errors : Error<
     "PCH file contains compiler errors">;
-    
+
+def err_imported_module_not_found : Error<
+    "module '%0' imported by AST file '%1' not found">, DefaultFatal;
+def err_imported_module_modmap_changed : Error<
+    "module '%0' imported by AST file '%1' found in a different module map file"
+    " (%2) than when the importing AST file was built (%3)">, DefaultFatal;
 def warn_module_conflict : Warning<
     "module '%0' conflicts with already-imported module '%1': %2">, 
     InGroup<ModuleConflict>;

Diff for: include/clang/Basic/Module.h

@@ -51,10 +51,21 @@ class Module {
 
   /// \brief The location of the module definition.
   SourceLocation DefinitionLoc;
-  
+
   /// \brief The parent of this module. This will be NULL for the top-level
   /// module.
   Module *Parent;
+
+  /// \brief The module map file that (along with the module name) uniquely
+  /// identifies this module.
+  ///
+  /// The particular module that \c Name refers to may depend on how the module
+  /// was found in header search. However, the combination of \c Name and
+  /// \c ModuleMap will be globally unique for top-level modules. In the case of
+  /// inferred modules, \c ModuleMap will contain the module map that allowed
+  /// the inference (e.g. contained 'Module *') rather than the virtual
+  /// inferred module map file.
+  const FileEntry *ModuleMap;
 
   /// \brief The umbrella header or directory.
   llvm::PointerUnion<const DirectoryEntry *, const FileEntry *> Umbrella;
@@ -264,8 +275,10 @@ class Module {
   std::vector<Conflict> Conflicts;
 
   /// \brief Construct a new module or submodule.
-  Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent, 
-         bool IsFramework, bool IsExplicit);
+  ///
+  /// For an explanation of \p ModuleMap, see Module::ModuleMap.
+  Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
+         const FileEntry *ModuleMap, bool IsFramework, bool IsExplicit);
 
   ~Module();
 

Diff for: include/clang/Frontend/FrontendActions.h

@@ -17,6 +17,7 @@
 namespace clang {
 
 class Module;
+class FileEntry;
 
 //===----------------------------------------------------------------------===//
 // Custom Consumer Actions
@@ -92,7 +93,8 @@ class GeneratePCHAction : public ASTFrontendAction {
 };
 
 class GenerateModuleAction : public ASTFrontendAction {
-  clang::Module *Module;
+  Module *Module;
+  const FileEntry *ModuleMapForUniquing;
   bool IsSystem;
 
 protected:
@@ -106,20 +108,22 @@ class GenerateModuleAction : public ASTFrontendAction {
   bool hasASTFileSupport() const override { return false; }
 
 public:
-  explicit GenerateModuleAction(bool IsSystem = false)
-    : ASTFrontendAction(), IsSystem(IsSystem) { }
+  GenerateModuleAction(const FileEntry *ModuleMap = nullptr,
+                       bool IsSystem = false)
+    : ASTFrontendAction(), ModuleMapForUniquing(ModuleMap), IsSystem(IsSystem)
+  { }
 
   bool BeginSourceFileAction(CompilerInstance &CI, StringRef Filename) override;
 
   /// \brief Compute the AST consumer arguments that will be used to
   /// create the PCHGenerator instance returned by CreateASTConsumer.
   ///
   /// \returns true if an error occurred, false otherwise.
-  static bool ComputeASTConsumerArguments(CompilerInstance &CI,
-                                          StringRef InFile,
-                                          std::string &Sysroot,
-                                          std::string &OutputFile,
-                                          raw_ostream *&OS);
+  bool ComputeASTConsumerArguments(CompilerInstance &CI,
+                                   StringRef InFile,
+                                   std::string &Sysroot,
+                                   std::string &OutputFile,
+                                   raw_ostream *&OS);
 };
 
 class SyntaxOnlyAction : public ASTFrontendAction {

Diff for: include/clang/Lex/HeaderSearch.h

@@ -493,9 +493,12 @@ class HeaderSearch {
   ///
   /// \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.
+  ///
   /// \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);
+  std::string getModuleFileName(StringRef ModuleName, StringRef ModuleMapPath);
 
   /// \brief Lookup a module Search for a module with the given name.
   ///

Diff for: include/clang/Lex/ModuleMap.h

@@ -131,6 +131,10 @@ class ModuleMap {
     /// \brief Whether the modules we infer are [system] modules.
     unsigned InferSystemModules : 1;
 
+    /// \brief If \c InferModules is non-zero, the module map file that allowed
+    /// inferred modules.  Otherwise, nullptr.
+    const FileEntry *ModuleMapFile;
+
     /// \brief The names of modules that cannot be inferred within this
     /// directory.
     SmallVector<std::string, 2> ExcludedModules;
@@ -293,13 +297,17 @@ class ModuleMap {
   /// \param Parent The module that will act as the parent of this submodule,
   /// or NULL to indicate that this is a top-level module.
   ///
+  /// \param ModuleMap The module map that defines or allows the inference of
+  /// this module.
+  ///
   /// \param IsFramework Whether this is a framework module.
   ///
   /// \param IsExplicit Whether this is an explicit submodule.
   ///
   /// \returns The found or newly-created module, along with a boolean value
   /// that will be true if the module is newly-created.
-  std::pair<Module *, bool> findOrCreateModule(StringRef Name, Module *Parent, 
+  std::pair<Module *, bool> findOrCreateModule(StringRef Name, Module *Parent,
+                                               const FileEntry *ModuleMap,
                                                bool IsFramework,
                                                bool IsExplicit);
 

Diff for: include/clang/Serialization/ASTBitCodes.h

@@ -281,7 +281,14 @@ namespace clang {
       HEADER_SEARCH_OPTIONS = 11,
 
       /// \brief Record code for the preprocessor options table.
-      PREPROCESSOR_OPTIONS = 12
+      PREPROCESSOR_OPTIONS = 12,
+
+      /// \brief Record code for the module name.
+      MODULE_NAME = 13,
+
+      /// \brief Record code for the module map file that was used to build this
+      /// AST file.
+      MODULE_MAP_FILE = 14
     };
 
     /// \brief Record types that occur within the input-files block

Diff for: include/clang/Serialization/ASTReader.h

@@ -1079,6 +1079,7 @@ class ASTReader
                             unsigned ClientLoadCapabilities);
   ASTReadResult ReadControlBlock(ModuleFile &F,
                                  SmallVectorImpl<ImportedModule> &Loaded,
+                                 const ModuleFile *ImportedBy,
                                  unsigned ClientLoadCapabilities);
   ASTReadResult ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities);
   bool ParseLineTable(ModuleFile &F, SmallVectorImpl<uint64_t> &Record);

Diff for: include/clang/Serialization/Module.h

@@ -116,6 +116,9 @@ class ModuleFile {
   /// \brief The file name of the module file.
   std::string FileName;
 
+  /// \brief The name of the module.
+  std::string ModuleName;
+
   std::string getTimestampFilename() const {
     return FileName + ".timestamp";
   }
@@ -137,6 +140,8 @@ class ModuleFile {
   /// allow resolving headers even after headers+PCH was moved to a new path.
   std::string OriginalDir;
 
+  std::string ModuleMapPath;
+
   /// \brief Whether this precompiled header is a relocatable PCH file.
   bool RelocatablePCH;
 

Diff for: lib/Basic/Module.cpp

@@ -25,8 +25,8 @@
 using namespace clang;
 
 Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
-               bool IsFramework, bool IsExplicit)
-  : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent),
+               const FileEntry *File, bool IsFramework, bool IsExplicit)
+  : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent), ModuleMap(File),
     Umbrella(), ASTFile(0), IsAvailable(true), IsFromModuleFile(false),
     IsFramework(IsFramework), IsExplicit(IsExplicit), IsSystem(false),
     IsExternC(false), InferSubmodules(false), InferExplicitSubmodules(false),

Diff for: lib/Frontend/CompilerInstance.cpp

@@ -870,8 +870,9 @@ static void compileModuleImpl(CompilerInstance &ImportingInstance,
     SourceMgr.overrideFileContents(ModuleMapFile, ModuleMapBuffer);
   }
 
-  // Construct a module-generating action.
-  GenerateModuleAction CreateModuleAction(Module->IsSystem);
+  // Construct a module-generating action. Passing through Module->ModuleMap is
+  // safe because the FileManager is shared between the compiler instances.
+  GenerateModuleAction CreateModuleAction(Module->ModuleMap, Module->IsSystem);
 
   // Execute the action to actually build the module in-place. Use a separate
   // thread so that we get a stack large enough.

Diff for: lib/Frontend/FrontendAction.cpp

@@ -256,6 +256,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     if (!BeginSourceFileAction(CI, InputFile))
       goto failure;
 
+    // Initialize the main file entry.
+    if (!CI.InitializeSourceManager(CurrentInput))
+      goto failure;
+
     return true;
   }
 
@@ -302,6 +306,11 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
   if (!BeginSourceFileAction(CI, InputFile))
     goto failure;
 
+  // Initialize the main file entry. It is important that this occurs after
+  // BeginSourceFileAction, which may change CurrentInput during module builds.
+  if (!CI.InitializeSourceManager(CurrentInput))
+    goto failure;
+
   // Create the AST context and consumer unless this is a preprocessor only
   // action.
   if (!usesPreprocessorOnly()) {
@@ -389,13 +398,6 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
 bool FrontendAction::Execute() {
   CompilerInstance &CI = getCompilerInstance();
 
-  // Initialize the main file entry. This needs to be delayed until after PCH
-  // has loaded.
-  if (!isCurrentFileAST()) {
-    if (!CI.InitializeSourceManager(getCurrentInput()))
-      return false;
-  }
-
   if (CI.hasFrontendTimer()) {
     llvm::TimeRegion Timer(CI.getFrontendTimer());
     ExecuteAction();

Diff for: lib/Frontend/FrontendActions.cpp

@@ -299,6 +299,11 @@ bool GenerateModuleAction::BeginSourceFileAction(CompilerInstance &CI,
     return false;
   }
 
+  if (!ModuleMapForUniquing)
+    ModuleMapForUniquing = ModuleMap;
+  Module->ModuleMap = ModuleMapForUniquing;
+  assert(Module->ModuleMap && "missing module map file");
+
   FileManager &FileMgr = CI.getFileManager();
 
   // Collect the set of #includes we need to build the module.
@@ -337,10 +342,9 @@ bool GenerateModuleAction::ComputeASTConsumerArguments(CompilerInstance &CI,
   // in the module cache.
   if (CI.getFrontendOpts().OutputFile.empty()) {
     HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
-    SmallString<256> ModuleFileName(HS.getModuleCachePath());
-    llvm::sys::path::append(ModuleFileName, 
-                            CI.getLangOpts().CurrentModule + ".pcm");
-    CI.getFrontendOpts().OutputFile = ModuleFileName.str();
+    CI.getFrontendOpts().OutputFile =
+        HS.getModuleFileName(CI.getLangOpts().CurrentModule,
+                             ModuleMapForUniquing->getName());
   }
 
   // We use createOutputFile here because this is exposed via libclang, and we

Diff for: lib/Lex/HeaderSearch.cpp

@@ -18,6 +18,8 @@
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/FileSystem.h"
@@ -112,24 +114,33 @@ const HeaderMap *HeaderSearch::CreateHeaderMap(const FileEntry *FE) {
 }
 
 std::string HeaderSearch::getModuleFileName(Module *Module) {
-  // If we don't have a module cache path, we can't do anything.
-  if (ModuleCachePath.empty()) 
-    return std::string();
-
-
-  SmallString<256> Result(ModuleCachePath);
-  llvm::sys::path::append(Result, Module->getTopLevelModule()->Name + ".pcm");
-  return Result.str().str();
+  return getModuleFileName(Module->Name, Module->ModuleMap->getName());
 }
 
-std::string HeaderSearch::getModuleFileName(StringRef ModuleName) {
+std::string HeaderSearch::getModuleFileName(StringRef ModuleName,
+                                            StringRef ModuleMapPath) {
   // If we don't have a module cache path, we can't do anything.
   if (ModuleCachePath.empty()) 
     return std::string();
-  
-  
+
   SmallString<256> Result(ModuleCachePath);
-  llvm::sys::path::append(Result, ModuleName + ".pcm");
+  llvm::sys::fs::make_absolute(Result);
+
+  if (HSOpts->DisableModuleHash) {
+    llvm::sys::path::append(Result, ModuleName + ".pcm");
+  } else {
+    // Construct the name <ModuleName>-<hash of ModuleMapPath>.pcm which should
+    // be globally unique to this particular module. To avoid false-negatives
+    // on case-insensitive filesystems, we use lower-case, which is safe because
+    // to cause a collision the modules must have the same name, which is an
+    // error if they are imported in the same translation.
+    SmallString<256> AbsModuleMapPath(ModuleMapPath);
+    llvm::sys::fs::make_absolute(AbsModuleMapPath);
+    llvm::APInt Code(64, llvm::hash_value(AbsModuleMapPath.str().lower()));
+    SmallString<128> HashStr;
+    Code.toStringUnsigned(HashStr, /*Radix*/36);
+    llvm::sys::path::append(Result, ModuleName + "-" + HashStr.str() + ".pcm");
+  }
   return Result.str().str();
 }