Skip to content

Commit dc27126

Browse files
author
Nathan Hawes
committed
[IDE][AST] Handle frameworks with traditional overlays where the underlying module declares cross imports in sourcekit.
We weren't handling this case, so their generated interfaces / doc info wouldn't include symbols from the cross-import overlays, and we wouldn't map the underscored cross-import overlay name back to the declaring framework's name in cusor-info, completion results or when indexing. Resolves rdar://problem/62138551
1 parent b130ece commit dc27126

15 files changed

+842
-31
lines changed

include/swift/AST/FileUnit.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ class FileUnit : public DeclContext {
272272
return false;
273273
}
274274

275+
virtual ModuleDecl *getUnderlyingModuleIfOverlay() const {
276+
return nullptr;
277+
}
278+
275279
/// Returns the associated clang module if one exists.
276280
virtual const clang::Module *getUnderlyingClangModule() const {
277281
return nullptr;

include/swift/AST/Module.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,16 @@ class ModuleDecl : public DeclContext, public TypeDecl {
351351
/// present overlays as if they were part of their underlying module.
352352
std::pair<ModuleDecl *, Identifier> getDeclaringModuleAndBystander();
353353

354+
/// If this is a traditional (non-cross-import) overlay, get its underlying
355+
/// module if one exists.
356+
ModuleDecl *getUnderlyingModuleIfOverlay() const;
357+
354358
public:
355359

356360
/// Returns true if this module is an underscored cross import overlay
357-
/// declared by \p other, either directly or transitively (via intermediate
358-
/// cross-import overlays - for cross-imports involving more than two
359-
/// modules).
361+
/// declared by \p other or its underlying clang module, either directly or
362+
/// transitively (via intermediate cross-import overlays - for cross-imports
363+
/// involving more than two modules).
360364
bool isCrossImportOverlayOf(ModuleDecl *other);
361365

362366
/// If this module is an underscored cross-import overlay, returns the
@@ -365,16 +369,18 @@ class ModuleDecl : public DeclContext, public TypeDecl {
365369
/// cross-imports involving more than two modules).
366370
ModuleDecl *getDeclaringModuleIfCrossImportOverlay();
367371

368-
/// If this module is an underscored cross-import overlay of \p declaring
369-
/// either directly or transitively, populates \p bystanderNames with the set
370-
/// of bystander modules that must be present alongside \p declaring for
371-
/// the overlay to be imported and returns true. Returns false otherwise.
372+
/// If this module is an underscored cross-import overlay of \p declaring or
373+
/// its underlying clang module, either directly or transitively, populates
374+
/// \p bystanderNames with the set of bystander modules that must be present
375+
/// alongside \p declaring for the overlay to be imported and returns true.
376+
/// Returns false otherwise.
372377
bool getRequiredBystandersIfCrossImportOverlay(
373378
ModuleDecl *declaring, SmallVectorImpl<Identifier> &bystanderNames);
374379

375380

376381
/// Walks and loads the declared, underscored cross-import overlays of this
377-
/// module, transitively, to find all overlays this module underlies.
382+
/// module and its underlying clang module, transitively, to find all cross
383+
/// import overlays this module underlies.
378384
///
379385
/// This is used by tooling to present these overlays as part of this module.
380386
void findDeclaredCrossImportOverlaysTransitive(

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,8 @@ class SerializedASTFile final : public LoadedFile {
416416

417417
virtual const clang::Module *getUnderlyingClangModule() const override;
418418

419+
virtual ModuleDecl *getUnderlyingModuleIfOverlay() const override;
420+
419421
virtual bool getAllGenericSignatures(
420422
SmallVectorImpl<GenericSignature> &genericSignatures)
421423
override;

lib/AST/Module.cpp

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,6 +1716,14 @@ bool ModuleDecl::walk(ASTWalker &Walker) {
17161716
return false;
17171717
}
17181718

1719+
ModuleDecl *ModuleDecl::getUnderlyingModuleIfOverlay() const {
1720+
for (auto *FU : getFiles()) {
1721+
if (auto *Mod = FU->getUnderlyingModuleIfOverlay())
1722+
return Mod;
1723+
}
1724+
return nullptr;
1725+
}
1726+
17191727
const clang::Module *ModuleDecl::findUnderlyingClangModule() const {
17201728
for (auto *FU : getFiles()) {
17211729
if (auto *Mod = FU->getUnderlyingClangModule())
@@ -1819,6 +1827,9 @@ void ModuleDecl::findDeclaredCrossImportOverlaysTransitive(
18191827
SourceLoc unused;
18201828

18211829
worklist.push_back(this);
1830+
if (auto *clangModule = getUnderlyingModuleIfOverlay())
1831+
worklist.push_back(clangModule);
1832+
18221833
while (!worklist.empty()) {
18231834
ModuleDecl *current = worklist.back();
18241835
worklist.pop_back();
@@ -1838,13 +1849,37 @@ void ModuleDecl::findDeclaredCrossImportOverlaysTransitive(
18381849
if (seen.insert(overlayMod).second) {
18391850
overlayModules.push_back(overlayMod);
18401851
worklist.push_back(overlayMod);
1852+
if (auto *clangModule = overlayMod->getUnderlyingModuleIfOverlay())
1853+
worklist.push_back(clangModule);
18411854
}
18421855
}
18431856
}
18441857
}
18451858
}
18461859
}
18471860

1861+
namespace {
1862+
using CrossImportMap =
1863+
llvm::SmallDenseMap<Identifier, SmallVector<OverlayFile *, 1>>;
1864+
1865+
1866+
Identifier getBystanderIfDeclaring(ModuleDecl *mod, ModuleDecl *overlay,
1867+
CrossImportMap modCrossImports) {
1868+
auto ret = std::find_if(modCrossImports.begin(), modCrossImports.end(),
1869+
[&](CrossImportMap::iterator::value_type &pair) {
1870+
for (OverlayFile *file: std::get<1>(pair)) {
1871+
ArrayRef<Identifier> overlays = file->getOverlayModuleNames(
1872+
mod, SourceLoc(), std::get<0>(pair));
1873+
if (std::find(overlays.begin(), overlays.end(),
1874+
overlay->getName()) != overlays.end())
1875+
return true;
1876+
}
1877+
return false;
1878+
});
1879+
return ret != modCrossImports.end() ? ret->first : Identifier();
1880+
}
1881+
}
1882+
18481883
std::pair<ModuleDecl *, Identifier>
18491884
ModuleDecl::getDeclaringModuleAndBystander() {
18501885
if (declaringModuleAndBystander)
@@ -1867,25 +1902,19 @@ ModuleDecl::getDeclaringModuleAndBystander() {
18671902
if (!seen.insert(importedModule).second)
18681903
continue;
18691904

1870-
using CrossImportMap =
1871-
llvm::SmallDenseMap<Identifier, SmallVector<OverlayFile *, 1>>;
1872-
CrossImportMap &crossImports = importedModule->declaredCrossImports;
1873-
1874-
// If any of MD's cross imports declare this module as its overlay, report
1875-
// MD as the underlying module.
1876-
auto ret = std::find_if(crossImports.begin(), crossImports.end(),
1877-
[&](CrossImportMap::iterator::value_type &pair) {
1878-
for (OverlayFile *file: std::get<1>(pair)) {
1879-
ArrayRef<Identifier> overlays = file->getOverlayModuleNames(
1880-
importedModule, SourceLoc(), std::get<0>(pair));
1881-
if (std::find(overlays.begin(), overlays.end(),
1882-
overlayModule->getName()) != overlays.end())
1883-
return true;
1884-
}
1885-
return false;
1886-
});
1887-
if (ret != crossImports.end())
1888-
return *(declaringModuleAndBystander = {importedModule, ret->first});
1905+
Identifier bystander = getBystanderIfDeclaring(
1906+
importedModule, overlayModule, importedModule->declaredCrossImports);
1907+
if (!bystander.empty())
1908+
return *(declaringModuleAndBystander = {importedModule, bystander});
1909+
1910+
// Also check the imported module's underlying module if it's a traditional
1911+
// overlay (i.e. not a cross-import overlay).
1912+
if (auto *clangModule = importedModule->getUnderlyingModuleIfOverlay()) {
1913+
Identifier bystander = getBystanderIfDeclaring(
1914+
clangModule, overlayModule, clangModule->declaredCrossImports);
1915+
if (!bystander.empty())
1916+
return *(declaringModuleAndBystander = {clangModule, bystander});
1917+
}
18891918

18901919
if (!importedModule->hasUnderscoredNaming())
18911920
continue;
@@ -1901,8 +1930,9 @@ ModuleDecl::getDeclaringModuleAndBystander() {
19011930

19021931
bool ModuleDecl::isCrossImportOverlayOf(ModuleDecl *other) {
19031932
ModuleDecl *current = this;
1933+
ModuleDecl *otherClang = other->getUnderlyingModuleIfOverlay();
19041934
while ((current = current->getDeclaringModuleAndBystander().first)) {
1905-
if (current == other)
1935+
if (current == other || current == otherClang)
19061936
return true;
19071937
}
19081938
return false;
@@ -1917,10 +1947,11 @@ ModuleDecl *ModuleDecl::getDeclaringModuleIfCrossImportOverlay() {
19171947

19181948
bool ModuleDecl::getRequiredBystandersIfCrossImportOverlay(
19191949
ModuleDecl *declaring, SmallVectorImpl<Identifier> &bystanderNames) {
1950+
auto *clangModule = declaring->getUnderlyingModuleIfOverlay();
19201951
auto current = std::make_pair(this, Identifier());
19211952
while ((current = current.first->getDeclaringModuleAndBystander()).first) {
19221953
bystanderNames.push_back(current.second);
1923-
if (current.first == declaring)
1954+
if (current.first == declaring || current.first == clangModule)
19241955
return true;
19251956
}
19261957
return false;

lib/IDE/ModuleInterfacePrinting.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,9 @@ getDeclsFromCrossImportOverlay(ModuleDecl *Overlay, ModuleDecl *Declaring,
429429
auto NewEnd = std::partition(Decls.begin(), Decls.end(), [&](Decl *D) {
430430
if (auto *ID = dyn_cast<ImportDecl>(D)) {
431431
ModuleDecl *Imported = ID->getModule();
432+
if (!Imported)
433+
return true;
434+
432435
// Ignore imports of the underlying module, or any cross-import
433436
// that would map back to it.
434437
if (Imported == Declaring || Imported->isCrossImportOverlayOf(Declaring))
@@ -497,8 +500,13 @@ static void printCrossImportOverlays(ModuleDecl *Declaring, ASTContext &Ctx,
497500
continue;
498501

499502
Bystanders.clear();
500-
Overlay->getRequiredBystandersIfCrossImportOverlay(Declaring, Bystanders);
501-
assert(!Bystanders.empty() && "Overlay with no bystanders?");
503+
auto BystandersValid =
504+
Overlay->getRequiredBystandersIfCrossImportOverlay(Declaring, Bystanders);
505+
506+
// Ignore badly formed overlays that don't import their declaring module.
507+
if (!BystandersValid)
508+
continue;
509+
502510
std::sort(Bystanders.begin(), Bystanders.end(),
503511
[](Identifier LHS, Identifier RHS) {
504512
return LHS.str() < RHS.str();

lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,10 @@ StringRef SerializedASTFile::getTargetTriple() const {
12181218
return File.getTargetTriple();
12191219
}
12201220

1221+
ModuleDecl *SerializedASTFile::getUnderlyingModuleIfOverlay() const {
1222+
return File.getUnderlyingModule();
1223+
}
1224+
12211225
const clang::Module *SerializedASTFile::getUnderlyingClangModule() const {
12221226
if (auto *UnderlyingModule = File.getUnderlyingModule())
12231227
return UnderlyingModule->findUnderlyingClangModule();
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp -r %S/../../CrossImport/Inputs/lib-templates/* %t/
3+
// RUN: %{python} %S/../../CrossImport/Inputs/rewrite-module-triples.py %t %module-target-triple
4+
5+
import BystandingLibrary
6+
7+
8+
import SwiftFramework
9+
10+
fromSwiftFramework()
11+
fromSwiftFrameworkCrossImport()
12+
13+
14+
import ClangFramework
15+
16+
fromClangFramework()
17+
fromClangFrameworkCrossImport()
18+
19+
20+
import OverlaidClangFramework
21+
22+
fromOverlaidClangFramework()
23+
fromOverlaidClangFrameworkOverlay()
24+
fromOverlaidClangFrameworkCrossImport()
25+
26+
27+
// 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
28+
// 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
29+
30+
// 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
31+
// 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
32+
33+
// 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
34+
// 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
35+
// 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
36+
37+
// CHECKSWIFT: key.modulename: "SwiftFramework"
38+
// CHECKCLANG: key.modulename: "ClangFramework"
39+
// CHECKOVERLAID: key.modulename: "OverlaidClangFramework"
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %empty-directory(%t/mcp)
3+
// RUN: cp -r %S/../../CrossImport/Inputs/lib-templates/* %t/
4+
// RUN: %{python} %S/../../CrossImport/Inputs/rewrite-module-triples.py %t %module-target-triple
5+
6+
7+
// 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
8+
// RUN: %diff -u %s.SwiftFramework.response %t.response
9+
10+
// 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
11+
// RUN: %diff -u %s.ClangFramework.response %t.response
12+
13+
// 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
14+
// RUN: %diff -u %s.OverlaidClangFramework.response %t.response
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
2+
func fromClangFramework()
3+
4+
// MARK: - BystandingLibrary Additions
5+
6+
import SwiftOnoneSupport
7+
8+
// Available when BystandingLibrary is imported with ClangFramework
9+
func fromClangFrameworkCrossImport()
10+
11+
12+
[
13+
{
14+
key.kind: source.lang.swift.syntaxtype.keyword,
15+
key.offset: 1,
16+
key.length: 4
17+
},
18+
{
19+
key.kind: source.lang.swift.syntaxtype.identifier,
20+
key.offset: 6,
21+
key.length: 18
22+
},
23+
{
24+
key.kind: source.lang.swift.syntaxtype.comment,
25+
key.offset: 28,
26+
key.length: 39
27+
},
28+
{
29+
key.kind: source.lang.swift.syntaxtype.comment.mark,
30+
key.offset: 31,
31+
key.length: 35
32+
},
33+
{
34+
key.kind: source.lang.swift.syntaxtype.keyword,
35+
key.offset: 68,
36+
key.length: 6
37+
},
38+
{
39+
key.kind: source.lang.swift.syntaxtype.identifier,
40+
key.offset: 75,
41+
key.length: 17
42+
},
43+
{
44+
key.kind: source.lang.swift.syntaxtype.comment,
45+
key.offset: 94,
46+
key.length: 68
47+
},
48+
{
49+
key.kind: source.lang.swift.syntaxtype.keyword,
50+
key.offset: 162,
51+
key.length: 4
52+
},
53+
{
54+
key.kind: source.lang.swift.syntaxtype.identifier,
55+
key.offset: 167,
56+
key.length: 29
57+
}
58+
]
59+
[
60+
{
61+
key.kind: source.lang.swift.decl.function.free,
62+
key.name: "fromClangFramework()",
63+
key.usr: "c:@F@fromClangFramework",
64+
key.offset: 1,
65+
key.length: 25,
66+
key.fully_annotated_decl: "<decl.function.free><syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>fromClangFramework</decl.name>()</decl.function.free>"
67+
},
68+
{
69+
key.kind: source.lang.swift.decl.function.free,
70+
key.name: "fromClangFrameworkCrossImport()",
71+
key.usr: "s:33_ClangFramework_BystandingLibrary04fromaB11CrossImportyyF",
72+
key.offset: 162,
73+
key.length: 36,
74+
key.fully_annotated_decl: "<decl.function.free><syntaxtype.keyword>func</syntaxtype.keyword> <decl.name>fromClangFrameworkCrossImport</decl.name>()</decl.function.free>",
75+
key.required_bystanders: [
76+
"BystandingLibrary"
77+
]
78+
}
79+
]

0 commit comments

Comments
 (0)