Skip to content

Commit 08c8cf1

Browse files
committed
swift-module-digester: diagnose adding/removing protocol conformances as API breakages.
1 parent 90a330b commit 08c8cf1

File tree

9 files changed

+94
-5
lines changed

9 files changed

+94
-5
lines changed

include/swift/AST/DiagnosticsModuleDiffer.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ ERROR(decl_added,none,"%0 is added to a non-resilient type", (StringRef))
5858

5959
ERROR(default_arg_removed,none,"%0 has removed default argument from %1", (StringRef, StringRef))
6060

61+
ERROR(conformance_removed,none,"%0 has removed %select{conformance to|inherited protocol}2 %1", (StringRef, StringRef, bool))
62+
63+
ERROR(conformance_added,none,"%0 has added inherited protocol %1", (StringRef, StringRef))
64+
6165
#ifndef DIAG_NO_UNDEF
6266
# if defined(DIAG)
6367
# undef DIAG

test/api-digester/Inputs/cake1.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,8 @@ public enum FrozenKind {
6464

6565
public class C7 {
6666
public func foo(_ a: Int = 1, _ b: Int = 2) {}
67-
}
67+
}
68+
69+
public protocol P3: P2, P1 {}
70+
71+
extension fixedLayoutStruct: P1 {}

test/api-digester/Inputs/cake2.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ public enum FrozenKind {
6767
case AddedCase
6868
}
6969

70-
public class C7 {
70+
public class C7: P1 {
7171
public func foo(_ a: Int, _ b: Int) {}
72-
}
72+
}
73+
74+
public protocol P3: P1, P4 {}
75+
76+
public protocol P4 {}
77+
78+
extension fixedLayoutStruct: P2 {}

test/api-digester/Outputs/Cake-abi.txt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11

22
/* Generic Signature Changes */
33
cake1: Func P1.P1Constraint() has generic signature change from <τ_0_0 where τ_0_0 : P1, τ_0_0 : P2> to <τ_0_0 where τ_0_0 : P1>
4+
cake1: Protocol P3 has generic signature change from <τ_0_0 : P1, τ_0_0 : P2> to <τ_0_0 : P1, τ_0_0 : P4>
45

56
/* RawRepresentable Changes */
67

@@ -40,4 +41,9 @@ cake1: Var fixedLayoutStruct.a in a non-resilient type changes position from 1 t
4041
cake1: Var fixedLayoutStruct.b in a non-resilient type changes position from 0 to 1
4142
cake2: EnumElement FrozenKind.AddedCase is added to a non-resilient type
4243
cake2: Var fixedLayoutStruct.c is added to a non-resilient type
43-
cake2: Var fixedLayoutStruct.lazy_d.storage is added to a non-resilient type
44+
cake2: Var fixedLayoutStruct.lazy_d.storage is added to a non-resilient type
45+
46+
/* Conformance changes */
47+
cake1: Protocol P3 has added inherited protocol P4
48+
cake1: Protocol P3 has removed inherited protocol P2
49+
cake1: Struct fixedLayoutStruct has removed conformance to P1

test/api-digester/Outputs/Cake.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11

22
/* Generic Signature Changes */
33
cake1: Func P1.P1Constraint() has generic signature change from <Self where Self : P1, Self : P2> to <Self where Self : P1>
4+
cake1: Protocol P3 has generic signature change from <Self : P1, Self : P2> to <Self : P1, Self : P4>
45

56
/* RawRepresentable Changes */
67

@@ -27,3 +28,8 @@ cake1: Func S1.foo1() is now mutating
2728
cake1: Func S1.foo3() is now static
2829
cake1: Var C1.CIIns1 changes from weak to strong
2930
cake1: Var C1.CIIns2 changes from strong to weak
31+
32+
/* Conformance changes */
33+
cake1: Protocol P3 has added inherited protocol P4
34+
cake1: Protocol P3 has removed inherited protocol P2
35+
cake1: Struct fixedLayoutStruct has removed conformance to P1

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <ModuleAnalyzerNodes.h>
2+
#include <algorithm>
23

34
using namespace swift;
45
using namespace ide;
@@ -642,6 +643,25 @@ bool SDKNode::hasSameChildren(const SDKNode &Other) const {
642643
return true;
643644
}
644645

