Skip to content

Commit 57450f5

Browse files
[SymbolGraphGen] Un-revert #78959 and clean up usage of DenseMap (#79124)
* Revert "Revert "[SymbolGraphGen] synthesize child symbols for type aliases of private…" (#79062)" This reverts commit cac8297. * clean up use of DenseMap in SymbolGraphGen rdar://143865173
1 parent 8daf94c commit 57450f5

File tree

8 files changed

+226
-62
lines changed

8 files changed

+226
-62
lines changed

lib/SymbolGraphGen/Symbol.cpp

+7-5
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,21 @@ using namespace swift;
3737
using namespace symbolgraphgen;
3838

3939
Symbol::Symbol(SymbolGraph *Graph, const ExtensionDecl *ED,
40-
const NominalTypeDecl *SynthesizedBaseTypeDecl, Type BaseType)
40+
const ValueDecl *SynthesizedBaseTypeDecl, Type BaseType)
4141
: Symbol::Symbol(Graph, nullptr, ED, SynthesizedBaseTypeDecl, BaseType) {}
4242

4343
Symbol::Symbol(SymbolGraph *Graph, const ValueDecl *VD,
44-
const NominalTypeDecl *SynthesizedBaseTypeDecl, Type BaseType)
44+
const ValueDecl *SynthesizedBaseTypeDecl, Type BaseType)
4545
: Symbol::Symbol(Graph, VD, nullptr, SynthesizedBaseTypeDecl, BaseType) {}
4646

4747
Symbol::Symbol(SymbolGraph *Graph, const ValueDecl *VD, const ExtensionDecl *ED,
48-
const NominalTypeDecl *SynthesizedBaseTypeDecl, Type BaseType)
48+
const ValueDecl *SynthesizedBaseTypeDecl, Type BaseType)
4949
: Graph(Graph), D(VD), BaseType(BaseType),
5050
SynthesizedBaseTypeDecl(SynthesizedBaseTypeDecl) {
51-
if (!BaseType && SynthesizedBaseTypeDecl)
52-
BaseType = SynthesizedBaseTypeDecl->getDeclaredInterfaceType();
51+
if (!BaseType && SynthesizedBaseTypeDecl) {
52+
if (const auto *NTD = dyn_cast<NominalTypeDecl>(SynthesizedBaseTypeDecl))
53+
BaseType = NTD->getDeclaredInterfaceType();
54+
}
5355
if (D == nullptr) {
5456
D = ED;
5557
}

lib/SymbolGraphGen/Symbol.h

+17-16
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ class Symbol {
3535
/// Either a ValueDecl* or ExtensionDecl*.
3636
const Decl *D;
3737
Type BaseType;
38-
const NominalTypeDecl *SynthesizedBaseTypeDecl;
38+
const ValueDecl *SynthesizedBaseTypeDecl;
3939

4040
Symbol(SymbolGraph *Graph, const ValueDecl *VD, const ExtensionDecl *ED,
41-
const NominalTypeDecl *SynthesizedBaseTypeDecl,
41+
const ValueDecl *SynthesizedBaseTypeDecl,
4242
Type BaseTypeForSubstitution = Type());
4343

4444
swift::DeclName getName(const Decl *D) const;
@@ -87,11 +87,11 @@ class Symbol {
8787

8888
public:
8989
Symbol(SymbolGraph *Graph, const ExtensionDecl *ED,
90-
const NominalTypeDecl *SynthesizedBaseTypeDecl,
90+
const ValueDecl *SynthesizedBaseTypeDecl,
9191
Type BaseTypeForSubstitution = Type());
9292

9393
Symbol(SymbolGraph *Graph, const ValueDecl *VD,
94-
const NominalTypeDecl *SynthesizedBaseTypeDecl,
94+
const ValueDecl *SynthesizedBaseTypeDecl,
9595
Type BaseTypeForSubstitution = Type());
9696

9797
void serialize(llvm::json::OStream &OS) const;
@@ -108,7 +108,7 @@ class Symbol {
108108
return BaseType;
109109
}
110110

111-
const NominalTypeDecl *getSynthesizedBaseTypeDecl() const {
111+
const ValueDecl *getSynthesizedBaseTypeDecl() const {
112112
return SynthesizedBaseTypeDecl;
113113
}
114114

@@ -175,27 +175,28 @@ using SymbolGraph = swift::symbolgraphgen::SymbolGraph;
175175

176176
template <> struct DenseMapInfo<Symbol> {
177177
static inline Symbol getEmptyKey() {
178-
return Symbol {
179-
DenseMapInfo<SymbolGraph *>::getEmptyKey(),
180-
DenseMapInfo<const swift::ValueDecl *>::getEmptyKey(),
181-
DenseMapInfo<const swift::NominalTypeDecl *>::getTombstoneKey(),
182-
DenseMapInfo<swift::Type>::getEmptyKey(),
178+
return Symbol{
179+
DenseMapInfo<SymbolGraph *>::getEmptyKey(),
180+
DenseMapInfo<const swift::ValueDecl *>::getEmptyKey(),
181+
DenseMapInfo<const swift::ValueDecl *>::getTombstoneKey(),
182+
DenseMapInfo<swift::Type>::getEmptyKey(),
183183
};
184184
}
185185
static inline Symbol getTombstoneKey() {
186-
return Symbol {
187-
DenseMapInfo<SymbolGraph *>::getTombstoneKey(),
188-
DenseMapInfo<const swift::ValueDecl *>::getTombstoneKey(),
189-
DenseMapInfo<const swift::NominalTypeDecl *>::getTombstoneKey(),
190-
DenseMapInfo<swift::Type>::getTombstoneKey(),
186+
return Symbol{
187+
DenseMapInfo<SymbolGraph *>::getTombstoneKey(),
188+
DenseMapInfo<const swift::ValueDecl *>::getTombstoneKey(),
189+
DenseMapInfo<const swift::ValueDecl *>::getTombstoneKey(),
190+
DenseMapInfo<swift::Type>::getTombstoneKey(),
191191
};
192192
}
193193
static unsigned getHashValue(const Symbol S) {
194194
unsigned H = 0;
195195
H ^= DenseMapInfo<SymbolGraph *>::getHashValue(S.getGraph());
196196
H ^=
197197
DenseMapInfo<const swift::Decl *>::getHashValue(S.getLocalSymbolDecl());
198-
H ^= DenseMapInfo<const swift::NominalTypeDecl *>::getHashValue(S.getSynthesizedBaseTypeDecl());
198+
H ^= DenseMapInfo<const swift::ValueDecl *>::getHashValue(
199+
S.getSynthesizedBaseTypeDecl());
199200
H ^= DenseMapInfo<swift::Type>::getHashValue(S.getBaseType());
200201
return H;
201202
}

lib/SymbolGraphGen/SymbolGraph.cpp

+79-34
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,18 @@ void SymbolGraph::recordEdge(Symbol Source,
233233

234234
void SymbolGraph::recordMemberRelationship(Symbol S) {
235235
const auto *DC = S.getLocalSymbolDecl()->getDeclContext();
236+
const ValueDecl *ParentDecl = DC->getSelfNominalTypeDecl();
237+
if (!ParentDecl) {
238+
// If we couldn't look up the type the member is declared on (e.g.
239+
// because the member is declared in an extension whose extended type
240+
// doesn't exist), don't record a memberOf relationship.
241+
return;
242+
}
243+
if (const auto *PublicDecl = Walker.PublicPrivateTypeAliases.lookup(ParentDecl)) {
244+
// If our member target is a private type that has a public type alias,
245+
// point the membership to that type alias instead.
246+
ParentDecl = PublicDecl;
247+
}
236248
switch (DC->getContextKind()) {
237249
case DeclContextKind::GenericTypeDecl:
238250
case DeclContextKind::ExtensionDecl:
@@ -251,13 +263,6 @@ void SymbolGraph::recordMemberRelationship(Symbol S) {
251263
return;
252264
}
253265

254-
if (DC->getSelfNominalTypeDecl() == nullptr) {
255-
// If we couldn't look up the type the member is declared on (e.g.
256-
// because the member is declared in an extension whose extended type
257-
// doesn't exist), don't record a memberOf relationship.
258-
return;
259-
}
260-
261266
// If this is an extension to an external type, we use the extension
262267
// symbol itself as the target.
263268
if (auto const *Extension =
@@ -269,8 +274,7 @@ void SymbolGraph::recordMemberRelationship(Symbol S) {
269274
}
270275
}
271276

272-
return recordEdge(S,
273-
Symbol(this, DC->getSelfNominalTypeDecl(), nullptr),
277+
return recordEdge(S, Symbol(this, ParentDecl, nullptr),
274278
RelationshipKind::MemberOf());
275279
case swift::DeclContextKind::AbstractClosureExpr:
276280
case swift::DeclContextKind::SerializedAbstractClosure:
@@ -323,7 +327,16 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
323327
bool dropSynthesizedMembers = !Walker.Options.EmitSynthesizedMembers ||
324328
Walker.Options.SkipProtocolImplementations;
325329

326-
const auto D = S.getLocalSymbolDecl();
330+
const auto *D = S.getLocalSymbolDecl();
331+
332+
// If this symbol is a public type alias to a private symbol, collect
333+
// synthesized members for the underlying type.
334+
if (const auto *TD = dyn_cast<TypeAliasDecl>(D)) {
335+
const auto *NTD = TD->getUnderlyingType()->getAnyNominal();
336+
if (NTD && Walker.PublicPrivateTypeAliases.lookup(NTD) == D)
337+
D = NTD;
338+
}
339+
327340
const NominalTypeDecl *OwningNominal = nullptr;
328341
if (const auto *ThisNominal = dyn_cast<NominalTypeDecl>(D)) {
329342
OwningNominal = ThisNominal;
@@ -376,9 +389,11 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
376389
// that protocol would otherwise be hidden.
377390
if (auto *Nominal = Info.Ext->getExtendedNominal()) {
378391
if (dropSynthesizedMembers &&
379-
!isImplicitlyPrivate(
380-
Nominal, /*IgnoreContext =*/Nominal->getModuleContext() ==
381-
StdlibModule))
392+
!isImplicitlyPrivate(Nominal, /*IgnoreContext =*/
393+
[&](const Decl *P) {
394+
return Nominal->getModuleContext() ==
395+
StdlibModule;
396+
}))
382397
continue;
383398
} else if (dropSynthesizedMembers) {
384399
continue;
@@ -393,10 +408,12 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
393408
// There can be synthesized members on effectively private
394409
// protocols or things that conform to them. We don't want to
395410
// include those.
396-
if (isImplicitlyPrivate(SynthMember,
397-
/*IgnoreContext =*/
398-
SynthMember->getModuleContext() ==
399-
StdlibModule)) {
411+
if (isImplicitlyPrivate(
412+
SynthMember,
413+
/*IgnoreContext =*/
414+
[&](const Decl *P) {
415+
return SynthMember->getModuleContext() == StdlibModule;
416+
})) {
400417
continue;
401418
}
402419

@@ -405,22 +422,29 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
405422
continue;
406423
}
407424

408-
Symbol Source(this, SynthMember, OwningNominal);
425+
const ValueDecl *BaseDecl = OwningNominal;
426+
if (const auto *PublicDecl = Walker.PublicPrivateTypeAliases.lookup(BaseDecl))
427+
BaseDecl = PublicDecl;
428+
429+
Symbol Source(this, SynthMember, BaseDecl);
409430

410431
if (auto *InheritedDecl = Source.getInheritedDecl()) {
411432
if (auto *ParentDecl =
412433
InheritedDecl->getDeclContext()->getAsDecl()) {
413434
if (dropSynthesizedMembers &&
414435
!isImplicitlyPrivate(
415436
ParentDecl,
416-
/*IgnoreContext =*/ParentDecl->getModuleContext() ==
417-
StdlibModule)) {
437+
/*IgnoreContext =*/
438+
[&](const Decl *P) {
439+
return ParentDecl->getModuleContext() ==
440+
StdlibModule;
441+
})) {
418442
continue;
419443
}
420444
}
421445
}
422446

423-
auto ExtendedSG = Walker.getModuleSymbolGraph(OwningNominal);
447+
auto ExtendedSG = Walker.getModuleSymbolGraph(BaseDecl);
424448

425449
ExtendedSG->Nodes.insert(Source);
426450

@@ -433,7 +457,15 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
433457

434458
void
435459
SymbolGraph::recordInheritanceRelationships(Symbol S) {
436-
const auto D = S.getLocalSymbolDecl();
460+
const auto *D = S.getLocalSymbolDecl();
461+
462+
// If this is a public type alias for a private symbol, gather inheritance
463+
// for the underlying type instead.
464+
if (const auto *TD = dyn_cast<TypeAliasDecl>(D)) {
465+
const auto *NTD = TD->getUnderlyingType()->getAnyNominal();
466+
if (NTD && Walker.PublicPrivateTypeAliases.lookup(NTD) == D)
467+
D = NTD;
468+
}
437469

438470
ClassDecl *Super = nullptr;
439471
if (auto *CD = dyn_cast<ClassDecl>(D))
@@ -442,8 +474,7 @@ SymbolGraph::recordInheritanceRelationships(Symbol S) {
442474
Super = PD->getSuperclassDecl();
443475

444476
if (Super) {
445-
recordEdge(Symbol(this, cast<ValueDecl>(D), nullptr),
446-
Symbol(this, Super, nullptr),
477+
recordEdge(S, Symbol(this, Super, nullptr),
447478
RelationshipKind::InheritsFrom());
448479
}
449480
}
@@ -522,7 +553,16 @@ void SymbolGraph::recordOptionalRequirementRelationships(Symbol S) {
522553
}
523554

524555
void SymbolGraph::recordConformanceRelationships(Symbol S) {
525-
const auto D = S.getLocalSymbolDecl();
556+
const auto *D = S.getLocalSymbolDecl();
557+
558+
// If this is a public type alias for a private symbol, gather conformances
559+
// for the underlying type instead.
560+
if (const auto *TD = dyn_cast<TypeAliasDecl>(D)) {
561+
const auto *NTD = TD->getUnderlyingType()->getAnyNominal();
562+
if (NTD && Walker.PublicPrivateTypeAliases.lookup(NTD) == D)
563+
D = NTD;
564+
}
565+
526566
if (const auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
527567
if (auto *PD = dyn_cast<ProtocolDecl>(NTD)) {
528568
for (auto *inherited : PD->getAllInheritedProtocols()) {
@@ -700,8 +740,8 @@ const ValueDecl *getProtocolRequirement(const ValueDecl *VD) {
700740

701741
}
702742

703-
bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
704-
bool IgnoreContext) const {
743+
bool SymbolGraph::isImplicitlyPrivate(
744+
const Decl *D, llvm::function_ref<bool(const Decl *)> IgnoreContext) const {
705745
// Don't record unconditionally private declarations
706746
if (D->isPrivateSystemDecl(/*treatNonBuiltinProtocolsAsPublic=*/false)) {
707747
return true;
@@ -809,16 +849,15 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
809849
if (IsGlobalSIMDType) {
810850
return true;
811851
}
812-
813-
if (IgnoreContext) {
814-
return false;
815-
}
816852
}
817853

818854
// Check up the parent chain. Anything inside a privately named
819855
// thing is also private. We could be looking at the `B` of `_A.B`.
820856
if (const auto *DC = D->getDeclContext()) {
821857
if (const auto *Parent = DC->getAsDecl()) {
858+
if (IgnoreContext && IgnoreContext(Parent))
859+
return false;
860+
822861
// Exception: Children of underscored protocols should be considered
823862
// public, even though the protocols themselves aren't. This way,
824863
// synthesized copies of those symbols are correctly added to the public
@@ -851,7 +890,11 @@ bool SymbolGraph::isUnconditionallyUnavailableOnAllPlatforms(const Decl *D) cons
851890
}
852891

853892
/// Returns `true` if the symbol should be included as a node in the graph.
854-
bool SymbolGraph::canIncludeDeclAsNode(const Decl *D) const {
893+
bool SymbolGraph::canIncludeDeclAsNode(const Decl *D,
894+
const Decl *PublicAncestorDecl) const {
895+
if (PublicAncestorDecl && D == PublicAncestorDecl)
896+
return true;
897+
855898
// If this decl isn't in this module or module that this module imported with `@_exported`, don't record it,
856899
// as it will appear elsewhere in its module's symbol graph.
857900

@@ -873,6 +916,8 @@ bool SymbolGraph::canIncludeDeclAsNode(const Decl *D) const {
873916
} else {
874917
return false;
875918
}
876-
return !isImplicitlyPrivate(cast<ValueDecl>(D))
877-
&& !isUnconditionallyUnavailableOnAllPlatforms(cast<ValueDecl>(D));
919+
return !isImplicitlyPrivate(
920+
cast<ValueDecl>(D), /*IgnoreContext=*/
921+
[&](const Decl *P) { return P == PublicAncestorDecl; }) &&
922+
!isUnconditionallyUnavailableOnAllPlatforms(cast<ValueDecl>(D));
878923
}

lib/SymbolGraphGen/SymbolGraph.h

+11-5
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,12 @@ struct SymbolGraph {
220220
/// implicitly internal/private, such as underscore prefixes,
221221
/// and checking every named parent context as well.
222222
///
223-
/// \param IgnoreContext If `true`, don't consider
224-
/// the context of the declaration to determine whether it is implicitly private.
225-
bool isImplicitlyPrivate(const Decl *D,
226-
bool IgnoreContext = false) const;
223+
/// \param IgnoreContext A function ref that receives the parent decl
224+
/// and returns whether or not the context should be ignored when determining
225+
/// privacy.
226+
bool isImplicitlyPrivate(
227+
const Decl *D,
228+
llvm::function_ref<bool(const Decl *)> IgnoreContext = nullptr) const;
227229

228230
/// Returns `true` if the declaration has an availability attribute
229231
/// that marks it as unconditionally unavailable on all platforms (i.e., where
@@ -232,7 +234,11 @@ struct SymbolGraph {
232234

233235
/// Returns `true` if the declaration should be included as a node
234236
/// in the graph.
235-
bool canIncludeDeclAsNode(const Decl *D) const;
237+
///
238+
/// If `PublicAncestorDecl` is set and is an ancestor of `D`, that declaration
239+
/// is considered to be public, regardless of its surrounding context.
240+
bool canIncludeDeclAsNode(const Decl *D,
241+
const Decl *PublicAncestorDecl = nullptr) const;
236242

237243
/// Returns `true` if the declaration is a requirement of a protocol
238244
/// or is a default implementation of a protocol

0 commit comments

Comments
 (0)