Track shadow modules with a generation counter.

This is a follow up to r321855, closing the gap between our internal shadow
modules implementation and upstream. It has been tested for longer and
provides a better approach for tracking shadow modules. Mostly NFCI.

rdar://problem/23612102

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

diff --git a/include/clang/Lex/HeaderSearch.h b/include/clang/Lex/HeaderSearch.h
index 73a5af1913ff6a7a66330738204cab7178e729e5..6b9dbfcd1e934f3cc15431c217e56116ce8c8aa6 100644 (file)
--- a/include/clang/Lex/HeaderSearch.h
+++ b/include/clang/Lex/HeaderSearch.h
@@ -726,7 +726,6 @@ private:
   LoadModuleMapResult loadModuleMapFileImpl(const FileEntry *File,
                                             bool IsSystem,
                                             const DirectoryEntry *Dir,
-                                            bool IsExplicitlyProvided,
                                             FileID ID = FileID(),
                                             unsigned *Offset = nullptr);
 

diff --git a/include/clang/Lex/ModuleMap.h b/include/clang/Lex/ModuleMap.h
index 9632ca6ce220e6cb99d7b20813e8a29e3504e147..045eab11208e86dda5d1d4a1f1a17a267c2744e3 100644 (file)
--- a/include/clang/Lex/ModuleMap.h
+++ b/include/clang/Lex/ModuleMap.h
@@ -198,16 +198,14 @@ private:
   /// header.
   llvm::DenseMap<const DirectoryEntry *, Module *> UmbrellaDirs;
 
-  /// \brief The set of modules provided explicitly (e.g. by -fmodule-map-file),
-  /// which are allowed to shadow other implicitly discovered modules.
-  llvm::DenseSet<const Module *> ExplicitlyProvidedModules;
+  /// \brief A generation counter that is used to test whether modules of the
+  /// same name may shadow or are illegal redefintions.
+  ///
+  /// Modules from earlier scopes may shadow modules from later ones.
+  /// Modules from the same scope may not have the same name.
+  unsigned CurrentModuleScopeID = 0;
 
-  bool mayShadowModuleBeingParsed(Module *ExistingModule,
-                                  bool IsExplicitlyProvided) {
-    assert(!ExistingModule->Parent && "expected top-level module");
-    return !IsExplicitlyProvided &&
-           ExplicitlyProvidedModules.count(ExistingModule);
-  }
+  llvm::DenseMap<Module *, unsigned> ModuleScopeIDs;
 
   /// \brief The set of attributes that can be attached to a module.
   struct Attributes {
@@ -489,9 +487,9 @@ 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, bool IsFramework,
-                     bool IsExplicit, bool UsesExplicitModuleMapFile = false);
+  std::pair<Module *, bool> findOrCreateModule(StringRef Name, Module *Parent,
+                                               bool IsFramework,
+                                               bool IsExplicit);
 
   /// \brief Create a 'global module' for a C++ Modules TS module interface
   /// unit.
@@ -521,6 +519,19 @@ public:
   Module *createShadowedModule(StringRef Name, bool IsFramework,
                                Module *ShadowingModule);
 
+  /// \brief Creates a new declaration scope for module names, allowing
+  /// previously defined modules to shadow definitions from the new scope.
+  ///
+  /// \note Module names from earlier scopes will shadow names from the new
+  /// scope, which is the opposite of how shadowing works for variables.
+  void finishModuleDeclarationScope() { CurrentModuleScopeID += 1; }
+
+  bool mayShadowNewModule(Module *ExistingModule) {
+    assert(!ExistingModule->Parent && "expected top-level module");
+    assert(ModuleScopeIDs.count(ExistingModule) && "unknown module");
+    return ModuleScopeIDs[ExistingModule] < CurrentModuleScopeID;
+  }
+
   /// \brief Retrieve the module map file containing the definition of the given
   /// module.
   ///
@@ -606,8 +617,6 @@ public:
   /// \brief Marks this header as being excluded from the given module.
   void excludeHeader(Module *Mod, Module::Header Header);
 
-  void setExplicitlyProvided(Module *Mod);
-
   /// \brief Parse the given module map file, and record any modules we 
   /// encounter.
   ///
@@ -634,7 +643,6 @@ public:
   /// \returns true if an error occurred, false otherwise.
   bool parseModuleMapFile(const FileEntry *File, bool IsSystem,
                           const DirectoryEntry *HomeDir,
-                          bool IsExplicitlyProvided = false,
                           FileID ID = FileID(), unsigned *Offset = nullptr,
                           SourceLocation ExternModuleLoc = SourceLocation());
 

diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp
index 12226b231417521f70c87e454d9ffa06cafd75bf..1b4d789b4d978eed01845067d8f01e83d627d718 100644 (file)
--- a/lib/Frontend/FrontendAction.cpp
+++ b/lib/Frontend/FrontendAction.cpp
@@ -858,6 +858,13 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
       CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
   }
 
+  // Add a module declaration scope so that modules from -fmodule-map-file
+  // arguments may shadow modules found implicitly in search paths.
+  CI.getPreprocessor()
+      .getHeaderSearchInfo()
+      .getModuleMap()
+      .finishModuleDeclarationScope();
+
   // If we were asked to load any module files, do so now.
   for (const auto &ModuleFile : CI.getFrontendOpts().ModuleFiles)
     if (!CI.loadModuleFile(ModuleFile))

diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp
index ec8175470632a2b46c25182877bcef513d3b8c93..6976294a2eaf3e7c89165e173d5833aa4527b124 100644 (file)
--- a/lib/Lex/HeaderSearch.cpp
+++ b/lib/Lex/HeaderSearch.cpp
@@ -1367,8 +1367,7 @@ bool HeaderSearch::loadModuleMapFile(const FileEntry *File, bool IsSystem,
     }
   }
 
-  switch (loadModuleMapFileImpl(File, IsSystem, Dir,
-                                /*IsExplictlyProvided=*/true, ID, Offset)) {
+  switch (loadModuleMapFileImpl(File, IsSystem, Dir, ID, Offset)) {
   case LMM_AlreadyLoaded:
   case LMM_NewlyLoaded:
     return false;
@@ -1379,9 +1378,10 @@ bool HeaderSearch::loadModuleMapFile(const FileEntry *File, bool IsSystem,
   llvm_unreachable("Unknown load module map result");
 }
 
-HeaderSearch::LoadModuleMapResult HeaderSearch::loadModuleMapFileImpl(
-    const FileEntry *File, bool IsSystem, const DirectoryEntry *Dir,
-    bool IsExplicitlyProvided, FileID ID, unsigned *Offset) {
+HeaderSearch::LoadModuleMapResult
+HeaderSearch::loadModuleMapFileImpl(const FileEntry *File, bool IsSystem,
+                                    const DirectoryEntry *Dir, FileID ID,
+                                    unsigned *Offset) {
   assert(File && "expected FileEntry");
 
   // Check whether we've already loaded this module map, and mark it as being
@@ -1390,16 +1390,14 @@ HeaderSearch::LoadModuleMapResult HeaderSearch::loadModuleMapFileImpl(
   if (!AddResult.second)
     return AddResult.first->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap;
 
-  if (ModMap.parseModuleMapFile(File, IsSystem, Dir, IsExplicitlyProvided, ID,
-                                Offset)) {
+  if (ModMap.parseModuleMapFile(File, IsSystem, Dir, ID, Offset)) {
     LoadedModuleMaps[File] = false;
     return LMM_InvalidModuleMap;
   }
 
   // Try to load a corresponding private module map.
   if (const FileEntry *PMMFile = getPrivateModuleMap(File, FileMgr)) {
-    if (ModMap.parseModuleMapFile(PMMFile, IsSystem, Dir,
-                                  IsExplicitlyProvided)) {
+    if (ModMap.parseModuleMapFile(PMMFile, IsSystem, Dir)) {
       LoadedModuleMaps[File] = false;
       return LMM_InvalidModuleMap;
     }
@@ -1470,8 +1468,8 @@ HeaderSearch::loadModuleMapFile(const DirectoryEntry *Dir, bool IsSystem,
     return KnownDir->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap;
 
   if (const FileEntry *ModuleMapFile = lookupModuleMapFile(Dir, IsFramework)) {
-    LoadModuleMapResult Result = loadModuleMapFileImpl(
-        ModuleMapFile, IsSystem, Dir, /*IsExplicitlyProvided=*/false);
+    LoadModuleMapResult Result =
+        loadModuleMapFileImpl(ModuleMapFile, IsSystem, Dir);
     // Add Dir explicitly in case ModuleMapFile is in a subdirectory.
     // E.g. Foo.framework/Modules/module.modulemap
     //      ^Dir                  ^ModuleMapFile

diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp
index 19050716e76a0cf94cfe08499274208066189ede..657eef167a69a1866c163c741d8af4251c7f347b 100644 (file)
--- a/lib/Lex/ModuleMap.cpp
+++ b/lib/Lex/ModuleMap.cpp
@@ -746,9 +746,10 @@ Module *ModuleMap::lookupModuleQualified(StringRef Name, Module *Context) const{
   return Context->findSubmodule(Name);
 }
 
-std::pair<Module *, bool>
-ModuleMap::findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
-                              bool IsExplicit, bool UsesExplicitModuleMapFile) {
+std::pair<Module *, bool> 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);
@@ -760,8 +761,7 @@ ModuleMap::findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
     if (LangOpts.CurrentModule == Name)
       SourceModule = Result;
     Modules[Name] = Result;
-    if (UsesExplicitModuleMapFile)
-      ExplicitlyProvidedModules.insert(Result);
+    ModuleScopeIDs[Result] = CurrentModuleScopeID;
   }
   return std::make_pair(Result, true);
 }
@@ -930,6 +930,7 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
     if (LangOpts.CurrentModule == ModuleName)
       SourceModule = Result;
     Modules[ModuleName] = Result;
+    ModuleScopeIDs[Result] = CurrentModuleScopeID;
   }
 
   Result->IsSystem |= Attrs.IsSystem;
@@ -1011,6 +1012,7 @@ Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
                  /*IsExplicit=*/false, NumCreatedModules++);
   Result->ShadowingModule = ShadowingModule;
   Result->IsAvailable = false;
+  ModuleScopeIDs[Result] = CurrentModuleScopeID;
   ShadowModules.push_back(Result);
 
   return Result;
@@ -1133,11 +1135,6 @@ void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) {
   Mod->Headers[Module::HK_Excluded].push_back(std::move(Header));
 }
 
-void ModuleMap::setExplicitlyProvided(Module *Mod) {
-  assert(Modules[Mod->Name] == Mod && "explicitly provided module is shadowed");
-  ExplicitlyProvidedModules.insert(Mod);
-}
-
 const FileEntry *
 ModuleMap::getContainingModuleMapFile(const Module *Module) const {
   if (Module->DefinitionLoc.isInvalid())
@@ -1342,8 +1339,6 @@ namespace clang {
     /// \brief Consume the current token and return its location.
     SourceLocation consumeToken();
 
-    bool UsesExplicitModuleMapFile = false;
-
     /// \brief Skip tokens until we reach the a token with the given kind
     /// (or the end of the file).
     void skipUntil(MMToken::TokenKind K);
@@ -1372,12 +1367,10 @@ namespace clang {
     explicit ModuleMapParser(Lexer &L, SourceManager &SourceMgr,
                              const TargetInfo *Target, DiagnosticsEngine &Diags,
                              ModuleMap &Map, const FileEntry *ModuleMapFile,
-                             const DirectoryEntry *Directory, bool IsSystem,
-                             bool UsesExplicitModuleMapFile)
+                             const DirectoryEntry *Directory, bool IsSystem)
         : L(L), SourceMgr(SourceMgr), Target(Target), Diags(Diags), Map(Map),
           ModuleMapFile(ModuleMapFile), Directory(Directory),
-          IsSystem(IsSystem),
-          UsesExplicitModuleMapFile(UsesExplicitModuleMapFile) {
+          IsSystem(IsSystem) {
       Tok.clear();
       consumeToken();
     }
@@ -1837,8 +1830,7 @@ void ModuleMapParser::parseModuleDecl() {
       return;
     }
 
-    if (!Existing->Parent &&
-        Map.mayShadowModuleBeingParsed(Existing, UsesExplicitModuleMapFile)) {
+    if (!Existing->Parent && Map.mayShadowNewModule(Existing)) {
       ShadowingModule = Existing;
     } else {
       // This is not a shawdowed module decl, it is an illegal redefinition.
@@ -1861,9 +1853,9 @@ void ModuleMapParser::parseModuleDecl() {
     ActiveModule =
         Map.createShadowedModule(ModuleName, Framework, ShadowingModule);
   } else {
-    ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, Framework,
-                                          Explicit, UsesExplicitModuleMapFile)
-                       .first;
+    ActiveModule =
+        Map.findOrCreateModule(ModuleName, ActiveModule, Framework, Explicit)
+            .first;
   }
 
   ActiveModule->DefinitionLoc = ModuleNameLoc;
@@ -2041,7 +2033,7 @@ void ModuleMapParser::parseExternModuleDecl() {
         Map.HeaderInfo.getHeaderSearchOpts().ModuleMapFileHomeIsCwd
             ? Directory
             : File->getDir(),
-        false /*IsExplicitlyProvided*/, FileID(), nullptr, ExternLoc);
+        FileID(), nullptr, ExternLoc);
 }
 
 /// Whether to add the requirement \p Feature to the module \p M.
@@ -2848,8 +2840,7 @@ bool ModuleMapParser::parseModuleMapFile() {
 }
 
 bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem,
-                                   const DirectoryEntry *Dir,
-                                   bool IsExplicitlyProvided, FileID ID,
+                                   const DirectoryEntry *Dir, FileID ID,
                                    unsigned *Offset,
                                    SourceLocation ExternModuleLoc) {
   assert(Target && "Missing target information");
@@ -2879,7 +2870,7 @@ bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem,
           Buffer->getBufferEnd());
   SourceLocation Start = L.getSourceLocation();
   ModuleMapParser Parser(L, SourceMgr, Target, Diags, *this, File, Dir,
-                         IsSystem, IsExplicitlyProvided);
+                         IsSystem);
   bool Result = Parser.parseModuleMapFile();
   ParsedModuleMap[File] = Result;