5 files changed, 38 insertions, 3 deletions
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index f02364a385e..bba0c38cec7 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -394,7 +394,9 @@ class Preprocessor : public RefCountedBase<Preprocessor> {
                                    const IdentifierInfo *II) const {
       // FIXME: Find a spare bit on IdentifierInfo and store a
       //        HasModuleMacros flag.
-      if (!II->hasMacroDefinition() || !PP.getLangOpts().Modules ||
+      if (!II->hasMacroDefinition() ||
+          (!PP.getLangOpts().Modules &&
+           !PP.getLangOpts().ModulesLocalVisibility) ||
           !PP.CurSubmoduleState->VisibleModules.getGeneration())
         return nullptr;
 
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index 1a35d32dedb..c231e18eecc 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -644,11 +644,20 @@ void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc) {
   if (FirstTime) {
     // Determine the set of starting macros for this submodule; take these
     // from the "null" module (the predefines buffer).
+    //
+    // FIXME: If we have local visibility but not modules enabled, the
+    // NullSubmoduleState is polluted by #defines in the top-level source
+    // file.
     auto &StartingMacros = NullSubmoduleState.Macros;
 
     // Restore to the starting state.
     // FIXME: Do this lazily, when each macro name is first referenced.
     for (auto &Macro : StartingMacros) {
+      // Skip uninteresting macros.
+      if (!Macro.second.getLatest() &&
+          Macro.second.getOverriddenMacros().empty())
+        continue;
+
       MacroState MS(Macro.second.getLatest());
       MS.setOverriddenMacros(*this, Macro.second.getOverriddenMacros());
       State.Macros.insert(std::make_pair(Macro.first, std::move(MS)));
@@ -732,6 +741,11 @@ void Preprocessor::LeaveSubmodule() {
     }
   }
 
+  // FIXME: Before we leave this submodule, we should parse all the other
+  // headers within it. Otherwise, we're left with an inconsistent state
+  // where we've made the module visible but don't yet have its complete
+  // contents.
+
   // Put back the outer module's state, if we're tracking it.
   if (getLangOpts().ModulesLocalVisibility)
     CurSubmoduleState = Info.OuterSubmoduleState;
@@ -739,6 +753,5 @@ void Preprocessor::LeaveSubmodule() {
   BuildingSubmoduleStack.pop_back();
 
   // A nested #include makes the included submodule visible.
-  if (!BuildingSubmoduleStack.empty() || !getLangOpts().ModulesLocalVisibility)
-    makeModuleVisible(LeavingMod, ImportLoc);
+  makeModuleVisible(LeavingMod, ImportLoc);
 }
diff --git a/clang/test/Modules/Inputs/submodule-visibility/a.h b/clang/test/Modules/Inputs/submodule-visibility/a.h
index d8805c92f24..34ac6b2b2ea 100644
--- a/clang/test/Modules/Inputs/submodule-visibility/a.h
+++ b/clang/test/Modules/Inputs/submodule-visibility/a.h
@@ -1 +1,7 @@
 int n;
+
+#ifdef B
+#error B is defined
+#endif
+
+#define A
diff --git a/clang/test/Modules/Inputs/submodule-visibility/b.h b/clang/test/Modules/Inputs/submodule-visibility/b.h
index fa419c0c5c4..dae75bf3315 100644
--- a/clang/test/Modules/Inputs/submodule-visibility/b.h
+++ b/clang/test/Modules/Inputs/submodule-visibility/b.h
@@ -1 +1,7 @@
 int m = n;
+
+#if defined(A) && !defined(ALLOW_NAME_LEAKAGE)
+#error A is defined
+#endif
+
+#define B
diff --git a/clang/test/Modules/submodule-visibility.cpp b/clang/test/Modules/submodule-visibility.cpp
index 084f811f231..b2c5fc7ba19 100644
--- a/clang/test/Modules/submodule-visibility.cpp
+++ b/clang/test/Modules/submodule-visibility.cpp
@@ -20,3 +20,11 @@
 #endif
 
 int k = n + m; // OK, a and b are visible here.
+
+#ifndef A
+#error A is not defined
+#endif
+
+#ifndef B
+#error B is not defined
+#endif