[Index] Add index support for cross import overlays.

Nathan Hawes · Nathan Hawes · commit 8b03b05f1d4b · 2020-03-25T10:04:41.000-07:00
Resolves rdar://problem/59445445
diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp
@@ -2105,10 +2105,15 @@ SourceFile::getModuleShadowedBySeparatelyImportedOverlay(const ModuleDecl *overl
       }
     }
   }
-  auto i = separatelyImportedOverlaysReversed.find(overlay);
-  return i != separatelyImportedOverlaysReversed.end()
-    ? std::get<1>(*i)
-    : nullptr;
+
+  ModuleDecl *underlying = const_cast<ModuleDecl *>(overlay);
+  while (underlying->getNameStr().startswith("_")) {
+    auto next = separatelyImportedOverlaysReversed.find(underlying);
+    if (next == separatelyImportedOverlaysReversed.end())
+      return nullptr;
+    underlying = std::get<1>(*next);
+  }
+  return underlying;
 };
 
 void ModuleDecl::clearLookupCache() {
diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp
@@ -781,12 +781,9 @@ void CodeCompletionResultBuilder::setAssociatedDecl(const Decl *D,
     // If this is an underscored cross-import overlay, map it to its underlying
     // module instead.
     if (SF) {
-      while (MD->getNameStr().startswith("_")) {
-        auto *Underlying = SF->getModuleShadowedBySeparatelyImportedOverlay(MD);
-        if (!Underlying)
-          break;
+      auto *Underlying = SF->getModuleShadowedBySeparatelyImportedOverlay(MD);
+      if (Underlying)
         MD = Underlying;
-      }
     }
     CurrentModule = MD;
   }
diff --git a/lib/Index/Index.cpp b/lib/Index/Index.cpp
@@ -129,7 +129,6 @@ class SourceFileOrModule {
     ImportFilter |= ModuleDecl::ImportFilterKind::Public;
     ImportFilter |= ModuleDecl::ImportFilterKind::Private;
     ImportFilter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
-    // FIXME: ImportFilterKind::ShadowedBySeparateOverlay?
 
     if (auto *SF = SFOrMod.dyn_cast<SourceFile *>()) {
       SF->getImportedModules(Modules, ImportFilter);
@@ -147,6 +146,7 @@ struct IndexedWitness {
 class IndexSwiftASTWalker : public SourceEntityWalker {
   IndexDataConsumer &IdxConsumer;
   SourceManager &SrcMgr;
+  SourceFile *InitialFile; ///< The SoureFile we started walking from, if any.
   unsigned BufferID;
   bool enableWarnings;
 
@@ -282,8 +282,9 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
 
 public:
   IndexSwiftASTWalker(IndexDataConsumer &IdxConsumer, ASTContext &Ctx,
-                      unsigned BufferID = -1)
-      : IdxConsumer(IdxConsumer), SrcMgr(Ctx.SourceMgr), BufferID(BufferID),
+                      SourceFile *SF = nullptr)
+      : IdxConsumer(IdxConsumer), SrcMgr(Ctx.SourceMgr), InitialFile(SF),
+        BufferID(SF ? SF->getBufferID().getValueOr(-1) : -1),
         enableWarnings(IdxConsumer.enableWarnings()) {}
 
   ~IndexSwiftASTWalker() override {
@@ -755,7 +756,18 @@ bool IndexSwiftASTWalker::visitImports(
       continue;
     bool IsClangModule = *IsClangModuleOpt;
 
-    if (!IdxConsumer.startDependency(Mod->getName().str(), Path, IsClangModule,
+    StringRef ModuleName = Mod->getNameStr();
+
+    // If this module is an underscored cross-import overlay, use the name
+    // of the underlying module instead.
+    if (InitialFile) {
+      ModuleDecl *Underlying =
+        InitialFile->getModuleShadowedBySeparatelyImportedOverlay(Mod);
+      if (Underlying)
+        ModuleName = Underlying->getNameStr();
+    }
+
+    if (!IdxConsumer.startDependency(ModuleName, Path, IsClangModule,
                                      Mod->isSystemModule()))
       return false;
     if (!IsClangModule)
@@ -1584,16 +1596,15 @@ void IndexSwiftASTWalker::collectRecursiveModuleImports(
 
 void index::indexDeclContext(DeclContext *DC, IndexDataConsumer &consumer) {
   assert(DC);
-  unsigned bufferId = DC->getParentSourceFile()->getBufferID().getValue();
-  IndexSwiftASTWalker walker(consumer, DC->getASTContext(), bufferId);
+  SourceFile *SF = DC->getParentSourceFile();
+  IndexSwiftASTWalker walker(consumer, DC->getASTContext(), SF);
   walker.visitDeclContext(DC);
   consumer.finish();
 }
 
 void index::indexSourceFile(SourceFile *SF, IndexDataConsumer &consumer) {
   assert(SF);
-  unsigned bufferID = SF->getBufferID().getValue();
-  IndexSwiftASTWalker walker(consumer, SF->getASTContext(), bufferID);
+  IndexSwiftASTWalker walker(consumer, SF->getASTContext(), SF);
   walker.visitModule(*SF->getParentModule());
   consumer.finish();
 }
diff --git a/lib/Index/IndexRecord.cpp b/lib/Index/IndexRecord.cpp
@@ -372,14 +372,28 @@ getModuleInfoFromOpaqueModule(clang::index::writer::OpaqueModule mod,
   return info;
 }
 
+/// Gets the module name of the given module, or its underlying module if it's a
+/// cross import overlay implicitly imported in \p initialFile.
+static StringRef getUnderlyingModuleName(ModuleDecl *module,
+                                         SourceFile *initialFile) {
+  if (initialFile) {
+    ModuleDecl *underlying =
+      initialFile->getModuleShadowedBySeparatelyImportedOverlay(module);
+    if (underlying)
+      module = underlying;
+  }
+  return module->getNameStr();
+}
+
 static bool
 emitDataForSwiftSerializedModule(ModuleDecl *module,
                                  StringRef indexStorePath,
                                  bool indexSystemModules,
                                  StringRef targetTriple,
                                  const clang::CompilerInstance &clangCI,
                                  DiagnosticEngine &diags,
-                                 IndexUnitWriter &parentUnitWriter);
+                                 IndexUnitWriter &parentUnitWriter,
+                                 SourceFile *initialFile);
 
 static void addModuleDependencies(ArrayRef<ModuleDecl::ImportedModule> imports,
                                   StringRef indexStorePath,
@@ -388,7 +402,8 @@ static void addModuleDependencies(ArrayRef<ModuleDecl::ImportedModule> imports,
                                   const clang::CompilerInstance &clangCI,
                                   DiagnosticEngine &diags,
                                   IndexUnitWriter &unitWriter,
-                                  StringScratchSpace &moduleNameScratch) {
+                                  StringScratchSpace &moduleNameScratch,
+                                  SourceFile *initialFile = nullptr) {
   auto &fileMgr = clangCI.getFileManager();
 
   for (auto &import : imports) {
@@ -430,9 +445,13 @@ static void addModuleDependencies(ArrayRef<ModuleDecl::ImportedModule> imports,
               emitDataForSwiftSerializedModule(mod, indexStorePath,
                                                indexSystemModules,
                                                targetTriple, clangCI, diags,
-                                               unitWriter);
+                                               unitWriter, initialFile);
               withoutUnitName = false;
             }
+
+            // If this is a cross-import overlay, make sure we use the name of
+            // the underlying module instead.
+            moduleName = getUnderlyingModuleName(mod, initialFile);
           }
           clang::index::writer::OpaqueModule opaqMod =
               moduleNameScratch.createString(moduleName);
@@ -454,9 +473,12 @@ emitDataForSwiftSerializedModule(ModuleDecl *module,
                                  StringRef targetTriple,
                                  const clang::CompilerInstance &clangCI,
                                  DiagnosticEngine &diags,
-                                 IndexUnitWriter &parentUnitWriter) {
+                                 IndexUnitWriter &parentUnitWriter,
+                                 SourceFile *initialFile) {
   StringRef filename = module->getModuleFilename();
-  std::string moduleName = module->getNameStr().str();
+  // If this is a cross-import overlay, make sure we use the name of the
+  // underlying module instead.
+  std::string moduleName = getUnderlyingModuleName(module, initialFile);
 
   std::string error;
   auto isUptodateOpt = parentUnitWriter.isUnitUpToDateForOutputFile(/*FilePath=*/filename,
@@ -565,12 +587,12 @@ emitDataForSwiftSerializedModule(ModuleDecl *module,
   ModuleDecl::ImportFilter importFilter;
   importFilter |= ModuleDecl::ImportFilterKind::Public;
   importFilter |= ModuleDecl::ImportFilterKind::Private;
-  // FIXME: ImportFilterKind::ShadowedBySeparateOverlay?
   SmallVector<ModuleDecl::ImportedModule, 8> imports;
   module->getImportedModules(imports, importFilter);
   StringScratchSpace moduleNameScratch;
   addModuleDependencies(imports, indexStorePath, indexSystemModules,
-                        targetTriple, clangCI, diags, unitWriter, moduleNameScratch);
+                        targetTriple, clangCI, diags, unitWriter,
+                        moduleNameScratch, initialFile);
 
   if (unitWriter.write(error)) {
     diags.diagnose(SourceLoc(), diag::error_write_index_unit, error);
@@ -611,7 +633,8 @@ recordSourceFileUnit(SourceFile *primarySourceFile, StringRef indexUnitToken,
   primarySourceFile->getImportedModules(imports, importFilter);
   StringScratchSpace moduleNameScratch;
   addModuleDependencies(imports, indexStorePath, indexSystemModules,
-                        targetTriple, clangCI, diags, unitWriter, moduleNameScratch);
+                        targetTriple, clangCI, diags, unitWriter,
+                        moduleNameScratch, primarySourceFile);
 
   // File dependencies.
   for (auto *F : fileDependencies)
diff --git a/test/Index/Inputs/CrossImport/A.framework/Modules/A.swiftcrossimport/B.swiftoverlay b/test/Index/Inputs/CrossImport/A.framework/Modules/A.swiftcrossimport/B.swiftoverlay
@@ -0,0 +1,5 @@
+%YAML 1.2
+---
+version: 1
+modules:
+  - name: _ABAdditions
diff --git a/test/Index/Inputs/CrossImport/A.framework/Modules/A.swiftmodule/module-triple-here.swiftinterface b/test/Index/Inputs/CrossImport/A.framework/Modules/A.swiftmodule/module-triple-here.swiftinterface
@@ -0,0 +1,4 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name A
+
+public func fromA()
diff --git a/test/Index/Inputs/CrossImport/B.framework/Modules/B.swiftmodule/module-triple-here.swiftinterface b/test/Index/Inputs/CrossImport/B.framework/Modules/B.swiftmodule/module-triple-here.swiftinterface
@@ -0,0 +1,4 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name B
+
+public func fromB()
diff --git a/test/Index/Inputs/CrossImport/C.framework/Modules/C.swiftmodule/module-triple-here.swiftinterface b/test/Index/Inputs/CrossImport/C.framework/Modules/C.swiftmodule/module-triple-here.swiftinterface
@@ -0,0 +1,4 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name C
+
+public func fromC()
diff --git a/test/Index/Inputs/CrossImport/_ABAdditions.framework/Modules/_ABAdditions.swiftcrossimport/C.swiftoverlay b/test/Index/Inputs/CrossImport/_ABAdditions.framework/Modules/_ABAdditions.swiftcrossimport/C.swiftoverlay
@@ -0,0 +1,5 @@
+%YAML 1.2
+---
+version: 1
+modules:
+  - name: __ABAdditionsCAdditions
diff --git a/test/Index/Inputs/CrossImport/_ABAdditions.framework/Modules/_ABAdditions.swiftmodule/module-triple-here.swiftinterface b/test/Index/Inputs/CrossImport/_ABAdditions.framework/Modules/_ABAdditions.swiftmodule/module-triple-here.swiftinterface
@@ -0,0 +1,8 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name _ABAdditions
+
+@_exported import A
+import B
+
+public func from_ABAdditions()
+public struct From_ABAdditionsType {}
diff --git a/test/Index/Inputs/CrossImport/__ABAdditionsCAdditions.framework/Modules/__ABAdditionsCAdditions.swiftmodule/module-triple-here.swiftinterface b/test/Index/Inputs/CrossImport/__ABAdditionsCAdditions.framework/Modules/__ABAdditionsCAdditions.swiftmodule/module-triple-here.swiftinterface
@@ -0,0 +1,7 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name __ABAdditionsCAdditions
+
+@_exported import _ABAdditions
+import C
+
+public func from__ABAdditionsCAdditions()
diff --git a/test/Index/Store/cross-import-overlay.swift b/test/Index/Store/cross-import-overlay.swift
@@ -0,0 +1,64 @@
+// RUN: %empty-directory(%t)
+// RUN: cp -r %S/../Inputs/CrossImport %t/CrossImport
+// RUN: %{python} %S/../../CrossImport/Inputs/rewrite-module-triples.py %t/CrossImport %module-target-triple
+
+// RUN: %target-swift-frontend -c -index-store-path %t/idx -index-system-modules -enable-cross-import-overlays %s -Fsystem %t/CrossImport -o %t/file1.o -module-name cross_import_overlay
+// RUN: c-index-test core -print-unit %t/idx > %t/units
+// RUN: %FileCheck %s --input-file %t/units --check-prefix=UNIT
+// RUN: %FileCheck %s --input-file %t/units --check-prefix=UNIT-NEGATIVE
+
+import A
+import B
+import C
+
+fromA()
+fromB()
+from_ABAdditions()
+from__ABAdditionsCAdditions()
+
+// UNIT: module-name: cross_import_overlay
+// UNIT: main-path: {{.*}}/cross-import-overlay.swift
+// UNIT: DEPEND START
+// UNIT: Unit | system | B | {{.*}}/B.swiftmodule/{{.*}}
+// UNIT: Unit | system | C | {{.*}}/C.swiftmodule/{{.*}}
+// UNIT: Unit | system | A | {{.*}}/__ABAdditionsCAdditions.swiftmodule/{{.*}}
+// UNIT: Record | user | {{.*}}/cross-import-overlay.swift | cross-import-overlay.swift-{{.*}}
+// UNIT: DEPEND END
+// UNIT: --------
+// UNIT: module-name: A
+// UNIT: out-file: {{.*}}/_ABAdditions.swiftmodule/{{.*}}
+// UNIT: DEPEND START
+// UNIT: Unit | system | A | {{.*}}/A.swiftmodule/{{.*}}
+// UNIT: Unit | system | B | {{.*}}/B.swiftmodule/{{.*}}
+// UNIT: Record | system | A | {{.*}}/_ABAdditions.swiftmodule/{{.*}}
+// UNIT: DEPEND END
+// UNIT: --------
+// UNIT: module-name: A
+// UNIT: out-file: {{.*}}/__ABAdditionsCAdditions.swiftmodule/{{.*}}
+// UNIT: DEPEND START
+// UNIT: Unit | system | C | {{.*}}/C.swiftmodule/{{.*}}
+// UNIT: Unit | system | A | {{.*}}/_ABAdditions.swiftmodule/{{.*}}
+// UNIT: Record | system | A | {{.*}}/__ABAdditionsCAdditions.swiftmodule/{{.*}}
+// UNIT: DEPEND END
+// UNIT: --------
+// UNIT: module-name: C
+// UNIT: out-file: {{.*}}/C.swiftmodule/{{.*}}
+// UNIT: DEPEND START
+// UNIT: Record | system | C | {{.*}}/C.swiftmodule/{{.*}}
+// UNIT: DEPEND END
+// UNIT: --------
+// UNIT: module-name: B
+// UNIT: out-file: {{.*}}/B.swiftmodule/{{.*}}
+// UNIT: DEPEND START
+// UNIT: Record | system | B | {{.*}}/B.swiftmodule/{{.*}}
+// UNIT: DEPEND END
+// UNIT: --------
+// UNIT: module-name: A
+// UNIT: out-file: {{.*}}/A.swiftmodule/{{.*}}
+// UNIT: DEPEND START
+// UNIT: Record | system | A | {{.*}}/A.swiftmodule/{{.*}}
+// UNIT: DEPEND END
+
+// UNIT-NEGATIVE-NOT: Unit | {{.*}} | _ABAdditions |
+// UNIT-NEGATIVE-NOT: Unit | {{.*}} | __ABAdditionsCAdditions |
+
diff --git a/test/Index/index_cross_import.swift b/test/Index/index_cross_import.swift
@@ -0,0 +1,31 @@
+// RUN: %empty-directory(%t)
+// RUN: cp -r %S/Inputs/CrossImport %t/CrossImport
+// RUN: %{python} %S/../CrossImport/Inputs/rewrite-module-triples.py %t/CrossImport %module-target-triple
+
+// RUN: %target-swift-ide-test -print-indexed-symbols -enable-cross-import-overlays -source-filename %s -F %t/CrossImport > %t/out
+// RUN: %FileCheck %s --check-prefix=CHECK-NEGATIVE -input-file=%t/out
+//
+// CHECK-NEGATIVE-NOT: module | user | _ABAdditions
+
+// RUN: %FileCheck %s -input-file=%t/out
+//
+// CHECK: index_cross_import.swift
+// CHECK: ------------
+// CHECK-DAG: module | user | A | {{.*}}/A.swiftmodule/{{.*}}
+// CHECK-DAG: module | user | A | {{.*}}/_ABAdditions.swiftmodule/{{.*}}
+// CHECK-DAG: module | user | A | {{.*}}/__ABAdditionsCAdditions.swiftmodule/{{.*}}
+// CHECK-DAG: module | user | B | {{.*}}/B.swiftmodule/{{.*}}
+// CHECK-DAG: module | user | C | {{.*}}/C.swiftmodule/{{.*}}
+// CHECK: ------------
+
+import A
+// CHECK: [[@LINE-1]]:8 | module/Swift | A | {{.*}} | Ref | rel: 0
+import B
+// CHECK: [[@LINE-1]]:8 | module/Swift | B | {{.*}} | Ref | rel: 0
+import C
+// CHECK: [[@LINE-1]]:8 | module/Swift | C | {{.*}} | Ref | rel: 0
+
+from_ABAdditions()
+// CHECK: [[@LINE-1]]:1 | function/Swift | from_ABAdditions() | {{.*}} | Ref,Call | rel: 0
+from__ABAdditionsCAdditions()
+// CHECK: [[@LINE-1]]:1 | function/Swift | from__ABAdditionsCAdditions() | {{.*}} | Ref,Call | rel: 0
diff --git a/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp
@@ -912,12 +912,9 @@ static bool passCursorInfoForDecl(SourceFile* SF,
     if (SF) {
       // In a source file we map the imported overlays to the underlying
       // modules they shadow.
-      while (MD->getNameStr().startswith("_")) {
-        auto *Underlying = SF->getModuleShadowedBySeparatelyImportedOverlay(MD);
-        if (!Underlying)
-          break;
+      auto *Underlying = SF->getModuleShadowedBySeparatelyImportedOverlay(MD);
+      if (Underlying)
         MD = Underlying;
-      }
     } else if (MainModule) {
       // In a module interface we need to map the declared overlays of the main
       // module (which are included in its generated interface) back to the main

Original file line number	Diff line number	Diff line change
`@@ -2105,10 +2105,15 @@ SourceFile::getModuleShadowedBySeparatelyImportedOverlay(const ModuleDecl *overl`
`2105`	`2105`	`}`
`2106`	`2106`	`}`
`2107`	`2107`	`}`
`2108`		`- auto i = separatelyImportedOverlaysReversed.find(overlay);`
`2109`		`- return i != separatelyImportedOverlaysReversed.end()`
`2110`		`- ? std::get<1>(*i)`
`2111`		`- : nullptr;`
	`2108`	`+`
	`2109`	`+ ModuleDecl underlying = const_cast<ModuleDecl >(overlay);`
	`2110`	`+ while (underlying->getNameStr().startswith("_")) {`
	`2111`	`+ auto next = separatelyImportedOverlaysReversed.find(underlying);`
	`2112`	`+ if (next == separatelyImportedOverlaysReversed.end())`
	`2113`	`+ return nullptr;`
	`2114`	`+ underlying = std::get<1>(*next);`
	`2115`	`+ }`
	`2116`	`+ return underlying;`
`2112`	`2117`	`};`
`2113`	`2118`
`2114`	`2119`	`void ModuleDecl::clearLookupCache() {`