r215268 - Refactor the module map file used for uniquing a module name out of

Ben Langmuir blangmuir at apple.com
Fri Aug 8 17:57:23 PDT 2014


Author: benlangmuir
Date: Fri Aug  8 19:57:23 2014
New Revision: 215268

URL: http://llvm.org/viewvc/llvm-project?rev=215268&view=rev
Log:
Refactor the module map file used for uniquing a module name out of

class Module. It's almost always going to be the same as
getContainingModule() for top-level modules, so just add a map to cover
the remaining cases.  This lets us do less bookkeeping to keep the
ModuleMap fields up to date.

Modified:
    cfe/trunk/include/clang/Basic/Module.h
    cfe/trunk/include/clang/Lex/ModuleMap.h
    cfe/trunk/lib/Basic/Module.cpp
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/lib/Frontend/FrontendActions.cpp
    cfe/trunk/lib/Lex/HeaderSearch.cpp
    cfe/trunk/lib/Lex/ModuleMap.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Fri Aug  8 19:57:23 2014
@@ -55,17 +55,6 @@ public:
   /// \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;
@@ -283,10 +272,8 @@ public:
   std::vector<Conflict> Conflicts;
 
   /// \brief Construct a new module or submodule.
-  ///
-  /// For an explanation of \p ModuleMap, see Module::ModuleMap.
   Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
-         const FileEntry *ModuleMap, bool IsFramework, bool IsExplicit);
+         bool IsFramework, bool IsExplicit);
   
   ~Module();
   

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Fri Aug  8 19:57:23 2014
@@ -146,6 +146,10 @@ private:
   /// framework modules from within those directories.
   llvm::DenseMap<const DirectoryEntry *, InferredDirectory> InferredDirectories;
 
+  /// A mapping from an inferred module to the module map that allowed the
+  /// inference.
+  llvm::DenseMap<const Module *, const FileEntry *> InferredModuleAllowedBy;
+
   /// \brief Describes whether we haved parsed a particular file as a module
   /// map.
   llvm::DenseMap<const FileEntry *, bool> ParsedModuleMap;
@@ -316,7 +320,6 @@ public:
   /// \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,
-                                               const FileEntry *ModuleMap,
                                                bool IsFramework,
                                                bool IsExplicit);
 
@@ -351,6 +354,19 @@ public:
   /// module, or NULL if the module definition was inferred.
   const FileEntry *getContainingModuleMapFile(Module *Module) const;
 
+  /// \brief Get 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
+  /// this module map will be globally unique for top-level modules. In the case
+  /// of inferred modules, returns the module map that allowed the inference
+  /// (e.g. contained 'module *'). Otherwise, returns
+  /// getContainingModuleMapFile().
+  const FileEntry *getModuleMapFileForUniquing(Module *M) const;
+
+  void setInferredModuleAllowedBy(Module *M, const FileEntry *ModuleMap);
+
   /// \brief Resolve all of the unresolved exports in the given module.
   ///
   /// \param Mod The module whose exports should be resolved.

Modified: cfe/trunk/lib/Basic/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Module.cpp (original)
+++ cfe/trunk/lib/Basic/Module.cpp Fri Aug  8 19:57:23 2014
@@ -25,8 +25,8 @@
 using namespace clang;
 
 Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
-               const FileEntry *File, bool IsFramework, bool IsExplicit)
-    : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent), ModuleMap(File),
+               bool IsFramework, bool IsExplicit)
+    : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent),
       Umbrella(), ASTFile(nullptr), IsMissingRequirement(false),
       IsAvailable(true), IsFromModuleFile(false), IsFramework(IsFramework),
       IsExplicit(IsExplicit), IsSystem(false), IsExternC(false),
@@ -361,7 +361,11 @@ void Module::print(raw_ostream &OS, unsi
   
   for (submodule_const_iterator MI = submodule_begin(), MIEnd = submodule_end();
        MI != MIEnd; ++MI)
-    if (!(*MI)->IsInferred)
+    // Print inferred subframework modules so that we don't need to re-infer
+    // them (requires expensive directory iteration + stat calls) when we build
+    // the module. Regular inferred submodules are OK, as we need to look at all
+    // those header files anyway.
+    if (!(*MI)->IsInferred || (*MI)->IsFramework)
       (*MI)->print(OS, Indent + 2);
   
   for (unsigned I = 0, N = Exports.size(); I != N; ++I) {

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Aug  8 19:57:23 2014
@@ -958,9 +958,10 @@ static bool compileModuleImpl(CompilerIn
     SourceMgr.overrideFileContents(ModuleMapFile, ModuleMapBuffer);
   }
 
-  // Construct a module-generating action. Passing through Module->ModuleMap is
+  // Construct a module-generating action. Passing through the module map is
   // safe because the FileManager is shared between the compiler instances.
-  GenerateModuleAction CreateModuleAction(Module->ModuleMap, Module->IsSystem);
+  GenerateModuleAction CreateModuleAction(
+      ModMap.getModuleMapFileForUniquing(Module), Module->IsSystem);
   
   // Execute the action to actually build the module in-place. Use a separate
   // thread so that we get a stack large enough.

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Fri Aug  8 19:57:23 2014
@@ -301,10 +301,12 @@ bool GenerateModuleAction::BeginSourceFi
     return false;
   }
 
-  if (!ModuleMapForUniquing)
+  if (ModuleMapForUniquing && ModuleMapForUniquing != ModuleMap) {
+    Module->IsInferred = true;
+    HS.getModuleMap().setInferredModuleAllowedBy(Module, ModuleMapForUniquing);
+  } else {
     ModuleMapForUniquing = ModuleMap;
-  Module->ModuleMap = ModuleMapForUniquing;
-  assert(Module->ModuleMap && "missing module map file");
+  }
 
   FileManager &FileMgr = CI.getFileManager();
 

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Aug  8 19:57:23 2014
@@ -114,7 +114,9 @@ const HeaderMap *HeaderSearch::CreateHea
 }
 
 std::string HeaderSearch::getModuleFileName(Module *Module) {
-  return getModuleFileName(Module->Name, Module->ModuleMap->getName());
+  const FileEntry *ModuleMap =
+      getModuleMap().getModuleMapFileForUniquing(Module);
+  return getModuleFileName(Module->Name, ModuleMap->getName());
 }
 
 std::string HeaderSearch::getModuleFileName(StringRef ModuleName,

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Aug  8 19:57:23 2014
@@ -367,6 +367,9 @@ ModuleMap::findModuleForHeader(const Fil
       UmbrellaModule = UmbrellaModule->Parent;
 
     if (UmbrellaModule->InferSubmodules) {
+      const FileEntry *UmbrellaModuleMap =
+          getModuleMapFileForUniquing(UmbrellaModule);
+
       // Infer submodules for each of the directories we found between
       // the directory of the umbrella header and the directory where
       // the actual header is located.
@@ -377,8 +380,9 @@ ModuleMap::findModuleForHeader(const Fil
         SmallString<32> NameBuf;
         StringRef Name = sanitizeFilenameAsIdentifier(
             llvm::sys::path::stem(SkippedDirs[I-1]->getName()), NameBuf);
-        Result = findOrCreateModule(Name, Result, UmbrellaModule->ModuleMap,
-                                    /*IsFramework=*/false, Explicit).first;
+        Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
+                                    Explicit).first;
+        InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
         Result->IsInferred = true;
 
         // Associate the module and the directory.
@@ -394,8 +398,9 @@ ModuleMap::findModuleForHeader(const Fil
       SmallString<32> NameBuf;
       StringRef Name = sanitizeFilenameAsIdentifier(
                          llvm::sys::path::stem(File->getName()), NameBuf);
-      Result = findOrCreateModule(Name, Result, UmbrellaModule->ModuleMap,
-                                  /*IsFramework=*/false, Explicit).first;
+      Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
+                                  Explicit).first;
+      InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
       Result->IsInferred = true;
       Result->addTopHeader(File);
 
@@ -535,15 +540,14 @@ Module *ModuleMap::lookupModuleQualified
 }
 
 std::pair<Module *, bool> 
-ModuleMap::findOrCreateModule(StringRef Name, Module *Parent,
-                              const FileEntry *ModuleMap, bool IsFramework,
+ModuleMap::findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
                               bool IsExplicit) {
   // Try to find an existing module with this name.
   if (Module *Sub = lookupModuleQualified(Name, Parent))
     return std::make_pair(Sub, false);
   
   // Create a new module with this name.
-  Module *Result = new Module(Name, SourceLocation(), Parent, ModuleMap,
+  Module *Result = new Module(Name, SourceLocation(), Parent,
                               IsFramework, IsExplicit);
   if (LangOpts.CurrentModule == Name) {
     SourceModule = Result;
@@ -673,7 +677,7 @@ ModuleMap::inferFrameworkModule(StringRe
     if (!canInfer)
       return nullptr;
   } else
-    ModuleMapFile = Parent->ModuleMap;
+    ModuleMapFile = getModuleMapFileForUniquing(Parent);
 
 
   // Look for an umbrella header.
@@ -687,8 +691,10 @@ ModuleMap::inferFrameworkModule(StringRe
   if (!UmbrellaHeader)
     return nullptr;
 
-  Module *Result = new Module(ModuleName, SourceLocation(), Parent, ModuleMapFile,
+  Module *Result = new Module(ModuleName, SourceLocation(), Parent,
                               /*IsFramework=*/true, /*IsExplicit=*/false);
+  InferredModuleAllowedBy[Result] = ModuleMapFile;
+  Result->IsInferred = true;
   if (LangOpts.CurrentModule == ModuleName) {
     SourceModule = Result;
     SourceModuleName = ModuleName;
@@ -799,6 +805,19 @@ ModuleMap::getContainingModuleMapFile(Mo
            SourceMgr.getFileID(Module->DefinitionLoc));
 }
 
+const FileEntry *ModuleMap::getModuleMapFileForUniquing(Module *M) const {
+  if (M->IsInferred) {
+    assert(InferredModuleAllowedBy.count(M) && "missing inferred module map");
+    return InferredModuleAllowedBy.find(M)->second;
+  }
+  return getContainingModuleMapFile(M);
+}
+
+void ModuleMap::setInferredModuleAllowedBy(Module *M, const FileEntry *ModMap) {
+  assert(M->IsInferred && "module not inferred");
+  InferredModuleAllowedBy[M] = ModMap;
+}
+
 void ModuleMap::dump() {
   llvm::errs() << "Modules:";
   for (llvm::StringMap<Module *>::iterator M = Modules.begin(), 
@@ -1390,14 +1409,9 @@ void ModuleMapParser::parseModuleDecl()
     return;
   }
 
-  // If this is a submodule, use the parent's module map, since we don't want
-  // the private module map file.
-  const FileEntry *ModuleMap = ActiveModule ? ActiveModule->ModuleMap
-                                            : ModuleMapFile;
-
   // Start defining this module.
-  ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, ModuleMap,
-                                        Framework, Explicit).first;
+  ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, Framework,
+                                        Explicit).first;
   ActiveModule->DefinitionLoc = ModuleNameLoc;
   if (Attrs.IsSystem || IsSystem)
     ActiveModule->IsSystem = true;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Aug  8 19:57:23 2014
@@ -2489,15 +2489,18 @@ ASTReader::ReadControlBlock(ModuleFile &
           return Missing;
         }
 
+        HeaderSearch &HS = PP.getHeaderSearchInfo();
         const FileEntry *StoredModMap = FileMgr.getFile(F.ModuleMapPath);
-        if (StoredModMap == nullptr || StoredModMap != M->ModuleMap) {
-          assert(M->ModuleMap && "found module is missing module map file");
+        const FileEntry *ModMap =
+            HS.getModuleMap().getModuleMapFileForUniquing(M);
+        if (StoredModMap == nullptr || StoredModMap != ModMap) {
+          assert(ModMap && "found module is missing module map file");
           assert(M->Name == F.ModuleName && "found module with different name");
           assert(ImportedBy && "top-level import should be verified");
           if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
             Diag(diag::err_imported_module_modmap_changed)
               << F.ModuleName << ImportedBy->FileName
-              << M->ModuleMap->getName() << F.ModuleMapPath;
+              << ModMap->getName() << F.ModuleMapPath;
           return OutOfDate;
         }
       }
@@ -4281,20 +4284,17 @@ ASTReader::ReadSubmoduleBlock(ModuleFile
       bool ConfigMacrosExhaustive = Record[Idx++];
 
       Module *ParentModule = nullptr;
-      const FileEntry *ModuleMap = nullptr;
-      if (Parent) {
+      if (Parent)
         ParentModule = getSubmodule(Parent);
-        ModuleMap = ParentModule->ModuleMap;
-      }
-
-      if (!F.ModuleMapPath.empty())
-        ModuleMap = FileMgr.getFile(F.ModuleMapPath);
 
       // Retrieve this (sub)module from the module map, creating it if
       // necessary.
-      CurrentModule = ModMap.findOrCreateModule(Name, ParentModule, ModuleMap,
-                                                IsFramework, 
+      CurrentModule = ModMap.findOrCreateModule(Name, ParentModule, IsFramework,
                                                 IsExplicit).first;
+
+      // FIXME: set the definition loc for CurrentModule, or call
+      // ModMap.setInferredModuleAllowedBy()
+
       SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS;
       if (GlobalIndex >= SubmodulesLoaded.size() ||
           SubmodulesLoaded[GlobalIndex]) {

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=215268&r1=215267&r2=215268&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Aug  8 19:57:23 2014
@@ -1139,8 +1139,9 @@ void ASTWriter::WriteControlBlock(Prepro
     Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Filename
     unsigned AbbrevCode = Stream.EmitAbbrev(Abbrev);
 
-    assert(WritingModule->ModuleMap && "missing module map");
-    SmallString<128> ModuleMap(WritingModule->ModuleMap->getName());
+    const FileEntry *ModMapFile = PP.getHeaderSearchInfo().getModuleMap().
+        getModuleMapFileForUniquing(WritingModule);
+    SmallString<128> ModuleMap(ModMapFile->getName());
     llvm::sys::fs::make_absolute(ModuleMap);
     RecordData Record;
     Record.push_back(MODULE_MAP_FILE);





More information about the cfe-commits mailing list