Skip to content

Commit 2d87478

Browse files
QuietMisdreavusQuietMisdreavus
authored
[SymbolGraphGen] only include the given symbol for qualified imports (swiftlang#59852)
* only include the given symbol for qualified imports rdar://96309088 * re-exporting one type should not allow unrelated types to sneak in * ensure that children of re-exported types are also re-exported
1 parent 6b3e18b commit 2d87478

File tree

10 files changed

+146
-13
lines changed

10 files changed

+146
-13
lines changed

‎include/swift/AST/Module.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,9 @@ inline SourceLoc extractNearestSourceLoc(const ModuleDecl *mod) {
949949
}
950950

951951
/// Collects modules that this module imports via `@_exported import`.
952-
void collectParsedExportedImports(const ModuleDecl *M, SmallPtrSetImpl<ModuleDecl *> &Imports);
952+
void collectParsedExportedImports(const ModuleDecl *M,
953+
SmallPtrSetImpl<ModuleDecl *> &Imports,
954+
llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> &QualifiedImports);
953955

954956
} // end namespace swift
955957

‎lib/AST/Module.cpp

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -792,15 +792,33 @@ bool ModuleDecl::shouldCollectDisplayDecls() const {
792792
return true;
793793
}
794794

795-
void swift::collectParsedExportedImports(const ModuleDecl *M, SmallPtrSetImpl<ModuleDecl *> &Imports) {
795+
void swift::collectParsedExportedImports(const ModuleDecl *M,
796+
SmallPtrSetImpl<ModuleDecl *> &Imports,
797+
llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> &QualifiedImports) {
796798
for (const FileUnit *file : M->getFiles()) {
797799
if (const SourceFile *source = dyn_cast<SourceFile>(file)) {
798800
if (source->hasImports()) {
799801
for (auto import : source->getImports()) {
800802
if (import.options.contains(ImportFlags::Exported) &&
801-
!Imports.contains(import.module.importedModule) &&
802803
import.module.importedModule->shouldCollectDisplayDecls()) {
803-
Imports.insert(import.module.importedModule);
804+
auto *TheModule = import.module.importedModule;
805+
806+
if (import.module.getAccessPath().size() > 0) {
807+
if (QualifiedImports.find(TheModule) == QualifiedImports.end()) {
808+
QualifiedImports.try_emplace(TheModule);
809+
}
810+
auto collectDecls = [&](ValueDecl *VD,
811+
DeclVisibilityKind reason) {
812+
if (reason == DeclVisibilityKind::VisibleAtTopLevel)
813+
QualifiedImports[TheModule].insert(VD);
814+
};
815+
auto consumer = makeDeclConsumer(std::move(collectDecls));
816+
TheModule->lookupVisibleDecls(
817+
import.module.getAccessPath(), consumer,
818+
NLKind::UnqualifiedLookup);
819+
} else if (!Imports.contains(TheModule)) {
820+
Imports.insert(TheModule);
821+
}
804822
}
805823
}
806824
}
@@ -956,7 +974,12 @@ SourceFile::getExternalRawLocsForDecl(const Decl *D) const {
956974
void ModuleDecl::getDisplayDecls(SmallVectorImpl<Decl*> &Results, bool Recursive) const {
957975
if (Recursive && isParsedModule(this)) {
958976
SmallPtrSet<ModuleDecl *, 4> Modules;
959-
collectParsedExportedImports(this, Modules);
977+
llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> QualifiedImports;
978+
collectParsedExportedImports(this, Modules, QualifiedImports);
979+
for (const auto &QI : QualifiedImports) {
980+
auto &Decls = QI.getSecond();
981+
Results.append(Decls.begin(), Decls.end());
982+
}
960983
for (const ModuleDecl *import : Modules) {
961984
import->getDisplayDecls(Results, Recursive);
962985
}

‎lib/SymbolGraphGen/SymbolGraph.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ bool SymbolGraph::isUnconditionallyUnavailableOnAllPlatforms(const Decl *D) cons
732732
bool SymbolGraph::canIncludeDeclAsNode(const Decl *D) const {
733733
// If this decl isn't in this module or module that this module imported with `@_exported`, don't record it,
734734
// as it will appear elsewhere in its module's symbol graph.
735-
if (D->getModuleContext()->getName() != M.getName() && !Walker.isFromExportedImportedModule(D)) {
735+
if (D->getModuleContext()->getName() != M.getName() && !Walker.isConsideredExportedImported(D)) {
736736
return false;
737737
}
738738

‎lib/SymbolGraphGen/SymbolGraphASTWalker.cpp

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,23 +37,28 @@ bool areModulesEqual(const ModuleDecl *lhs, const ModuleDecl *rhs, bool ignoreUn
3737

3838
SymbolGraphASTWalker::SymbolGraphASTWalker(ModuleDecl &M,
3939
const SmallPtrSet<ModuleDecl *, 4> ExportedImportedModules,
40+
const llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> QualifiedExportedImports,
4041
const SymbolGraphOptions &Options)
4142
: Options(Options),
4243
M(M),
4344
ExportedImportedModules(ExportedImportedModules),
45+
QualifiedExportedImports(QualifiedExportedImports),
4446
MainGraph(*this, M, None, Ctx) {}
4547

4648
/// Get a "sub" symbol graph for the parent module of a type that
4749
/// the main module `M` is extending.
4850
SymbolGraph *SymbolGraphASTWalker::getModuleSymbolGraph(const Decl *D) {
4951
auto *M = D->getModuleContext();
5052
const auto *DC = D->getDeclContext();
53+
const Decl *ExtendedNominal = nullptr;
5154
while (DC) {
5255
M = DC->getParentModule();
5356
if (const auto *NTD = dyn_cast_or_null<NominalTypeDecl>(DC->getAsDecl())) {
5457
DC = NTD->getDeclContext();
5558
} else if (const auto *Ext = dyn_cast_or_null<ExtensionDecl>(DC->getAsDecl())) {
5659
DC = Ext->getExtendedNominal()->getDeclContext();
60+
if (!ExtendedNominal)
61+
ExtendedNominal = Ext->getExtendedNominal();
5762
} else {
5863
DC = nullptr;
5964
}
@@ -67,8 +72,16 @@ SymbolGraph *SymbolGraphASTWalker::getModuleSymbolGraph(const Decl *D) {
6772
// should put actual extensions of that module into the main graph
6873
return &MainGraph;
6974
}
70-
71-
if (isExportedImportedModule(M)) {
75+
76+
// Check the module and decl separately since the extension could be from a different module
77+
// than the decl itself.
78+
if (isExportedImportedModule(M) || isQualifiedExportedImport(D)) {
79+
return &MainGraph;
80+
}
81+
82+
if (ExtendedNominal && isFromExportedImportedModule(ExtendedNominal)) {
83+
return &MainGraph;
84+
} else if (!ExtendedNominal && isConsideredExportedImported(D)) {
7285
return &MainGraph;
7386
}
7487

@@ -230,9 +243,49 @@ bool SymbolGraphASTWalker::walkToDeclPre(Decl *D, CharSourceRange Range) {
230243
return true;
231244
}
232245

246+
bool SymbolGraphASTWalker::isConsideredExportedImported(const Decl *D) const {
247+
// First check the decl itself to see if it was directly re-exported.
248+
if (isFromExportedImportedModule(D))
249+
return true;
250+
251+
const auto *DC = D->getDeclContext();
252+
253+
// Next, see if the decl is a child symbol of another decl that was re-exported.
254+
if (DC) {
255+
if (const auto *VD = dyn_cast_or_null<ValueDecl>(DC->getAsDecl())) {
256+
if (isFromExportedImportedModule(VD))
257+
return true;
258+
}
259+
}
260+
261+
// Finally, check to see if this decl is an extension of something else that was re-exported.
262+
// FIXME: this considers synthesized members of extensions to be valid
263+
const Decl *ExtendedNominal = nullptr;
264+
while (DC && !ExtendedNominal) {
265+
if (const auto *ED = dyn_cast_or_null<ExtensionDecl>(DC->getAsDecl())) {
266+
ExtendedNominal = ED->getExtendedNominal();
267+
} else {
268+
DC = DC->getParent();
269+
}
270+
}
271+
272+
if (ExtendedNominal && isFromExportedImportedModule(ExtendedNominal)) {
273+
return true;
274+
}
275+
276+
// If none of the other checks passed, this wasn't from a re-export.
277+
return false;
278+
}
279+
233280
bool SymbolGraphASTWalker::isFromExportedImportedModule(const Decl* D) const {
234281
auto *M = D->getModuleContext();
235-
return isExportedImportedModule(M);
282+
return isQualifiedExportedImport(D) || isExportedImportedModule(M);
283+
}
284+
285+
bool SymbolGraphASTWalker::isQualifiedExportedImport(const Decl *D) const {
286+
return llvm::any_of(QualifiedExportedImports, [&D](const auto &QI) {
287+
return QI.getSecond().contains(D);
288+
});
236289
}
237290

238291
bool SymbolGraphASTWalker::isExportedImportedModule(const ModuleDecl *M) const {

‎lib/SymbolGraphGen/SymbolGraphASTWalker.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ struct SymbolGraphASTWalker : public SourceEntityWalker {
4949

5050
const SmallPtrSet<ModuleDecl *, 4> ExportedImportedModules;
5151

52+
const llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> QualifiedExportedImports;
53+
5254
/// The symbol graph for the main module of interest.
5355
SymbolGraph MainGraph;
5456

@@ -59,6 +61,7 @@ struct SymbolGraphASTWalker : public SourceEntityWalker {
5961

6062
SymbolGraphASTWalker(ModuleDecl &M,
6163
const SmallPtrSet<ModuleDecl *, 4> ExportedImportedModules,
64+
const llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> QualifiedExportedImports,
6265
const SymbolGraphOptions &Options);
6366
virtual ~SymbolGraphASTWalker() {}
6467

@@ -93,10 +96,17 @@ struct SymbolGraphASTWalker : public SourceEntityWalker {
9396
virtual bool walkToDeclPre(Decl *D, CharSourceRange Range) override;
9497

9598
// MARK: - Utilities
99+
100+
/// Returns whether the given declaration was itself imported via an `@_exported import`
101+
/// statement, or if it is an extension or child symbol of something else that was.
102+
virtual bool isConsideredExportedImported(const Decl *D) const;
96103

97104
/// Returns whether the given declaration comes from an `@_exported import` module.
98105
virtual bool isFromExportedImportedModule(const Decl *D) const;
99106

107+
/// Returns whether the given declaration was imported via an `@_exported import <type>` declaration.
108+
virtual bool isQualifiedExportedImport(const Decl *D) const;
109+
100110
/// Returns whether the given module is an `@_exported import` module.
101111
virtual bool isExportedImportedModule(const ModuleDecl *M) const;
102112
};

‎lib/SymbolGraphGen/SymbolGraphGen.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,14 @@ symbolgraphgen::emitSymbolGraphForModule(ModuleDecl *M,
5959
swift::getTopLevelDeclsForDisplay(M, ModuleDecls, /*recursive*/true);
6060

6161
SmallPtrSet<ModuleDecl *, 4> ExportedImportedModules;
62-
swift::collectParsedExportedImports(M, ExportedImportedModules);
62+
llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> QualifiedImports;
63+
swift::collectParsedExportedImports(M, ExportedImportedModules, QualifiedImports);
6364

6465
if (Options.PrintMessages)
6566
llvm::errs() << ModuleDecls.size()
6667
<< " top-level declarations in this module.\n";
6768

68-
SymbolGraphASTWalker Walker(*M, ExportedImportedModules, Options);
69+
SymbolGraphASTWalker Walker(*M, ExportedImportedModules, QualifiedImports, Options);
6970

7071
for (auto *Decl : ModuleDecls) {
7172
Walker.walk(Decl);
@@ -102,7 +103,8 @@ printSymbolGraphForDecl(const ValueDecl *D, Type BaseTy,
102103

103104
llvm::json::OStream JOS(OS, Options.PrettyPrint ? 2 : 0);
104105
ModuleDecl *MD = D->getModuleContext();
105-
SymbolGraphASTWalker Walker(*MD, {}, Options);
106+
llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> QualifiedImports;
107+
SymbolGraphASTWalker Walker(*MD, {}, QualifiedImports, Options);
106108
markup::MarkupContext MarkupCtx;
107109
SymbolGraph Graph(Walker, *MD, None, MarkupCtx, None,
108110
/*IsForSingleNode=*/true);

‎test/SymbolGraph/Module/ExportedImport.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend %S/Inputs/ExportedImport/A.swift -module-name A -emit-module -emit-module-path %t/A.swiftmodule
3+
// RUN: %target-swift-frontend %S/Inputs/ExportedImport/B.swift -module-name B -emit-module -emit-module-path %t/B.swiftmodule
34
// RUN: %target-swift-frontend %s -module-name ExportedImport -emit-module -emit-module-path /dev/null -I %t -emit-symbol-graph -emit-symbol-graph-dir %t/
45
// RUN: %FileCheck %s --input-file %t/ExportedImport.symbols.json
56
// RUN: ls %t | %FileCheck %s --check-prefix FILES
67

78
@_exported import A
9+
@_exported import struct B.StructOne
810

9-
// CHECK: "precise":"s:1A11SymbolFromAV"
1011
// CHECK-NOT: InternalSymbolFromA
12+
// CHECK-NOT: StructTwo
13+
// CHECK-DAG: "precise":"s:1A11SymbolFromAV"
14+
// CHECK-DAG: "precise":"s:1B9StructOneV"
1115

1216
// FIXME: Symbols from `@_exported import` do not get emitted when using swift-symbolgraph-extract
1317
// This is tracked by https://bugs.swift.org/browse/SR-15921.

‎test/SymbolGraph/Module/Inputs/ExportedImport/B.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
public struct StructOne {}
2+
3+
public struct StructTwo {}

‎test/SymbolGraph/Module/Inputs/QualifiedImportWithExtensions/A.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
public struct StructOne {
2+
public init() {}
3+
}
4+
5+
public struct StructTwo {}

‎test/SymbolGraph/Module/QualifiedImportWithExtensions.swift

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend %S/Inputs/QualifiedImportWithExtensions/A.swift -module-name A -emit-module -emit-module-path %t/A.swiftmodule
3+
// RUN: %target-swift-frontend %s -module-name QualifiedImportWithExtensions -emit-module -emit-module-path /dev/null -I %t -emit-symbol-graph -emit-symbol-graph-dir %t/
4+
// RUN: %FileCheck %s --input-file %t/QualifiedImportWithExtensions.symbols.json
5+
// RUN: %FileCheck %s --input-file %t/[email protected] --check-prefix EXT
6+
7+
@_exported import struct A.StructOne
8+
import A
9+
10+
// An extension to a type that's been re-exported with a qualified import should appear in the
11+
// main symbol graph.
12+
extension A.StructOne {
13+
public struct ExtendedStruct {}
14+
}
15+
16+
// An extension to a type that's _not_ been re-exported should still appear in the extension symbol
17+
// graph, even if a different type from that module has been re-exported.
18+
extension A.StructTwo {
19+
public struct InnerStruct {}
20+
}
21+
22+
// Make sure that the `ExtendedStruct` type is present, and also that `StructOne`'s initializer is
23+
// present as well. The `InnerStruct` type should not be present since it's extending a type
24+
// that's not being re-exported.
25+
26+
// CHECK-NOT: InnerStruct
27+
// CHECK-DAG: ExtendedStruct
28+
// CHECK-DAG: "precise":"s:1A9StructOneVACycfc"
29+
30+
// EXT: InnerStruct
31+
// EXT-NOT: ExtendedStruct

0 commit comments

Comments
 (0)