[modules] If a module #includes a modular header that #undef's its macro, it

should not export the macro.

... at least, not unless we have local submodule visibility enabled.

llvm-svn: 236369

3 files changed, 6 insertions, 10 deletions

diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index c506aa7f6f0..d4defdd71a7 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -620,8 +620,7 @@ void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc) {
 
   // Determine the set of starting macros for this submodule.
   // FIXME: If we re-enter a submodule, should we restore its MacroDirectives?
-  auto &StartingMacros = (getLangOpts().ModulesLocalVisibility &&
-                          BuildingSubmoduleStack.size() > 1)
+  auto &StartingMacros = getLangOpts().ModulesLocalVisibility
                              ? BuildingSubmoduleStack[0].Macros
                              : Info.Macros;
 
@@ -661,13 +660,13 @@ void Preprocessor::LeaveSubmodule() {
          MD = MD->getPrevious()) {
       assert(MD && "broken macro directive chain");
 
-      // Skip macros defined in other submodules we #included along the way.
+      // Stop on macros defined in other submodules we #included along the way.
       // There's no point doing this if we're tracking local submodule
-      // visibiltiy, since there can be no such directives in our list.
+      // visibility, since there can be no such directives in our list.
       if (!getLangOpts().ModulesLocalVisibility) {
         Module *Mod = getModuleContainingLocation(MD->getLocation());
         if (Mod != Info.M)
-          continue;
+          break;
       }
 
       if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index 2db109565af..1251e982533 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -763,9 +763,6 @@ void Preprocessor::LexAfterModuleImport(Token &Result) {
 }
 
 void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) {
-  if (VisibleModules.isVisible(M))
-    return;
-
   VisibleModules.setVisible(
       M, Loc, [](Module *) {},
       [&](ArrayRef<Module *> Path, Module *Conflict, StringRef Message) {
@@ -779,7 +776,7 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) {
 
   // Add this module to the imports list of the currently-built submodule.
   if (!BuildingSubmoduleStack.empty())
-    BuildingSubmoduleStack.back().M->Imports.push_back(M);
+    BuildingSubmoduleStack.back().M->Imports.insert(M);
 }
 
 bool Preprocessor::FinishLexStringLiteral(Token &Result, std::string &String,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index d67d79b52df..150306176eb 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3442,7 +3442,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName,
 
     case UnresolvedModuleRef::Import:
       if (ResolvedMod)
-        Unresolved.Mod->Imports.push_back(ResolvedMod);
+        Unresolved.Mod->Imports.insert(ResolvedMod);
       continue;
 
     case UnresolvedModuleRef::Export: