Skip to content

Commit 60da292

Browse files
bnbarhambnbarham
committed
Merge exported modules with the same public name in generated interface
If a module has the same `public-module-name` as the module being generated and its import is exported, merge it into the same generated interface. Fix various always-imported modules from being imported while here and update all the tests that checked for them. Resolves rdar://137887712.
1 parent 6735659 commit 60da292

30 files changed

+2801
-2989
lines changed

‎include/swift/IDE/ModuleInterfacePrinting.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,14 @@ namespace ide {
3838
/// Flags used when traversing a module for printing.
3939
enum class ModuleTraversal : unsigned {
4040
/// Visit modules even if their contents wouldn't be visible to name lookup.
41-
VisitHidden = 0x01,
41+
VisitHidden = 0x01,
4242
/// Visit submodules.
4343
VisitSubmodules = 0x02,
4444
/// Skip the declarations in a Swift overlay module.
45-
SkipOverlay = 0x04,
45+
SkipOverlay = 0x04,
46+
/// Visit exported modules where their public module name matches the current
47+
/// module.
48+
VisitMatchingExported = 0x08,
4649
};
4750

4851
/// Options used to describe the traversal of a module for printing.

‎lib/IDE/ModuleInterfacePrinting.cpp

Lines changed: 97 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,33 @@ static bool compareSwiftDecls(Decl *LHS, Decl *RHS) {
304304
return LHS->getKind() < RHS->getKind();
305305
}
306306

307+
static bool shouldPrintImport(ImportDecl *ImportD, ModuleDecl *OrigMod, const clang::Module *OrigClangMod) {
308+
if (ImportD->getAttrs().hasAttribute<ImplementationOnlyAttr>())
309+
return false;
310+
311+
auto *ImportedMod = ImportD->getModule();
312+
if (ImportedMod) {
313+
if (ImportedMod == OrigMod)
314+
return false;
315+
if (ImportedMod->isOnoneSupportModule())
316+
return false;
317+
if (ImportedMod->getName().hasUnderscoredNaming())
318+
return false;
319+
}
320+
321+
if (!OrigClangMod)
322+
return true;
323+
324+
auto ImportedClangMod = ImportD->getClangModule();
325+
if (!ImportedClangMod)
326+
return true;
327+
if (!ImportedClangMod->isSubModule())
328+
return true;
329+
if (ImportedClangMod == OrigClangMod)
330+
return false;
331+
return ImportedClangMod->isSubModuleOf(OrigClangMod);
332+
}
333+
307334
static std::pair<ArrayRef<Decl*>, ArrayRef<Decl*>>
308335
getDeclsFromCrossImportOverlay(ModuleDecl *Overlay, ModuleDecl *Declaring,
309336
SmallVectorImpl<Decl *> &Decls,
@@ -329,7 +356,8 @@ getDeclsFromCrossImportOverlay(ModuleDecl *Overlay, ModuleDecl *Declaring,
329356

330357
// Ignore imports of the underlying module, or any cross-import
331358
// that would map back to it.
332-
if (Imported == Declaring || Imported->isCrossImportOverlayOf(Declaring))
359+
if (!shouldPrintImport(ID, Declaring, nullptr) ||
360+
Imported->isCrossImportOverlayOf(Declaring))
333361
return false;
334362

335363
// Ignore an imports of modules also imported by the underlying module.
@@ -457,19 +485,40 @@ void swift::ide::printModuleInterface(
457485
auto AdjustedOptions = Options;
458486
adjustPrintOptions(AdjustedOptions);
459487

488+
llvm::DenseSet<const void *> SeenImportedDecls;
460489
SmallVector<ModuleDecl *, 1> ModuleList;
461490
ModuleList.push_back(TargetMod);
491+
SeenImportedDecls.insert(TargetMod);
462492

463-
SmallVector<ImportDecl *, 1> ImportDecls;
464-
llvm::DenseSet<const clang::Module *> ClangModulesForImports;
465-
SmallVector<Decl *, 1> SwiftDecls;
493+
SmallVector<ImportDecl *, 0> ImportDecls;
494+
SmallVector<Decl *, 0> SwiftDecls;
466495
llvm::DenseMap<const clang::Module *,
467-
SmallVector<std::pair<Decl *, clang::SourceLocation>, 1>>
468-
ClangDecls;
496+
SmallVector<std::pair<Decl *, clang::SourceLocation>, 0>>
497+
ClangDecls;
498+
499+
// Add exported modules that have the same public module name as this module
500+
// (excluding the underlying clang module if there is one).
501+
if (TraversalOptions & ModuleTraversal::VisitMatchingExported) {
502+
SmallVector<ImportedModule> Imports;
503+
TargetMod->getImportedModules(Imports,
504+
ModuleDecl::ImportFilterKind::Exported);
505+
for (ImportedModule Import : Imports) {
506+
if (Import.importedModule->getPublicModuleName(
507+
/*onlyIfImported=*/false) != TargetMod->getName())
508+
continue;
509+
510+
if (TargetClangMod != nullptr &&
511+
Import.importedModule->findUnderlyingClangModule() != TargetClangMod)
512+
continue;
513+
514+
ModuleList.push_back(Import.importedModule);
515+
SeenImportedDecls.insert(Import.importedModule);
516+
}
517+
}
469518

470-
// If we're printing recursively, find all of the submodules to print.
471519
if (TargetClangMod) {
472-
if (TraversalOptions) {
520+
// Add clang submodules if they're being visited
521+
if (TraversalOptions & ModuleTraversal::VisitSubmodules) {
473522
SmallVector<const clang::Module *, 8> Worklist;
474523
SmallPtrSet<const clang::Module *, 8> Visited;
475524
Worklist.push_back(TargetClangMod);
@@ -482,16 +531,15 @@ void swift::ide::printModuleInterface(
482531

483532
ClangDecls.insert({ CM, {} });
484533

485-
if (CM != TargetClangMod)
486-
if (auto *OwningModule = Importer.getWrapperForModule(CM))
534+
if (CM != TargetClangMod) {
535+
if (auto *OwningModule = Importer.getWrapperForModule(CM)) {
487536
ModuleList.push_back(OwningModule);
537+
}
538+
}
488539

489-
// If we're supposed to visit submodules, add them now.
490-
if (TraversalOptions & ModuleTraversal::VisitSubmodules) {
491-
for (clang::Module * submodule: CM->submodules()) {
492-
if (Visited.insert(submodule).second) {
493-
Worklist.push_back(submodule);
494-
}
540+
for (clang::Module *submodule : CM->submodules()) {
541+
if (Visited.insert(submodule).second) {
542+
Worklist.push_back(submodule);
495543
}
496544
}
497545
}
@@ -500,8 +548,7 @@ void swift::ide::printModuleInterface(
500548
}
501549
}
502550

503-
SmallVector<Decl *, 1> Decls;
504-
551+
SmallVector<Decl *, 0> Decls;
505552
for (ModuleDecl *M : ModuleList) {
506553
swift::getTopLevelDeclsForDisplay(M, Decls);
507554
}
@@ -527,42 +574,38 @@ void swift::ide::printModuleInterface(
527574
continue;
528575
}
529576

530-
auto ShouldPrintImport = [&](ImportDecl *ImportD) -> bool {
531-
if (ImportD->getAttrs().hasAttribute<ImplementationOnlyAttr>())
532-
return false;
577+
if (auto ID = dyn_cast<ImportDecl>(D)) {
578+
if (!shouldPrintImport(ID, TargetMod, TargetClangMod))
579+
continue;
533580

534-
if (!TargetClangMod)
535-
return true;
536-
if (ImportD->getModule() == TargetMod)
537-
return false;
581+
// Erase submodules that are not missing
582+
if (ID->getClangModule())
583+
NoImportSubModules.erase(ID->getClangModule());
538584

539-
auto ImportedMod = ImportD->getClangModule();
540-
if (!ImportedMod)
541-
return true;
542-
if (!ImportedMod->isSubModule())
543-
return true;
544-
if (ImportedMod == TargetClangMod)
545-
return false;
546-
return ImportedMod->isSubModuleOf(TargetClangMod);
547-
};
585+
if (ID->getImportKind() == ImportKind::Module) {
586+
// Could have a duplicate import from a clang module's overlay or
587+
// because we're merging modules. Skip them.
548588

549-
if (auto ID = dyn_cast<ImportDecl>(D)) {
550-
if (ShouldPrintImport(ID)) {
551-
if (ID->getClangModule())
552-
// Erase those submodules that are not missing.
553-
NoImportSubModules.erase(ID->getClangModule());
554-
if (ID->getImportKind() == ImportKind::Module) {
555-
// Make sure we don't print duplicate imports, due to getting imports
556-
// for both a clang module and its overlay.
557-
if (auto *ClangMod = getUnderlyingClangModuleForImport(ID)) {
558-
auto P = ClangModulesForImports.insert(ClangMod);
559-
bool IsNew = P.second;
560-
if (!IsNew)
561-
continue;
562-
}
589+
if (auto *ClangMod = getUnderlyingClangModuleForImport(ID)) {
590+
if (!SeenImportedDecls.insert(ClangMod).second)
591+
continue;
563592
}
564-
ImportDecls.push_back(ID);
593+
594+
if (auto *ImportedMod = ID->getModule()) {
595+
if (!SeenImportedDecls.insert(ImportedMod).second)
596+
continue;
597+
}
598+
} else {
599+
bool AnyNewDecls = false;
600+
for (auto *ImportedDecl : ID->getDecls()) {
601+
AnyNewDecls |= SeenImportedDecls.insert(ImportedDecl).second;
602+
}
603+
if (!AnyNewDecls)
604+
continue;
565605
}
606+
607+
ImportDecls.push_back(ID);
608+
566609
continue;
567610
}
568611

@@ -684,9 +727,12 @@ void swift::ide::printModuleInterface(
684727

685728
// Imports from the stdlib are internal details that don't need to be exposed.
686729
if (!TargetMod->isStdlibModule()) {
687-
for (auto *D : ImportDecls)
730+
for (auto *D : ImportDecls) {
688731
PrintDecl(D);
689-
Printer.printNewline();
732+
}
733+
if (!ImportDecls.empty()) {
734+
Printer.printNewline();
735+
}
690736
}
691737

692738
{

‎test/SourceKit/CursorInfo/cursor_generated_interface.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public class ASwiftType {
4141
}
4242

4343
// LibA is a mixed framework with no source info and a submodule
44-
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=12:36 -print-raw-response | %FileCheck %s --check-prefix=CHECKA
44+
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=11:36 -print-raw-response | %FileCheck %s --check-prefix=CHECKA
4545
// CHECKA: key.name: "ASwiftType"
4646
// CHECKA: key.modulename: "LibA"
4747
// CHECKA: key.decl_lang: source.lang.swift
@@ -60,7 +60,7 @@ framework module LibA {
6060
@interface AObjcType
6161
@end
6262

63-
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=12:54 -print-raw-response | %FileCheck %s --check-prefix=CHECKAOBJ
63+
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=11:54 -print-raw-response | %FileCheck %s --check-prefix=CHECKAOBJ
6464
// CHECKAOBJ: key.name: "AObjcType"
6565
// CHECKAOBJ: key.line: [[@LINE-5]]
6666
// CHECKAOBJ: key.column: 12
@@ -72,7 +72,7 @@ framework module LibA {
7272
@interface ASubType
7373
@end
7474

75-
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=12:70 -print-raw-response | %FileCheck %s --check-prefix=CHECKASUB
75+
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=11:70 -print-raw-response | %FileCheck %s --check-prefix=CHECKASUB
7676
// CHECKASUB: key.name: "ASubType"
7777
// CHECKASUB: key.line: [[@LINE-5]]
7878
// CHECKASUB: key.column: 12
@@ -84,7 +84,7 @@ framework module LibA {
8484
public class BType {}
8585

8686
// LibB has source info
87-
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=14:32 -print-raw-response | %FileCheck %s --check-prefix=CHECKB
87+
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=13:32 -print-raw-response | %FileCheck %s --check-prefix=CHECKB
8888
// CHECKB: key.name: "BType"
8989
// CHECKB: key.line: [[@LINE-5]]
9090
// CHECKB: key.column: 14
@@ -96,7 +96,7 @@ public class BType {}
9696
public class CType {}
9797

9898
// LibC has no source info
99-
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=14:47 -print-raw-response | %FileCheck %s --check-prefix=CHECKC
99+
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=13:47 -print-raw-response | %FileCheck %s --check-prefix=CHECKC
100100
// CHECKC: key.name: "CType"
101101
// CHECKC: key.modulename: "LibC"
102102
// CHECKC: key.decl_lang: source.lang.swift
@@ -105,7 +105,7 @@ public class CType {}
105105
@interface DType
106106
@end
107107
108-
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=14:57 -print-raw-response | %FileCheck %s --check-prefix=CHECKD
108+
// RUN: %sourcekitd-test -req=interface-gen-open -module LibA -- -F %t/frameworks -target %target-triple == -req=cursor -pos=13:57 -print-raw-response | %FileCheck %s --check-prefix=CHECKD
109109
// CHECKD: key.name: "DType"
110110
// CHECKD: key.line: [[@LINE-5]]
111111
// CHECKD: key.column: 12

0 commit comments

Comments
 (0)