Merge pull request #31227 from nathawes/cross-import-tooling-fixes

[IDE][AST] Handle frameworks with traditional overlays where the underlying module declares cross imports in sourcekit

Author: Nathan Hawes

include/swift/AST/FileUnit.h

@@ -272,6 +272,10 @@ class FileUnit : public DeclContext {
     return false;
   }
 
+  virtual ModuleDecl *getUnderlyingModuleIfOverlay() const {
+    return nullptr;
+  }
+
   /// Returns the associated clang module if one exists.
   virtual const clang::Module *getUnderlyingClangModule() const {
     return nullptr;

include/swift/AST/Module.h

@@ -406,12 +406,16 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   /// present overlays as if they were part of their underlying module.
   std::pair<ModuleDecl *, Identifier> getDeclaringModuleAndBystander();
 
+  ///  If this is a traditional (non-cross-import) overlay, get its underlying
+  ///  module if one exists.
+  ModuleDecl *getUnderlyingModuleIfOverlay() const;
+
 public:
 
   /// Returns true if this module is an underscored cross import overlay
-  /// declared by \p other, either directly or transitively (via intermediate
-  /// cross-import overlays - for cross-imports involving more than two
-  /// modules).
+  /// declared by \p other or its underlying clang module, either directly or
+  /// transitively (via intermediate cross-import overlays - for cross-imports
+  /// involving more than two modules).
   bool isCrossImportOverlayOf(ModuleDecl *other);
 
   /// If this module is an underscored cross-import overlay, returns the
@@ -420,16 +424,18 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   /// cross-imports involving more than two modules).
   ModuleDecl *getDeclaringModuleIfCrossImportOverlay();
 
-  /// If this module is an underscored cross-import overlay of \p declaring
-  /// either directly or transitively, populates \p bystanderNames with the set
-  /// of bystander modules that must be present alongside \p declaring for
-  /// the overlay to be imported and returns true. Returns false otherwise.
+  /// If this module is an underscored cross-import overlay of \p declaring or
+  /// its underlying clang module, either directly or transitively, populates
+  /// \p bystanderNames with the set of bystander modules that must be present
+  /// alongside \p declaring for the overlay to be imported and returns true.
+  /// Returns false otherwise.
   bool getRequiredBystandersIfCrossImportOverlay(
       ModuleDecl *declaring, SmallVectorImpl<Identifier> &bystanderNames);
 
 
   /// Walks and loads the declared, underscored cross-import overlays of this
-  /// module, transitively, to find all overlays this module underlies.
+  /// module and its underlying clang module, transitively, to find all cross
+  /// import overlays this module underlies.
   ///
   /// This is used by tooling to present these overlays as part of this module.
   void findDeclaredCrossImportOverlaysTransitive(

include/swift/Serialization/SerializedModuleLoader.h

@@ -416,6 +416,8 @@ class SerializedASTFile final : public LoadedFile {
 
   virtual const clang::Module *getUnderlyingClangModule() const override;
 
+  virtual ModuleDecl *getUnderlyingModuleIfOverlay() const override;
+
   virtual bool getAllGenericSignatures(
                    SmallVectorImpl<GenericSignature> &genericSignatures)
                 override;

lib/AST/Module.cpp

@@ -1724,6 +1724,14 @@ bool ModuleDecl::walk(ASTWalker &Walker) {
   return false;
 }
 
+ModuleDecl *ModuleDecl::getUnderlyingModuleIfOverlay() const {
+  for (auto *FU : getFiles()) {
+    if (auto *Mod = FU->getUnderlyingModuleIfOverlay())
+      return Mod;
+  }
+  return nullptr;
+}
+
 const clang::Module *ModuleDecl::findUnderlyingClangModule() const {
   for (auto *FU : getFiles()) {
     if (auto *Mod = FU->getUnderlyingClangModule())
@@ -1827,6 +1835,9 @@ void ModuleDecl::findDeclaredCrossImportOverlaysTransitive(
   SourceLoc unused;
 
   worklist.push_back(this);
+  if (auto *clangModule = getUnderlyingModuleIfOverlay())
+    worklist.push_back(clangModule);
+
   while (!worklist.empty()) {
     ModuleDecl *current = worklist.back();
     worklist.pop_back();
@@ -1846,13 +1857,37 @@ void ModuleDecl::findDeclaredCrossImportOverlaysTransitive(
           if (seen.insert(overlayMod).second) {
             overlayModules.push_back(overlayMod);
             worklist.push_back(overlayMod);
+            if (auto *clangModule = overlayMod->getUnderlyingModuleIfOverlay())
+              worklist.push_back(clangModule);
           }
         }
       }
     }
   }
 }
 
+namespace {
+  using CrossImportMap =
+      llvm::SmallDenseMap<Identifier, SmallVector<OverlayFile *, 1>>;
+
+
+  Identifier getBystanderIfDeclaring(ModuleDecl *mod, ModuleDecl *overlay,
+                                     CrossImportMap modCrossImports) {
+    auto ret = std::find_if(modCrossImports.begin(), modCrossImports.end(),
+                            [&](CrossImportMap::iterator::value_type &pair) {
+      for (OverlayFile *file: std::get<1>(pair)) {
+        ArrayRef<Identifier> overlays = file->getOverlayModuleNames(
+            mod, SourceLoc(), std::get<0>(pair));
+        if (std::find(overlays.begin(), overlays.end(),
+                      overlay->getName()) != overlays.end())
+          return true;
+      }
+      return false;
+    });
+    return ret != modCrossImports.end() ? ret->first : Identifier();
+  }
+}
+
 std::pair<ModuleDecl *, Identifier>
 ModuleDecl::getDeclaringModuleAndBystander() {
   if (declaringModuleAndBystander)
@@ -1875,25 +1910,19 @@ ModuleDecl::getDeclaringModuleAndBystander() {
     if (!seen.insert(importedModule).second)
       continue;
 
-    using CrossImportMap =
-        llvm::SmallDenseMap<Identifier, SmallVector<OverlayFile *, 1>>;
-    CrossImportMap &crossImports = importedModule->declaredCrossImports;
-
-    // If any of MD's cross imports declare this module as its overlay, report
-    // MD as the underlying module.
-    auto ret = std::find_if(crossImports.begin(), crossImports.end(),
-                            [&](CrossImportMap::iterator::value_type &pair) {
-      for (OverlayFile *file: std::get<1>(pair)) {
-        ArrayRef<Identifier> overlays = file->getOverlayModuleNames(
-            importedModule, SourceLoc(), std::get<0>(pair));
-        if (std::find(overlays.begin(), overlays.end(),
-                      overlayModule->getName()) != overlays.end())
-          return true;
-      }
-      return false;
-    });
-    if (ret != crossImports.end())
-      return *(declaringModuleAndBystander = {importedModule, ret->first});
+    Identifier bystander = getBystanderIfDeclaring(
+        importedModule, overlayModule, importedModule->declaredCrossImports);
+    if (!bystander.empty())
+      return *(declaringModuleAndBystander = {importedModule, bystander});
+
+    // Also check the imported module's underlying module if it's a traditional
+    // overlay (i.e. not a cross-import overlay).
+    if (auto *clangModule = importedModule->getUnderlyingModuleIfOverlay()) {
+      Identifier bystander = getBystanderIfDeclaring(
+          clangModule, overlayModule, clangModule->declaredCrossImports);
+      if (!bystander.empty())
+        return *(declaringModuleAndBystander = {clangModule, bystander});
+    }
 
     if (!importedModule->hasUnderscoredNaming())
       continue;
@@ -1909,8 +1938,9 @@ ModuleDecl::getDeclaringModuleAndBystander() {
 
 bool ModuleDecl::isCrossImportOverlayOf(ModuleDecl *other) {
   ModuleDecl *current = this;
+  ModuleDecl *otherClang = other->getUnderlyingModuleIfOverlay();
   while ((current = current->getDeclaringModuleAndBystander().first)) {
-    if (current == other)
+    if (current == other || current == otherClang)
       return true;
   }
   return false;
@@ -1925,10 +1955,11 @@ ModuleDecl *ModuleDecl::getDeclaringModuleIfCrossImportOverlay() {
 
 bool ModuleDecl::getRequiredBystandersIfCrossImportOverlay(
     ModuleDecl *declaring, SmallVectorImpl<Identifier> &bystanderNames) {
+  auto *clangModule = declaring->getUnderlyingModuleIfOverlay();
   auto current = std::make_pair(this, Identifier());
   while ((current = current.first->getDeclaringModuleAndBystander()).first) {
     bystanderNames.push_back(current.second);
-    if (current.first == declaring)
+    if (current.first == declaring || current.first == clangModule)
       return true;
   }
   return false;

lib/IDE/ModuleInterfacePrinting.cpp

@@ -429,6 +429,9 @@ getDeclsFromCrossImportOverlay(ModuleDecl *Overlay, ModuleDecl *Declaring,
   auto NewEnd = std::partition(Decls.begin(), Decls.end(), [&](Decl *D) {
     if (auto *ID = dyn_cast<ImportDecl>(D)) {
       ModuleDecl *Imported = ID->getModule();
+      if (!Imported)
+        return true;
+
       // Ignore imports of the underlying module, or any cross-import
       // that would map back to it.
       if (Imported == Declaring || Imported->isCrossImportOverlayOf(Declaring))
@@ -497,8 +500,13 @@ static void printCrossImportOverlays(ModuleDecl *Declaring, ASTContext &Ctx,
       continue;
 
     Bystanders.clear();
-    Overlay->getRequiredBystandersIfCrossImportOverlay(Declaring, Bystanders);
-    assert(!Bystanders.empty() && "Overlay with no bystanders?");
+    auto BystandersValid =
+      Overlay->getRequiredBystandersIfCrossImportOverlay(Declaring, Bystanders);
+
+    // Ignore badly formed overlays that don't import their declaring module.
+    if (!BystandersValid)
+      continue;
+
     std::sort(Bystanders.begin(), Bystanders.end(),
               [](Identifier LHS, Identifier RHS) {
       return LHS.str() < RHS.str();

lib/Serialization/SerializedModuleLoader.cpp

@@ -1218,6 +1218,10 @@ StringRef SerializedASTFile::getTargetTriple() const {
   return File.getTargetTriple();
 }
 
+ModuleDecl *SerializedASTFile::getUnderlyingModuleIfOverlay() const {
+  return File.getUnderlyingModule();
+}
+
 const clang::Module *SerializedASTFile::getUnderlyingClangModule() const {
   if (auto *UnderlyingModule = File.getUnderlyingModule())
     return UnderlyingModule->findUnderlyingClangModule();

test/CrossImport/Inputs/rewrite-module-triples.py

@@ -43,7 +43,10 @@ def rewrite(parent, names, copy_fn, rm_fn):
             else:
                 copy_fn(path, new_path)
 
-        rm_fn(path)
+        if platform.system() == 'Windows':
+            rm_fn(u'\\'.join([u'\\\\?', os.path.normpath(path)]))
+        else:
+            rm_fn(path)
 
 
 for parent, dirs, files in os.walk(root_dir, topdown=False):

test/SourceKit/CursorInfo/cursor_info_cross_import_common_case.swift

@@ -0,0 +1,39 @@
+// RUN: %empty-directory(%t)
+// RUN: cp -r %S/../../CrossImport/Inputs/lib-templates/* %t/
+// RUN: %{python} %S/../../CrossImport/Inputs/rewrite-module-triples.py %t %module-target-triple
+
+import BystandingLibrary
+
+
+import SwiftFramework
+
+fromSwiftFramework()
+fromSwiftFrameworkCrossImport()
+
+
+import ClangFramework
+
+fromClangFramework()
+fromClangFrameworkCrossImport()
+
+
+import OverlaidClangFramework
+
+fromOverlaidClangFramework()
+fromOverlaidClangFrameworkOverlay()
+fromOverlaidClangFrameworkCrossImport()
+
+
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=10:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks %s  | %FileCheck -check-prefix=CHECKSWIFT %s
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=11:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks %s  | %FileCheck -check-prefix=CHECKSWIFT %s
+
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=16:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks  %s | %FileCheck -check-prefix=CHECKCLANG %s
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=17:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks  %s | %FileCheck -check-prefix=CHECKCLANG %s
+
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=22:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks  %s | %FileCheck -check-prefix=CHECKOVERLAID %s
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=23:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks  %s | %FileCheck -check-prefix=CHECKOVERLAID %s
+// RUN: %sourcekitd-test -req=cursor -print-raw-response -pos=24:1 %s -- -target %target-triple -Xfrontend -enable-cross-import-overlays -I %t/include -I %t/lib/swift -F %t/Frameworks  %s | %FileCheck -check-prefix=CHECKOVERLAID %s
+
+// CHECKSWIFT: key.modulename: "SwiftFramework"
+// CHECKCLANG: key.modulename: "ClangFramework"
+// CHECKOVERLAID: key.modulename: "OverlaidClangFramework"

test/SourceKit/DocSupport/doc_cross_import_common.swift

@@ -0,0 +1,14 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/mcp)
+// RUN: cp -r %S/../../CrossImport/Inputs/lib-templates/* %t/
+// RUN: %{python} %S/../../CrossImport/Inputs/rewrite-module-triples.py %t %module-target-triple
+
+
+// RUN: %sourcekitd-test -req=doc-info -module SwiftFramework -- -target %target-triple -I %t/include -I %t/lib/swift -F %t/Frameworks -module-cache-path %t/mcp > %t.response
+// RUN: %diff -u %s.SwiftFramework.response %t.response
+
+// RUN: %sourcekitd-test -req=doc-info -module ClangFramework -- -target %target-triple -I %t/include -I %t/lib/swift -F %t/Frameworks -module-cache-path %t/mcp > %t.response
+// RUN: %diff -u %s.ClangFramework.response %t.response
+
+// RUN: %sourcekitd-test -req=doc-info -module OverlaidClangFramework -- -target %target-triple -I %t/include -I %t/lib/swift -F %t/Frameworks -module-cache-path %t/mcp > %t.response
+// RUN: %diff -u %s.OverlaidClangFramework.response %t.response

test/SourceKit/DocSupport/doc_cross_import_common.swift.ClangFramework.response

@@ -0,0 +1,79 @@
+
+func fromClangFramework()
+
+// MARK: - BystandingLibrary Additions
+
+import SwiftOnoneSupport
+
+// Available when BystandingLibrary is imported with ClangFramework
+func fromClangFrameworkCrossImport()
+
+
+[
+  {
+    key.kind: source.lang.swift.syntaxtype.keyword,
+    key.offset: 1,
+    key.length: 4
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.identifier,
+    key.offset: 6,
+    key.length: 18
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.comment,
+    key.offset: 28,
+    key.length: 39
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.comment.mark,
+    key.offset: 31,
+    key.length: 35
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.keyword,
+    key.offset: 68,
+    key.length: 6
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.identifier,
+    key.offset: 75,
+    key.length: 17
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.comment,
+    key.offset: 94,
+    key.length: 68
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.keyword,
+    key.offset: 162,
+    key.length: 4
+  },
+  {
+    key.kind: source.lang.swift.syntaxtype.identifier,
+    key.offset: 167,
+    key.length: 29
+  }
+]
+[
+  {
+    key.kind: source.lang.swift.decl.function.free,
+    key.name: "fromClangFramework()",
+    key.usr: "c:@F@fromClangFramework",
+    key.offset: 1,
+    key.length: 25,
+    key.fully_annotated_decl: "<decl.function.free><syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>fromClangFramework</decl.name>()</decl.function.free>"
+  },
+  {
+    key.kind: source.lang.swift.decl.function.free,
+    key.name: "fromClangFrameworkCrossImport()",
+    key.usr: "s:33_ClangFramework_BystandingLibrary04fromaB11CrossImportyyF",
+    key.offset: 162,
+    key.length: 36,
+    key.fully_annotated_decl: "<decl.function.free><syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>fromClangFrameworkCrossImport</decl.name>()</decl.function.free>",
+    key.required_bystanders: [
+      "BystandingLibrary"
+    ]
+  }
+]