PR19215: When writing/reading a PCH that imported a module, store the location

at which that PCH imported each visible submodule of the module. Such locations
are needed when synthesizing macro directives resulting from the import.

llvm-svn: 204417

clang/include/clang/Serialization/ASTReader.h

@@ -767,9 +767,14 @@ private:
   // \brief A list of late parsed template function data.
   SmallVector<uint64_t, 1> LateParsedTemplates;
 
+  struct ImportedSubmodule {
+    serialization::SubmoduleID ID;
+    SourceLocation ImportLoc;
+  };
+
   /// \brief A list of modules that were imported by precompiled headers or
   /// any other non-module AST file.
-  SmallVector<serialization::SubmoduleID, 2> ImportedModules;
+  SmallVector<ImportedSubmodule, 2> ImportedModules;
   //@}
 
   /// \brief The directory that the PCH we are reading is stored in.

clang/lib/Serialization/ASTReader.cpp

@@ -1811,6 +1811,7 @@ void ASTReader::installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,
     // Use the location at which the containing module file was first imported
     // for now.
     ImportLoc = MMI->F->DirectImportLoc;
+    assert(ImportLoc.isValid() && "no import location for a visible macro?");
   }
 
   llvm::SmallVectorImpl<DefMacroDirective*> *Prev =
@@ -3012,9 +3013,11 @@ bool ASTReader::ReadASTBlock(ModuleFile &F) {
         // If we aren't loading a module (which has its own exports), make
         // all of the imported modules visible.
         // FIXME: Deal with macros-only imports.
-        for (unsigned I = 0, N = Record.size(); I != N; ++I) {
-          if (unsigned GlobalID = getGlobalSubmoduleID(F, Record[I]))
-            ImportedModules.push_back(GlobalID);
+        for (unsigned I = 0, N = Record.size(); I != N; /**/) {
+          unsigned GlobalID = getGlobalSubmoduleID(F, Record[I++]);
+          SourceLocation Loc = ReadSourceLocation(F, Record, I);
+          if (GlobalID)
+            ImportedModules.push_back({GlobalID, Loc});
         }
       }
       break;
@@ -3669,12 +3672,14 @@ void ASTReader::InitializeContext() {
     Context.setcudaConfigureCallDecl(
                            cast<FunctionDecl>(GetDecl(CUDASpecialDeclRefs[0])));
   }
-  
+
   // Re-export any modules that were imported by a non-module AST file.
-  for (unsigned I = 0, N = ImportedModules.size(); I != N; ++I) {
-    if (Module *Imported = getSubmodule(ImportedModules[I]))
+  // FIXME: This does not make macro-only imports visible again. It also doesn't
+  // make #includes mapped to module imports visible.
+  for (auto &Import : ImportedModules) {
+    if (Module *Imported = getSubmodule(Import.ID))
       makeModuleVisible(Imported, Module::AllVisible,
-                        /*ImportLoc=*/SourceLocation(),
+                        /*ImportLoc=*/Import.ImportLoc,
                         /*Complain=*/false);
   }
   ImportedModules.clear();

clang/lib/Serialization/ASTWriter.cpp

@@ -4281,20 +4281,33 @@ void ASTWriter::WriteASTCore(Sema &SemaRef,
 
   if (!WritingModule) {
     // Write the submodules that were imported, if any.
-    RecordData ImportedModules;
+    struct ModuleInfo { uint64_t ID; Module *M; };
+    llvm::SmallVector<ModuleInfo, 64> Imports;
     for (const auto *I : Context.local_imports()) {
       assert(SubmoduleIDs.find(I->getImportedModule()) != SubmoduleIDs.end());
-      ImportedModules.push_back(SubmoduleIDs[I->getImportedModule()]);
+      Imports.push_back({SubmoduleIDs[I->getImportedModule()],
+                         I->getImportedModule()});
     }
-    if (!ImportedModules.empty()) {
-      // Sort module IDs.
-      llvm::array_pod_sort(ImportedModules.begin(), ImportedModules.end());
-      
-      // Unique module IDs.
-      ImportedModules.erase(std::unique(ImportedModules.begin(), 
-                                        ImportedModules.end()),
-                            ImportedModules.end());
-      
+
+    if (!Imports.empty()) {
+      auto Cmp = [](const ModuleInfo &A, const ModuleInfo &B) {
+        return A.ID < B.ID;
+      };
+
+      // Sort and deduplicate module IDs.
+      std::sort(Imports.begin(), Imports.end(), Cmp);
+      Imports.erase(std::unique(Imports.begin(), Imports.end(), Cmp),
+                    Imports.end());
+
+      RecordData ImportedModules;
+      for (const auto &Import : Imports) {
+        ImportedModules.push_back(Import.ID);
+        // FIXME: If the module has macros imported then later has declarations
+        // imported, this location won't be the right one as a location for the
+        // declaration imports.
+        AddSourceLocation(Import.M->MacroVisibilityLoc, ImportedModules);
+      }
+
       Stream.EmitRecord(IMPORTED_MODULES, ImportedModules);
     }
   }

clang/test/Modules/Inputs/macro-undef-through-pch/A.h

@@ -0,0 +1,2 @@
+#define AB
+#undef AB

clang/test/Modules/Inputs/macro-undef-through-pch/foo.h

@@ -0,0 +1 @@
+@import A;

clang/test/Modules/Inputs/macro-undef-through-pch/module.map

@@ -0,0 +1,3 @@
+module A {
+  header "A.h"
+}

clang/test/Modules/macro-undef-through-pch.m

@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fmodules-cache-path=%t \
+// RUN:            -I%S/Inputs/macro-undef-through-pch -emit-pch \
+// RUN:            %S/Inputs/macro-undef-through-pch/foo.h -o %t.pch
+// RUN: %clang_cc1 -x objective-c -fmodules -fmodules-cache-path=%t -include-pch %t.pch %s
+
+// PR19215
+#undef AB