646+
void swift::ide::api::stringSetDifference(ArrayRef<StringRef> Left,
647+
ArrayRef<StringRef> Right,
648+
std::vector<StringRef> &LeftMinusRight,
649+
std::vector<StringRef> &RightMinusLeft) {
650+
std::set<StringRef> LS(Left.begin(), Left.end());
651+
std::set<StringRef> RS(Right.begin(), Right.end());
652+
std::set_difference(LS.begin(), LS.end(), RS.begin(), RS.end(),
653+
std::back_inserter(LeftMinusRight));
654+
std::set_difference(RS.begin(), RS.end(), LS.begin(), LS.end(),
655+
std::back_inserter(RightMinusLeft));
656+
}
657+
658+
static bool hasSameContents(ArrayRef<StringRef> Left,
659+
ArrayRef<StringRef> Right) {
660+
std::vector<StringRef> LeftMinusRight, RightMinusLeft;
661+
stringSetDifference(Left, Right, LeftMinusRight, RightMinusLeft);
662+
return LeftMinusRight.empty() && RightMinusLeft.empty();
663+
}
664+
645665
bool SDKNode::operator==(const SDKNode &Other) const {
646666
auto *LeftAlias = dyn_cast<SDKNodeTypeAlias>(this);
647667
auto *RightAlias = dyn_cast<SDKNodeTypeAlias>(&Other);
@@ -701,7 +721,16 @@ bool SDKNode::operator==(const SDKNode &Other) const {
701721
}
702722
LLVM_FALLTHROUGH;
703723
}
704-
case SDKNodeKind::DeclType:
724+
case SDKNodeKind::DeclType: {
725+
auto *Left = dyn_cast<SDKNodeDeclType>(this);
726+
auto *Right = dyn_cast<SDKNodeDeclType>(&Other);
727+
if (Left && Right) {
728+
if (!hasSameContents(Left->getAllProtocols(), Right->getAllProtocols())) {
729+
return false;
730+
}
731+
}
732+
LLVM_FALLTHROUGH;
733+
}
705734
case SDKNodeKind::DeclTypeAlias: {
706735
auto Left = this->getAs<SDKNodeDecl>();
707736
auto Right = (&Other)->getAs<SDKNodeDecl>();

tools/swift-api-digester/ModuleAnalyzerNodes.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,9 @@ int deserializeSDKDump(StringRef dumpPath, StringRef OutputPath,
543543

544544
int findDeclUsr(StringRef dumpPath, CheckerOptions Opts);
545545

546+
void stringSetDifference(ArrayRef<StringRef> Left, ArrayRef<StringRef> Right,
547+
std::vector<StringRef> &LeftMinusRight, std::vector<StringRef> &RightMinusLeft);
548+
546549
} // end of abi namespace
547550
} // end of ide namespace
548551
} // end of Swift namespace

tools/swift-api-digester/ModuleDiagsConsumer.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ static StringRef getCategoryName(uint32_t ID) {
5151
case LocalDiagID::decl_added:
5252
case LocalDiagID::decl_reorder:
5353
return "/* Fixed-layout Type Changes */";
54+
case LocalDiagID::conformance_added:
55+
case LocalDiagID::conformance_removed:
56+
return "/* Protocol Conformance Change */";
5457
default:
5558
return StringRef();
5659
}

tools/swift-api-digester/swift-api-digester.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,28 @@ static bool isOwnershipEquivalent(ReferenceOwnership Left,
674674
return false;
675675
}
676676

677+
static void diagnoseNominalTypeDeclChange(SDKNodeDeclType *L, SDKNodeDeclType *R) {
678+
auto &Ctx = L->getSDKContext();
679+
auto &Diags = Ctx.getDiags();
680+
std::vector<StringRef> LeftMinusRight;
681+
std::vector<StringRef> RightMinusLeft;
682+
swift::ide::api::stringSetDifference(L->getAllProtocols(), R->getAllProtocols(),
683+
LeftMinusRight, RightMinusLeft);
684+
bool isProtocol = L->getDeclKind() == DeclKind::Protocol;
685+
std::for_each(LeftMinusRight.begin(), LeftMinusRight.end(), [&](StringRef Name) {
686+
Diags.diagnose(SourceLoc(), diag::conformance_removed, L->getScreenInfo(), Name,
687+
isProtocol);
688+
});
689+
690+
// Adding inherited protocols can be API breaking.
691+
if (isProtocol) {
692+
std::for_each(RightMinusLeft.begin(), RightMinusLeft.end(), [&](StringRef Name) {
693+
Diags.diagnose(SourceLoc(), diag::conformance_added, L->getScreenInfo(),
694+
Name);
695+
});
696+
}
697+
}
698+
677699
static void detectDeclChange(NodePtr L, NodePtr R, SDKContext &Ctx) {
678700
assert(L->getKind() == R->getKind());
679701
auto &Diags = Ctx.getDiags();
@@ -727,6 +749,12 @@ static void detectDeclChange(NodePtr L, NodePtr R, SDKContext &Ctx) {
727749
Diags.diagnose(SourceLoc(), diag::generic_sig_change, LD->getScreenInfo(),
728750
LD->getGenericSignature(), RD->getGenericSignature());
729751
}
752+
753+
if (auto *LDT = dyn_cast<SDKNodeDeclType>(L)) {
754+
if (auto *RDT = dyn_cast<SDKNodeDeclType>(R)) {
755+
diagnoseNominalTypeDeclChange(LDT, RDT);
756+
}
757+
}
730758
detectRename(L, R);
731759
}
732760
}

0 commit comments

Comments
 (0)