Skip to content

Commit 7b45ae9

Browse files
committed
swift-module-digester: diagnose adding/removing @escaping as ABI breakage.
1 parent 6665b56 commit 7b45ae9

File tree

8 files changed

+91
-47
lines changed

8 files changed

+91
-47
lines changed

include/swift/AST/DiagnosticsModuleDiffer.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ ERROR(var_let_changed,none,"%0 changes from %select{var|let}1 to %select{let|var
8080

8181
ERROR(no_longer_open,none,"%0 is no longer open for subclassing", (StringRef))
8282

83+
ERROR(func_type_escaping_changed,none,"%0 has %select{removed|added}2 @escaping in %1", (StringRef, StringRef, bool))
84+
8385
#ifndef DIAG_NO_UNDEF
8486
# if defined(DIAG)
8587
# undef DIAG

test/api-digester/Inputs/cake1.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,3 +120,8 @@ public class ClassWithOpenMember {
120120
open var property: Int {get { return 1}}
121121
open func bar() {}
122122
}
123+
124+
public class EscapingFunctionType {
125+
public func removedEscaping(_ a: @escaping ()->()) {}
126+
public func addedEscaping(_ a: ()->()) {}
127+
}

test/api-digester/Inputs/cake2.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,8 @@ public class ClassWithOpenMember {
130130
public var property: Int {get { return 1}}
131131
public func bar() {}
132132
}
133+
134+
public class EscapingFunctionType {
135+
public func removedEscaping(_ a: ()->()) {}
136+
public func addedEscaping(_ a: @escaping ()->()) {}
137+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ cake1: Constructor S1.init(_:) has parameter 0 type change from Int to Double
3131
cake1: Func C1.foo2(_:) has parameter 0 type change from Int to () -> ()
3232
cake1: Func C7.foo(_:_:) has removed default argument from parameter 0
3333
cake1: Func C7.foo(_:_:) has removed default argument from parameter 1
34+
cake1: Func EscapingFunctionType.addedEscaping(_:) has added @escaping in parameter 0
35+
cake1: Func EscapingFunctionType.removedEscaping(_:) has removed @escaping in parameter 0
3436
cake1: Func Somestruct2.foo1(_:) has parameter 0 type change from C3 to C1
3537

3638
/* Decl Attribute changes */

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -352,13 +352,19 @@ ArrayRef<DeclAttrKind> SDKNodeDecl::getDeclAttributes() const {
352352
}
353353

354354
bool SDKNodeDecl::hasAttributeChange(const SDKNodeDecl &Another) const {
355-
if (getDeclAttributes().size() != Another.getDeclAttributes().size())
356-
return true;
357-
for (auto K: getDeclAttributes()) {
358-
if (!Another.hasDeclAttribute(K))
359-
return true;
360-
}
361-
return false;
355+
std::set<DeclAttrKind> Left(getDeclAttributes().begin(),
356+
getDeclAttributes().end());
357+
std::set<DeclAttrKind> Right(Another.getDeclAttributes().begin(),
358+
Another.getDeclAttributes().end());
359+
return Left != Right;
360+
}
361+
362+
bool SDKNodeType::hasAttributeChange(const SDKNodeType &Another) const {
363+
std::set<TypeAttrKind> Left(getTypeAttributes().begin(),
364+
getTypeAttributes().end());
365+
std::set<TypeAttrKind> Right(Another.getTypeAttributes().begin(),
366+
Another.getTypeAttributes().end());
367+
return Left != Right;
362368
}
363369

364370
SDKNodeDecl *SDKNodeType::getClosestParentDecl() const {
@@ -614,7 +620,7 @@ bool SDKNode::operator==(const SDKNode &Other) const {
614620
case SDKNodeKind::TypeFunc: {
615621
auto Left = this->getAs<SDKNodeType>();
616622
auto Right = (&Other)->getAs<SDKNodeType>();
617-
if (!Left->getTypeAttributes().equals(Right->getTypeAttributes()))
623+
if (Left->hasAttributeChange(*Right))
618624
return false;
619625
if (Left->hasDefaultArgument() != Right->hasDefaultArgument())
620626
return false;

tools/swift-api-digester/ModuleAnalyzerNodes.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,10 +348,10 @@ class SDKNodeType : public SDKNode {
348348
bool HasDefaultArg;
349349

350350
protected:
351-
bool hasTypeAttribute(TypeAttrKind DAKind) const;
352351
SDKNodeType(SDKNodeInitInfo Info, SDKNodeKind Kind);
353352
~SDKNodeType() = default;
354353
public:
354+
bool hasTypeAttribute(TypeAttrKind DAKind) const;
355355
KnownTypeKind getTypeKind() const;
356356
void addTypeAttribute(TypeAttrKind AttrKind);
357357
ArrayRef<TypeAttrKind> getTypeAttributes() const;
@@ -363,6 +363,7 @@ class SDKNodeType : public SDKNode {
363363
bool isTopLevelType() const { return !isa<SDKNodeType>(getParent()); }
364364
static bool classof(const SDKNode *N);
365365
virtual void jsonize(json::Output &Out) override;
366+
bool hasAttributeChange(const SDKNodeType &Another) const;
366367
};
367368

368369
class SDKNodeTypeNominal : public SDKNodeType {
@@ -378,7 +379,7 @@ class SDKNodeTypeNominal : public SDKNodeType {
378379
class SDKNodeTypeFunc : public SDKNodeType {
379380
public:
380381
SDKNodeTypeFunc(SDKNodeInitInfo Info);
381-
bool isEscaping() const { return !hasTypeAttribute(TypeAttrKind::TAK_noescape); }
382+
bool isEscaping() const { return hasTypeAttribute(TypeAttrKind::TAK_noescape); }
382383
static bool classof(const SDKNode *N);
383384
};
384385

tools/swift-api-digester/ModuleDiagsConsumer.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ static StringRef getCategoryName(uint32_t ID) {
4646
return "/* Decl Attribute changes */";
4747
case LocalDiagID::default_arg_removed:
4848
case LocalDiagID::decl_type_change:
49+
case LocalDiagID::func_type_escaping_changed:
4950
return "/* Type Changes */";
5051
case LocalDiagID::raw_type_change:
5152
return "/* RawRepresentable Changes */";

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

Lines changed: 59 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -811,14 +811,69 @@ static void diagnoseTypeChange(SDKNode* L, SDKNode* R) {
811811
auto *RT = dyn_cast<SDKNodeType>(R);
812812
if (!LT || !RT)
813813
return;
814+
assert(LT->isTopLevelType() == RT->isTopLevelType());
815+
if (!LT->isTopLevelType())
816+
return;
817+
StringRef Descriptor;
818+
auto LParent = cast<SDKNodeDecl>(LT->getParent());
819+
auto RParent = cast<SDKNodeDecl>(RT->getParent());
820+
assert(LParent->getKind() == RParent->getKind());
821+
if (LParent->isSDKPrivate())
822+
return;
823+
switch(LParent->getKind()) {
824+
case SDKNodeKind::Root:
825+
case SDKNodeKind::TypeNominal:
826+
case SDKNodeKind::TypeFunc:
827+
case SDKNodeKind::TypeAlias:
828+
case SDKNodeKind::DeclType:
829+
llvm_unreachable("Type Parent is wrong");
830+
case SDKNodeKind::DeclFunction:
831+
case SDKNodeKind::DeclConstructor:
832+
case SDKNodeKind::DeclGetter:
833+
case SDKNodeKind::DeclSetter:
834+
case SDKNodeKind::DeclSubscript:
835+
Descriptor = SDKNodeDeclAbstractFunc::
836+
getTypeRoleDescription(Ctx, LParent->getChildIndex(LT));
837+
break;
838+
case SDKNodeKind::DeclVar:
839+
Descriptor = "declared";
840+
break;
841+
case SDKNodeKind::DeclTypeAlias:
842+
Descriptor = "underlying";
843+
break;
844+
case SDKNodeKind::DeclAssociatedType:
845+
Descriptor = "default";
846+
break;
847+
}
848+
849+
if (LT->getPrintedName() != RT->getPrintedName()) {
850+
Diags.diagnose(SourceLoc(), diag::decl_type_change, LParent->getScreenInfo(),
851+
Descriptor, LT->getPrintedName(), RT->getPrintedName());
852+
return;
853+
}
854+
814855
if (LT->hasDefaultArgument() && !RT->hasDefaultArgument()) {
815-
auto *Func = cast<SDKNodeDeclAbstractFunc>(LT->getClosestParentDecl());
816-
Diags.diagnose(SourceLoc(), diag::default_arg_removed, Func->getScreenInfo(),
817-
Func->getTypeRoleDescription(Ctx, Func->getChildIndex(LT)));
856+
Diags.diagnose(SourceLoc(), diag::default_arg_removed,
857+
LParent->getScreenInfo(), Descriptor);
858+
}
859+
if (LT->getKind() != RT->getKind())
860+
return;
861+
assert(LT->getKind() == RT->getKind());
862+
switch (LT->getKind()) {
863+
case SDKNodeKind::TypeFunc: {
864+
auto *LFT = cast<SDKNodeTypeFunc>(LT);
865+
auto *RFT = cast<SDKNodeTypeFunc>(RT);
866+
if (Ctx.checkingABI() && LFT->isEscaping() != RFT->isEscaping()) {
867+
Diags.diagnose(SourceLoc(), diag::func_type_escaping_changed,
868+
LParent->getScreenInfo(), Descriptor, LFT->isEscaping());
869+
}
870+
break;
871+
}
872+
default:
873+
break;
818874
}
819875
}
820876

821-
822877
// This is first pass on two given SDKNode trees. This pass removes the common part
823878
// of two versions of SDK, leaving only the changed part.
824879
class PrunePass : public MatchedNodeListener, public SDKTreeDiffPass {
@@ -1656,7 +1711,6 @@ class DiffItemEmitter : public SDKNodeVisitor {
16561711

16571712
class DiagnosisEmitter : public SDKNodeVisitor {
16581713
void handle(const SDKNodeDecl *D, NodeAnnotation Anno);
1659-
void visitType(SDKNodeType *T);
16601714
void visitDecl(SDKNodeDecl *D);
16611715
void visit(NodePtr Node) override;
16621716
SDKNodeDecl *findAddedDecl(const SDKNodeDecl *Node);
@@ -1796,42 +1850,10 @@ void DiagnosisEmitter::visitDecl(SDKNodeDecl *Node) {
17961850
handle(Node, Anno);
17971851
}
17981852

1799-
void DiagnosisEmitter::visitType(SDKNodeType *Node) {
1800-
auto *Parent = dyn_cast<SDKNodeDecl>(Node->getParent());
1801-
if (!Parent || Parent->isSDKPrivate())
1802-
return;
1803-
SDKContext &Ctx = Node->getSDKContext();
1804-
if (Node->isAnnotatedAs(NodeAnnotation::Updated)) {
1805-
auto *Count = UpdateMap.findUpdateCounterpart(Node)->getAs<SDKNodeType>();
1806-
StringRef Descriptor;
1807-
switch (Parent->getKind()) {
1808-
case SDKNodeKind::DeclConstructor:
1809-
case SDKNodeKind::DeclFunction:
1810-
case SDKNodeKind::DeclVar:
1811-
Descriptor = isa<SDKNodeDeclAbstractFunc>(Parent) ?
1812-
SDKNodeDeclAbstractFunc::getTypeRoleDescription(Ctx, Parent->getChildIndex(Node)) :
1813-
Ctx.buffer("declared");
1814-
if (Node->getPrintedName() != Count->getPrintedName())
1815-
Diags.diagnose(SourceLoc(), diag::decl_type_change, Parent->getScreenInfo(),
1816-
Descriptor, Node->getPrintedName(), Count->getPrintedName());
1817-
break;
1818-
case SDKNodeKind::DeclAssociatedType:
1819-
Diags.diagnose(SourceLoc(), diag::decl_type_change, Parent->getScreenInfo(),
1820-
"default", Node->getPrintedName(), Count->getPrintedName());
1821-
break;
1822-
default:
1823-
break;
1824-
}
1825-
}
1826-
}
1827-
18281853
void DiagnosisEmitter::visit(NodePtr Node) {
18291854
if (auto *DNode = dyn_cast<SDKNodeDecl>(Node)) {
18301855
visitDecl(DNode);
18311856
}
1832-
if (auto *TNode = dyn_cast<SDKNodeType>(Node)) {
1833-
visitType(TNode);
1834-
}
18351857
}
18361858

18371859
typedef std::vector<NoEscapeFuncParam> NoEscapeFuncParamVector;

0 commit comments

Comments
 (0)