[modules] Start moving the module visibility information off the Module itself.

It has no place there; it's not a property of the Module, and it makes
restoring the visibility set when we leave a submodule more difficult.

llvm-svn: 236300

10 files changed, 99 insertions, 71 deletions

diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 5fad1a9b229..97f0a035b0a 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -25,14 +25,14 @@
 using namespace clang;
 
 Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent,
-               bool IsFramework, bool IsExplicit)
+               bool IsFramework, bool IsExplicit, unsigned VisibilityID)
     : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent), Directory(),
-      Umbrella(), ASTFile(nullptr), IsMissingRequirement(false),
-      IsAvailable(true), IsFromModuleFile(false), IsFramework(IsFramework),
-      IsExplicit(IsExplicit), IsSystem(false), IsExternC(false),
-      IsInferred(false), InferSubmodules(false), InferExplicitSubmodules(false),
-      InferExportWildcard(false), ConfigMacrosExhaustive(false),
-      NameVisibility(Hidden) {
+      Umbrella(), ASTFile(nullptr), VisibilityID(VisibilityID),
+      IsMissingRequirement(false), IsAvailable(true), IsFromModuleFile(false),
+      IsFramework(IsFramework), IsExplicit(IsExplicit), IsSystem(false),
+      IsExternC(false), IsInferred(false), InferSubmodules(false),
+      InferExplicitSubmodules(false), InferExportWildcard(false),
+      ConfigMacrosExhaustive(false), NameVisibility(Hidden) {
   if (Parent) {
     if (!Parent->isAvailable())
       IsAvailable = false;
@@ -475,4 +475,47 @@ void Module::dump() const {
   print(llvm::errs());
 }
 
+void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc,
+                                  VisibleCallback Vis, ConflictCallback Cb) {
+  if (isVisible(M))
+    return;
 
+  ++Generation;
+
+  struct Visiting {
+    Module *M;
+    Visiting *ExportedBy;
+  };
+
+  std::function<void(Visiting)> VisitModule = [&](Visiting V) {
+    // Modules that aren't available cannot be made visible.
+    if (!V.M->isAvailable())
+      return;
+
+    // Nothing to do for a module that's already visible.
+    unsigned ID = V.M->getVisibilityID();
+    if (ImportLocs.size() <= ID)
+      ImportLocs.resize(ID + 1);
+    else if (ImportLocs[ID].isValid())
+      return;
+
+    ImportLocs[ID] = Loc;
+    Vis(M);
+
+    // Make any exported modules visible.
+    SmallVector<Module *, 16> Exports;
+    V.M->getExportedModules(Exports);
+    for (Module *E : Exports)
+      VisitModule({E, &V});
+
+    for (auto &C : V.M->Conflicts) {
+      if (isVisible(C.Other)) {
+        llvm::SmallVector<Module*, 8> Path;
+        for (Visiting *I = &V; I; I = I->ExportedBy)
+          Path.push_back(I->M);
+        Cb(Path, C.Other, C.Message);
+      }
+    }
+  };
+  VisitModule({M, nullptr});
+}
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index f6602c8b508..2583ad199de 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1357,7 +1357,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
     if (LastModuleImportResult && ModuleName != getLangOpts().CurrentModule &&
         ModuleName != getLangOpts().ImplementationOfModule)
       ModuleManager->makeModuleVisible(LastModuleImportResult, Visibility,
-                                       ImportLoc, /*Complain=*/false);
+                                       ImportLoc);
     return LastModuleImportResult;
   }
 
@@ -1600,8 +1600,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
       return ModuleLoadResult();
     }
 
-    ModuleManager->makeModuleVisible(Module, Visibility, ImportLoc,
-                                     /*Complain=*/true);
+    ModuleManager->makeModuleVisible(Module, Visibility, ImportLoc);
   }
 
   // Check for any configuration macros that have changed.
@@ -1637,9 +1636,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
 
 void CompilerInstance::makeModuleVisible(Module *Mod,
                                          Module::NameVisibilityKind Visibility,
-                                         SourceLocation ImportLoc,
-                                         bool Complain){
-  ModuleManager->makeModuleVisible(Mod, Visibility, ImportLoc, Complain);
+                                         SourceLocation ImportLoc) {
+  ModuleManager->makeModuleVisible(Mod, Visibility, ImportLoc);
 }
 
 GlobalModuleIndex *CompilerInstance::loadGlobalModuleIndex(
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index a4f1c05dc16..29594994ff6 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -89,7 +89,7 @@ ModuleMap::ModuleMap(SourceManager &SourceMgr, DiagnosticsEngine &Diags,
                      HeaderSearch &HeaderInfo)
     : SourceMgr(SourceMgr), Diags(Diags), LangOpts(LangOpts), Target(Target),
       HeaderInfo(HeaderInfo), BuiltinIncludeDir(nullptr),
-      CompilingModule(nullptr), SourceModule(nullptr) {
+      CompilingModule(nullptr), SourceModule(nullptr), NumCreatedModules(0) {
   MMapLangOpts.LineComment = true;
 }
 
@@ -563,7 +563,7 @@ ModuleMap::findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
   
   // Create a new module with this name.
   Module *Result = new Module(Name, SourceLocation(), Parent,
-                              IsFramework, IsExplicit);
+                              IsFramework, IsExplicit, NumCreatedModules++);
   if (LangOpts.CurrentModule == Name) {
     SourceModule = Result;
     SourceModuleName = Name;
@@ -693,7 +693,8 @@ Module *ModuleMap::inferFrameworkModule(StringRef ModuleName,
     return nullptr;
 
   Module *Result = new Module(ModuleName, SourceLocation(), Parent,
-                              /*IsFramework=*/true, /*IsExplicit=*/false);
+                              /*IsFramework=*/true, /*IsExplicit=*/false,
+                              NumCreatedModules++);
   InferredModuleAllowedBy[Result] = ModuleMapFile;
   Result->IsInferred = true;
   if (LangOpts.CurrentModule == ModuleName) {
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 1c66aac533b..7de6c14ef36 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1686,7 +1686,8 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
     ModuleLoadResult Imported
       = TheModuleLoader.loadModule(IncludeTok.getLocation(), Path, Visibility,
                                    /*IsIncludeDirective=*/true);
-    ++MacroVisibilityGeneration;
+    if (Imported)
+      makeModuleVisible(Imported, IncludeTok.getLocation());
     assert((Imported == nullptr || Imported == SuggestedModule.getModule()) &&
            "the imported module is different than the suggested one");
 
@@ -1766,6 +1767,9 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
   assert(!FID.isInvalid() && "Expected valid file ID");
 
   // Determine if we're switching to building a new submodule, and which one.
+  //
+  // FIXME: If we've already processed this header, just make it visible rather
+  // than entering it again.
   ModuleMap::KnownHeader BuildingModule;
   if (getLangOpts().Modules && !getLangOpts().CurrentModule.empty()) {
     Module *RequestingModule = getModuleForLocation(FilenameLoc);
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index dc1b5bed049..6766fdfcd07 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -671,13 +671,7 @@ void Preprocessor::LeaveSubmodule() {
     Macro.second.setOverriddenMacros(SavedInfo.Overridden);
   }
 
-  if (Info.M->NameVisibility < Module::MacrosVisible) {
-    Info.M->NameVisibility = Module::MacrosVisible;
-    Info.M->MacroVisibilityLoc = Info.ImportLoc;
-    ++MacroVisibilityGeneration;
-    // FIXME: Also mark any exported modules as visible, and check for
-    // conflicts.
-  }
+  makeModuleVisible(Info.M, Info.ImportLoc);
 
   BuildingSubmoduleStack.pop_back();
 }
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index fca3f8b97ce..edeed42c17e 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -124,9 +124,9 @@ ModuleMacro *Preprocessor::getModuleMacro(Module *Mod, IdentifierInfo *II) {
 
 void Preprocessor::updateModuleMacroInfo(const IdentifierInfo *II,
                                          ModuleMacroInfo &Info) {
-  assert(Info.ActiveModuleMacrosGeneration != MacroVisibilityGeneration &&
+  assert(Info.ActiveModuleMacrosGeneration != VisibleModules.getGeneration() &&
          "don't need to update this macro name info");
-  Info.ActiveModuleMacrosGeneration = MacroVisibilityGeneration;
+  Info.ActiveModuleMacrosGeneration = VisibleModules.getGeneration();
 
   auto Leaf = LeafModuleMacros.find(II);
   if (Leaf == LeafModuleMacros.end()) {
@@ -146,7 +146,7 @@ void Preprocessor::updateModuleMacroInfo(const IdentifierInfo *II,
                                                 Leaf->second.end());
   while (!Worklist.empty()) {
     auto *MM = Worklist.pop_back_val();
-    if (MM->getOwningModule()->NameVisibility >= Module::MacrosVisible) {
+    if (VisibleModules.isVisible(MM->getOwningModule())) {
       // We only care about collecting definitions; undefinitions only act
       // to override other definitions.
       if (MM->getMacroInfo())
@@ -236,7 +236,7 @@ void Preprocessor::dumpMacroInfo(const IdentifierInfo *II) {
 
     if (Active.count(MM))
       llvm::errs() << " active";
-    else if (MM->getOwningModule()->NameVisibility < Module::MacrosVisible)
+    else if (!VisibleModules.isVisible(MM->getOwningModule()))
       llvm::errs() << " hidden";
     else
       llvm::errs() << " overridden";
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 231720b8db1..2db109565af 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -107,9 +107,6 @@ Preprocessor::Preprocessor(IntrusiveRefCntPtr<PreprocessorOptions> PPOpts,
 
   // We haven't read anything from the external source.
   ReadMacrosFromExternalSource = false;
-  // We might already have some macros from an imported module (via a PCH or
-  // preamble) if modules is enabled.
-  MacroVisibilityGeneration = LangOpts.Modules ? 1 : 0;
   
   // "Poison" __VA_ARGS__, which can only appear in the expansion of a macro.
   // This gets unpoisoned where it is allowed.
@@ -757,13 +754,34 @@ void Preprocessor::LexAfterModuleImport(Token &Result) {
                                             ModuleImportPath,
                                             Module::MacrosVisible,
                                             /*IsIncludeDirective=*/false);
-      ++MacroVisibilityGeneration;
+      if (Imported)
+        makeModuleVisible(Imported, ModuleImportLoc);
     }
     if (Callbacks && (getLangOpts().Modules || getLangOpts().DebuggerSupport))
       Callbacks->moduleImport(ModuleImportLoc, ModuleImportPath, Imported);
   }
 }
 
+void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) {
+  if (VisibleModules.isVisible(M))
+    return;
+
+  VisibleModules.setVisible(
+      M, Loc, [](Module *) {},
+      [&](ArrayRef<Module *> Path, Module *Conflict, StringRef Message) {
+        // FIXME: Include the path in the diagnostic.
+        // FIXME: Include the import location for the conflicting module.
+        Diag(ModuleImportLoc, diag::warn_module_conflict)
+            << Path[0]->getFullModuleName()
+            << Conflict->getFullModuleName()
+            << Message;
+      });
+
+  // Add this module to the imports list of the currently-built submodule.
+  if (!BuildingSubmoduleStack.empty())
+    BuildingSubmoduleStack.back().M->Imports.push_back(M);
+}
+
 bool Preprocessor::FinishLexStringLiteral(Token &Result, std::string &String,
                                           const char *DiagnosticTag,
                                           bool AllowMacroExpansion) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 34e5784e862..00922f73a8e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14088,8 +14088,7 @@ void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
   checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext);
 
   // FIXME: Should we synthesize an ImportDecl here?
-  getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, DirectiveLoc,
-                                      /*Complain=*/true);
+  getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, DirectiveLoc);
 }
 
 void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc,
@@ -14106,8 +14105,7 @@ void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc,
   Consumer.HandleImplicitImportDecl(ImportD);
 
   // Make the module visible.
-  getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, Loc,
-                                      /*Complain=*/false);
+  getModuleLoader().makeModuleVisible(Mod, Module::AllVisible, Loc);
 }
 
 void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 7225d2f610a..d67d79b52df 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3240,8 +3240,7 @@ void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner) {
 
 void ASTReader::makeModuleVisible(Module *Mod,
                                   Module::NameVisibilityKind NameVisibility,
-                                  SourceLocation ImportLoc,
-                                  bool Complain) {
+                                  SourceLocation ImportLoc) {
   llvm::SmallPtrSet<Module *, 4> Visited;
   SmallVector<Module *, 4> Stack;
   Stack.push_back(Mod);
@@ -3285,20 +3284,6 @@ void ASTReader::makeModuleVisible(Module *Mod,
       if (Visited.insert(Exported).second)
         Stack.push_back(Exported);
     }
-
-    // Detect any conflicts.
-    if (Complain) {
-      assert(ImportLoc.isValid() && "Missing import location");
-      for (unsigned I = 0, N = Mod->Conflicts.size(); I != N; ++I) {
-        if (Mod->Conflicts[I].Other->NameVisibility >= NameVisibility) {
-          Diag(ImportLoc, diag::warn_module_conflict)
-            << Mod->getFullModuleName()
-            << Mod->Conflicts[I].Other->getFullModuleName()
-            << Mod->Conflicts[I].Message;
-          // FIXME: Need note where the other module was imported.
-        }
-      }
-    }
   }
 }
 
@@ -3669,7 +3654,7 @@ ASTReader::ReadASTCore(StringRef FileName,
   return Success;
 }
 
-void ASTReader::InitializeContext() {  
+void ASTReader::InitializeContext() {
   // If there's a listener, notify them that we "read" the translation unit.
   if (DeserializationListener)
     DeserializationListener->DeclRead(PREDEF_DECL_TRANSLATION_UNIT_ID, 
@@ -3792,13 +3777,13 @@ void ASTReader::InitializeContext() {
   }
 
   // Re-export any modules that were imported by a non-module AST file.
-  // FIXME: This does not make macro-only imports visible again. It also doesn't
-  // make #includes mapped to module imports visible.
+  // FIXME: This does not make macro-only imports visible again.
   for (auto &Import : ImportedModules) {
-    if (Module *Imported = getSubmodule(Import.ID))
+    if (Module *Imported = getSubmodule(Import.ID)) {
       makeModuleVisible(Imported, Module::AllVisible,
-                        /*ImportLoc=*/Import.ImportLoc,
-                        /*Complain=*/false);
+                        /*ImportLoc=*/Import.ImportLoc);
+      PP.makeModuleVisible(Imported, Import.ImportLoc);
+    }
   }
   ImportedModules.clear();
 }
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 38436977341..8a3b9d5163f 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2323,19 +2323,6 @@ static unsigned getNumberOfModules(Module *Mod) {
 }
 
 void ASTWriter::WriteSubmodules(Module *WritingModule) {
-  // Determine the dependencies of our module and each of it's submodules.
-  // FIXME: This feels like it belongs somewhere else, but there are no
-  // other consumers of this information.
-  SourceManager &SrcMgr = PP->getSourceManager();
-  ModuleMap &ModMap = PP->getHeaderSearchInfo().getModuleMap();
-  for (const auto *I : Context->local_imports()) {
-    if (Module *ImportedFrom
-          = ModMap.inferModuleFromLocation(FullSourceLoc(I->getLocation(), 
-                                                         SrcMgr))) {
-      ImportedFrom->Imports.push_back(I->getImportedModule());
-    }
-  }
-  
   // Enter the submodule description block.
   Stream.EnterSubblock(SUBMODULE_BLOCK_ID, /*bits for abbreviations*/5);