Skip to content

Commit d5d0001

Browse files
QuietMisdreavusQuietMisdreavus
authored
[SymbolGraphGen] improve handling of underscored protocols (#77251)
* treat children of underscored protocols as public Children of underscored protocols should be treated as native children of their conforming types. To accomplish this, ignore underscored protocols in the isInherentlyPrivate check. rdar://124483146 * include underscored protocol methods even when skipping protocols rdar://128143861
1 parent f66a2b1 commit d5d0001

File tree

5 files changed

+189
-47
lines changed

5 files changed

+189
-47
lines changed

‎lib/SymbolGraphGen/Edge.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,29 @@
2020
using namespace swift;
2121
using namespace symbolgraphgen;
2222

23+
namespace {
24+
25+
bool parentDeclIsPrivate(const ValueDecl *VD, SymbolGraph *Graph) {
26+
if (!VD)
27+
return false;
28+
29+
auto *ParentDecl = VD->getDeclContext()->getAsDecl();
30+
31+
if (auto *ParentExtension = dyn_cast_or_null<ExtensionDecl>(ParentDecl)) {
32+
if (auto *Nominal = ParentExtension->getExtendedNominal()) {
33+
return Graph->isImplicitlyPrivate(Nominal);
34+
}
35+
}
36+
37+
if (ParentDecl) {
38+
return Graph->isImplicitlyPrivate(ParentDecl);
39+
} else {
40+
return false;
41+
}
42+
}
43+
44+
} // anonymous namespace
45+
2346
void Edge::serialize(llvm::json::OStream &OS) const {
2447
OS.object([&](){
2548
OS.attribute("kind", Kind.Name);
@@ -62,7 +85,7 @@ void Edge::serialize(llvm::json::OStream &OS) const {
6285

6386
// If our source symbol is a inheriting decl, write in information about
6487
// where it's inheriting docs from.
65-
if (InheritingDecl) {
88+
if (InheritingDecl && !parentDeclIsPrivate(InheritingDecl, Graph)) {
6689
Symbol inheritedSym(Graph, InheritingDecl, nullptr);
6790
SmallString<256> USR, Display;
6891
llvm::raw_svector_ostream DisplayOS(Display);

‎lib/SymbolGraphGen/SymbolGraph.cpp

Lines changed: 92 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,13 @@ bool SymbolGraph::synthesizedMemberIsBestCandidate(const ValueDecl *VD,
315315
}
316316

317317
void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
318-
if (!Walker.Options.EmitSynthesizedMembers || Walker.Options.SkipProtocolImplementations) {
319-
return;
320-
}
318+
// Even if we don't want to emit synthesized members or protocol
319+
// implementations, we still want to emit synthesized members from hidden
320+
// underscored protocols. Save this check so we can skip emitting members
321+
// later if needed.
322+
bool dropSynthesizedMembers = !Walker.Options.EmitSynthesizedMembers ||
323+
Walker.Options.SkipProtocolImplementations;
324+
321325
const auto D = S.getLocalSymbolDecl();
322326
const NominalTypeDecl *OwningNominal = nullptr;
323327
if (const auto *ThisNominal = dyn_cast<NominalTypeDecl>(D)) {
@@ -341,59 +345,89 @@ void SymbolGraph::recordConformanceSynthesizedMemberRelationships(Symbol S) {
341345
PrintOptions::printModuleInterface(
342346
OwningNominal->getASTContext().TypeCheckerOpts.PrintFullConvention));
343347
auto MergeGroupKind = SynthesizedExtensionAnalyzer::MergeGroupKind::All;
344-
ExtensionAnalyzer.forEachExtensionMergeGroup(MergeGroupKind,
345-
[&](ArrayRef<ExtensionInfo> ExtensionInfos){
346-
for (const auto &Info : ExtensionInfos) {
347-
if (!Info.IsSynthesized) {
348-
continue;
349-
}
350-
351-
// We are only interested in synthesized members that come from an
352-
// extension that we defined in our module.
353-
if (Info.EnablingExt) {
354-
const auto *ExtM = Info.EnablingExt->getModuleContext();
355-
if (!Walker.isOurModule(ExtM))
356-
continue;
357-
}
358-
359-
// If D is not the OwningNominal, it is an ExtensionDecl. In that case
360-
// we only want to get members that were enabled by this exact extension.
361-
if (D != OwningNominal && Info.EnablingExt != D) {
362-
continue;
363-
}
364-
365-
for (const auto ExtensionMember : Info.Ext->getMembers()) {
366-
if (const auto SynthMember = dyn_cast<ValueDecl>(ExtensionMember)) {
367-
if (SynthMember->isObjC()) {
348+
ExtensionAnalyzer.forEachExtensionMergeGroup(
349+
MergeGroupKind, [&](ArrayRef<ExtensionInfo> ExtensionInfos) {
350+
const auto StdlibModule =
351+
OwningNominal->getASTContext().getStdlibModule(
352+
/*loadIfAbsent=*/true);
353+
354+
for (const auto &Info : ExtensionInfos) {
355+
if (!Info.IsSynthesized) {
368356
continue;
369357
}
370358

371-
const auto StdlibModule = OwningNominal->getASTContext()
372-
.getStdlibModule(/*loadIfAbsent=*/true);
359+
// We are only interested in synthesized members that come from an
360+
// extension that we defined in our module.
361+
if (Info.EnablingExt) {
362+
const auto *ExtM = Info.EnablingExt->getModuleContext();
363+
if (!Walker.isOurModule(ExtM))
364+
continue;
365+
}
373366

374-
// There can be synthesized members on effectively private protocols
375-
// or things that conform to them. We don't want to include those.
376-
if (isImplicitlyPrivate(SynthMember,
377-
/*IgnoreContext =*/
378-
SynthMember->getModuleContext() == StdlibModule)) {
367+
// If D is not the OwningNominal, it is an ExtensionDecl. In that case
368+
// we only want to get members that were enabled by this exact
369+
// extension.
370+
if (D != OwningNominal && Info.EnablingExt != D) {
379371
continue;
380372
}
381373

382-
if (!synthesizedMemberIsBestCandidate(SynthMember, OwningNominal)) {
374+
// Extensions to protocols should generate synthesized members only if
375+
// that protocol would otherwise be hidden.
376+
if (auto *Nominal = Info.Ext->getExtendedNominal()) {
377+
if (dropSynthesizedMembers &&
378+
!isImplicitlyPrivate(
379+
Nominal, /*IgnoreContext =*/Nominal->getModuleContext() ==
380+
StdlibModule))
381+
continue;
382+
} else if (dropSynthesizedMembers) {
383383
continue;
384384
}
385385

386-
auto ExtendedSG = Walker.getModuleSymbolGraph(OwningNominal);
386+
for (const auto ExtensionMember : Info.Ext->getMembers()) {
387+
if (const auto SynthMember = dyn_cast<ValueDecl>(ExtensionMember)) {
388+
if (SynthMember->isObjC()) {
389+
continue;
390+
}
391+
392+
// There can be synthesized members on effectively private
393+
// protocols or things that conform to them. We don't want to
394+
// include those.
395+
if (isImplicitlyPrivate(SynthMember,
396+
/*IgnoreContext =*/
397+
SynthMember->getModuleContext() ==
398+
StdlibModule)) {
399+
continue;
400+
}
401+
402+
if (!synthesizedMemberIsBestCandidate(SynthMember,
403+
OwningNominal)) {
404+
continue;
405+
}
406+
407+
Symbol Source(this, SynthMember, OwningNominal);
408+
409+
if (auto *InheritedDecl = Source.getInheritedDecl()) {
410+
if (auto *ParentDecl =
411+
InheritedDecl->getDeclContext()->getAsDecl()) {
412+
if (dropSynthesizedMembers &&
413+
!isImplicitlyPrivate(
414+
ParentDecl,
415+
/*IgnoreContext =*/ParentDecl->getModuleContext() ==
416+
StdlibModule)) {
417+
continue;
418+
}
419+
}
420+
}
387421

388-
Symbol Source(this, SynthMember, OwningNominal);
422+
auto ExtendedSG = Walker.getModuleSymbolGraph(OwningNominal);
389423

390-
ExtendedSG->Nodes.insert(Source);
424+
ExtendedSG->Nodes.insert(Source);
391425

392-
ExtendedSG->recordEdge(Source, S, RelationshipKind::MemberOf());
393-
}
394-
}
395-
}
396-
});
426+
ExtendedSG->recordEdge(Source, S, RelationshipKind::MemberOf());
427+
}
428+
}
429+
}
430+
});
397431
}
398432

399433
void
@@ -784,6 +818,21 @@ bool SymbolGraph::isImplicitlyPrivate(const Decl *D,
784818
// thing is also private. We could be looking at the `B` of `_A.B`.
785819
if (const auto *DC = D->getDeclContext()) {
786820
if (const auto *Parent = DC->getAsDecl()) {
821+
// Exception: Children of underscored protocols should be considered
822+
// public, even though the protocols themselves aren't. This way,
823+
// synthesized copies of those symbols are correctly added to the public
824+
// API of public types that conform to underscored protocols.
825+
if (isa<ProtocolDecl>(Parent) && Parent->hasUnderscoredNaming()) {
826+
return false;
827+
}
828+
if (const auto *ParentExtension = dyn_cast<ExtensionDecl>(Parent)) {
829+
if (const auto *ParentNominal = ParentExtension->getExtendedNominal()) {
830+
if (isa<ProtocolDecl>(ParentNominal) &&
831+
ParentNominal->hasUnderscoredNaming()) {
832+
return false;
833+
}
834+
}
835+
}
787836
return isImplicitlyPrivate(Parent, IgnoreContext);
788837
}
789838
}

‎test/SymbolGraph/ClangImporter/ErrorEnum.swift

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// REQUIRES: OS=macosx
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: split-file %s %t
5+
6+
// RUN: %target-swift-symbolgraph-extract -sdk %sdk -module-name MyError -I %t/MyError -output-dir %t -pretty-print -v -skip-protocol-implementations
7+
// RUN: %FileCheck %s --input-file %t/MyError.symbols.json
8+
9+
// CHECK-NOT: "sourceOrigin"
10+
11+
// CHECK-NOT: "displayName": "_BridgedStoredNSError.Code"
12+
13+
// == is implemented as part of the hidden _BridgedStoredNSError protocol,
14+
// but it should be hidden since it ultimately comes from Equatable
15+
// CHECK-NOT: "precise": "s:10Foundation21_BridgedStoredNSErrorPAAE2eeoiySbx_xtFZ::SYNTHESIZED::s:SC11MyErrorCodeLeV"
16+
17+
// hash(into:) is implemented as part of an extension on that same protocol,
18+
// but it should be hidden for a similar reason
19+
// CHECK-NOT: "precise": "s:SYsSHRzSH8RawValueSYRpzrlE4hash4intoys6HasherVz_tF::SYNTHESIZED::c:@E@MyErrorCode"
20+
21+
// MyErrorCode.Code
22+
// CHECK-DAG: "precise": "c:@E@MyErrorCode"
23+
24+
// MyErrorCode.Code.init(rawValue:)
25+
// CHECK-DAG: "precise": "s:So11MyErrorCodeV8rawValueABSgs5Int32V_tcfc"
26+
27+
// the following header and module map were copied from test/SourceKit/DocSupport/Inputs
28+
//--- MyError/module.modulemap
29+
module "MyError" {
30+
header "MyError.h"
31+
export *
32+
}
33+
34+
//--- MyError/MyError.h
35+
@import Foundation;
36+
37+
#define NS_ERROR_ENUM(_type, _name, _domain) \
38+
enum _name : _type _name; enum __attribute__((ns_error_domain(_domain))) _name : _type
39+
40+
@class NSString;
41+
extern const NSString *const MyErrorDomain;
42+
/// This is my cool error code.
43+
typedef NS_ERROR_ENUM(int, MyErrorCode, MyErrorDomain) {
44+
/// This is first error.
45+
MyErrFirst,
46+
/// This is second error.
47+
MyErrSecond,
48+
};

‎test/SymbolGraph/Symbols/SkipProtocolImplementations.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,9 @@
3636
// There should only be three symbols with sourceOrigin information:
3737
// - SomeStruct.otherFunc()
3838
// - ExtraStruct.Inner
39-
// - HiddenStruct.T
4039
// (ExtraStruct.Inner will have two sourceOrigins because it has two relationships: a memberOf and a
4140
// conformsTo)
42-
// COUNT-COUNT-4: sourceOrigin
41+
// COUNT-COUNT-3: sourceOrigin
4342
// COUNT-NOT: sourceOrigin
4443

4544
public protocol SomeProtocol {

‎test/SymbolGraph/Symbols/UnderscoredProtocols.swift

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,40 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-build-swift %s -module-name UnderscoredProtocols -emit-module -emit-module-path %t/
3+
34
// RUN: %target-swift-symbolgraph-extract -module-name UnderscoredProtocols -I %t -pretty-print -output-dir %t
5+
// RUN: %FileCheck %s --input-file %t/UnderscoredProtocols.symbols.json
6+
7+
// RUN: %target-swift-symbolgraph-extract -module-name UnderscoredProtocols -I %t -skip-protocol-implementations -pretty-print -output-dir %t
8+
// RUN: %FileCheck %s --input-file %t/UnderscoredProtocols.symbols.json
49

510
// Make sure that underscored protocols do not appear in public symbol graphs, but their
611
// requirements do appear on types which conform to them.
712

13+
// Also ensure that default implementations of underscored protocols appear on implementing types as
14+
// if they were native children of that type. No 'sourceOrigin' information should appear in their
15+
// relationships and no implementation relationships should exist.
16+
17+
// CHECK-DAG: "precise": "s:20UnderscoredProtocols10SomeStructV8someFuncyyF",
18+
// CHECK-DAG: "precise": "s:20UnderscoredProtocols15_HiddenProtocolPAAE9bonusFuncyyF::SYNTHESIZED::s:20UnderscoredProtocols10SomeStructV"
19+
820
// CHECK-NOT: "precise": "s:20UnderscoredProtocols15_HiddenProtocolP",
9-
// CHECK: "precise": "s:20UnderscoredProtocols10SomeStructV8someFuncyyF",
21+
22+
// CHECK-NOT: "defaultImplementationOf"
23+
// CHECK-NOT: "requirementOf"
24+
// CHECK-NOT: "optionalRequirementOf"
25+
// CHECK-NOT: "overrides"
26+
27+
// CHECK-NOT: "sourceOrigin"
28+
// CHECK-NOT: "s:20UnderscoredProtocols15_HiddenProtocolPAAE9bonusFuncyyF"
1029

1130
public protocol _HiddenProtocol {
1231
func someFunc()
1332
}
1433

34+
extension _HiddenProtocol {
35+
public func bonusFunc() {}
36+
}
37+
1538
public struct SomeStruct {}
1639

1740
extension SomeStruct: _HiddenProtocol {

0 commit comments

Comments
 (0)