2 files changed, 25 insertions, 26 deletions

diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 419f6b32008..e3a16c3c5f2 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -3323,8 +3323,7 @@ static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) {
 void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner,
                                  bool FromFinalization) {
   // FIXME: Only do this if Owner->NameVisibility == AllVisible.
-  for (unsigned I = 0, N = Names.HiddenDecls.size(); I != N; ++I) {
-    Decl *D = Names.HiddenDecls[I];
+  for (Decl *D : Names.HiddenDecls) {
     bool wasHidden = D->Hidden;
     D->Hidden = false;
 
@@ -3337,10 +3336,8 @@ void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner,
 
   assert((FromFinalization || Owner->NameVisibility >= Module::MacrosVisible) &&
          "nothing to make visible?");
-  for (HiddenMacrosMap::const_iterator I = Names.HiddenMacros.begin(),
-                                       E = Names.HiddenMacros.end();
-       I != E; ++I)
-    installImportedMacro(I->first, I->second, Owner, FromFinalization);
+  for (const auto &Macro : Names.HiddenMacros)
+    installImportedMacro(Macro.first, Macro.second, Owner, FromFinalization);
 }
 
 void ASTReader::makeModuleVisible(Module *Mod,
@@ -3374,9 +3371,12 @@ void ASTReader::makeModuleVisible(Module *Mod,
     // mark them as visible.
     HiddenNamesMapType::iterator Hidden = HiddenNamesMap.find(Mod);
     if (Hidden != HiddenNamesMap.end()) {
-      makeNamesVisible(Hidden->second, Hidden->first,
-                       /*FromFinalization*/false);
+      auto HiddenNames = std::move(*Hidden);
       HiddenNamesMap.erase(Hidden);
+      makeNamesVisible(HiddenNames.second, HiddenNames.first,
+                       /*FromFinalization*/false);
+      assert(HiddenNamesMap.find(Mod) == HiddenNamesMap.end() &&
+             "making names visible added hidden names");
     }
 
     // Push any exported modules onto the stack to be marked as visible.
@@ -3899,12 +3899,12 @@ void ASTReader::InitializeContext() {
 }
 
 void ASTReader::finalizeForWriting() {
-  for (HiddenNamesMapType::iterator Hidden = HiddenNamesMap.begin(),
-                                 HiddenEnd = HiddenNamesMap.end();
-       Hidden != HiddenEnd; ++Hidden) {
-    makeNamesVisible(Hidden->second, Hidden->first, /*FromFinalization*/true);
+  while (!HiddenNamesMap.empty()) {
+    auto HiddenNames = std::move(*HiddenNamesMap.begin());
+    HiddenNamesMap.erase(HiddenNamesMap.begin());
+    makeNamesVisible(HiddenNames.second, HiddenNames.first,
+                     /*FromFinalization*/true);
   }
-  HiddenNamesMap.clear();
 }
 
 /// \brief Given a cursor at the start of an AST file, scan ahead and drop the
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 9b44c54eb35..82932426032 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3034,16 +3034,25 @@ class ASTIdentifierTableTrait {
   MacroDirective *getPublicSubmoduleMacro(MacroDirective *MD,
                                           SubmoduleID &ModID,
                                           OverriddenList &Overridden) {
+    Overridden.clear();
     if (!MD)
       return nullptr;
 
-    Overridden.clear();
     SubmoduleID OrigModID = ModID;
     Optional<bool> IsPublic;
     for (; MD; MD = MD->getPrevious()) {
       SubmoduleID ThisModID = getSubmoduleID(MD);
       if (ThisModID == 0) {
         IsPublic = Optional<bool>();
+
+        // If we have no directive location, this macro was installed when
+        // finalizing the ASTReader.
+        if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))
+          if (DefMD->getInfo()->getOwningModuleID())
+            return MD;
+        // Skip imports that only produce #undefs for now.
+        // FIXME: We should still re-export them!
+
         continue;
       }
       if (ThisModID != ModID) {
@@ -3068,7 +3077,7 @@ class ASTIdentifierTableTrait {
           else
             SourceID = Writer.inferSubmoduleIDFromLocation(DefLoc);
         }
-        if (SourceID != OrigModID)
+        if (OrigModID && SourceID != OrigModID)
           Overridden.push_back(SourceID);
       }
 
@@ -3095,17 +3104,7 @@ class ASTIdentifierTableTrait {
   }
 
   SubmoduleID getSubmoduleID(MacroDirective *MD) {
-    if (MD->getLocation().isValid())
-      return Writer.inferSubmoduleIDFromLocation(MD->getLocation());
-
-    // If we have no directive location, this macro was installed when
-    // finalizing the ASTReader.
-    if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))
-      return DefMD->getInfo()->getOwningModuleID();
-
-    // Skip imports that only produce #undefs for now.
-    // FIXME: We should still re-export them!
-    return 0;
+    return Writer.inferSubmoduleIDFromLocation(MD->getLocation());
   }
 
 public: