Skip to content

Commit 7bab30c

Browse files
committed
ABIChecker: diagnose moving a non-final member from a class to an extension to the class
rdar://93498817
1 parent 26952f7 commit 7bab30c

File tree

4 files changed

+76
-0
lines changed

4 files changed

+76
-0
lines changed

include/swift/APIDigester/ModuleAnalyzerNodes.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,7 @@ class SDKNodeDeclConstructor: public SDKNodeDeclAbstractFunc {
712712
void jsonize(json::Output &Out) override;
713713
};
714714

715+
// Note: Accessor doesn't have Parent pointer.
715716
class SDKNodeDeclAccessor: public SDKNodeDeclAbstractFunc {
716717
SDKNodeDecl *Owner;
717718
AccessorKind AccKind;
@@ -830,6 +831,8 @@ int findDeclUsr(StringRef dumpPath, CheckerOptions Opts);
830831

831832
void nodeSetDifference(ArrayRef<SDKNode*> Left, ArrayRef<SDKNode*> Right,
832833
NodeVector &LeftMinusRight, NodeVector &RightMinusLeft);
834+
835+
bool hasValidParentPtr(SDKNodeKind kind);
833836
} // end of abi namespace
834837
} // end of ide namespace
835838
} // end of Swift namespace

include/swift/AST/DiagnosticsModuleDiffer.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ ERROR(type_witness_change,APIDigesterBreakage,"%0 has type witness type for %1 c
7676

7777
ERROR(decl_new_witness_table_entry,APIDigesterBreakage,"%0 now requires%select{| no}1 new witness table entry", (StringRef, bool))
7878

79+
ERROR(class_member_moved_to_extension,APIDigesterBreakage,"Non-final class member %0 is moved to extension", (StringRef))
80+
7981
WARNING(new_decl_without_intro,APIDigesterBreakage,"%0 is a new API without @available attribute", (StringRef))
8082

8183
ERROR(objc_name_change,APIDigesterBreakage,"%0 has ObjC name change from %1 to %2", (StringRef, StringRef, StringRef))

lib/APIDigester/ModuleAnalyzerNodes.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ struct swift::ide::api::SDKNodeInitInfo {
5454
SDKNode* createSDKNode(SDKNodeKind Kind);
5555
};
5656

57+
bool swift::ide::api::hasValidParentPtr(SDKNodeKind kind) {
58+
switch(kind) {
59+
case SDKNodeKind::Conformance:
60+
case SDKNodeKind::DeclAccessor:
61+
return false;
62+
default:
63+
return true;
64+
}
65+
}
66+
5767
SDKContext::SDKContext(CheckerOptions Opts): Diags(SourceMgr), Opts(Opts) {}
5868

5969
DiagnosticEngine &SDKContext::getDiags(SourceLoc Loc) {
@@ -827,6 +837,25 @@ static bool hasSameParameterFlags(const SDKNodeType *Left, const SDKNodeType *Ri
827837
return true;
828838
}
829839

840+
// Return whether a decl has been moved in/out to an extension
841+
static Optional<bool> isFromExtensionChanged(const SDKNode &L, const SDKNode &R) {
842+
assert(L.getKind() == R.getKind());
843+
// Version 8 starts to include whether a decl is from an extension.
844+
if (L.getJsonFormatVersion() + R.getJsonFormatVersion() < 2 * 8) {
845+
return llvm::None;
846+
}
847+
auto *Left = dyn_cast<SDKNodeDecl>(&L);
848+
auto *Right = dyn_cast<SDKNodeDecl>(&R);
849+
if (!Left) {
850+
return llvm::None;
851+
}
852+
if (Left->isFromExtension() == Right->isFromExtension()) {
853+
return llvm::None;
854+
} else {
855+
return Right->isFromExtension();
856+
}
857+
}
858+
830859
static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R) {
831860
auto *LeftAlias = dyn_cast<SDKNodeTypeAlias>(&L);
832861
auto *RightAlias = dyn_cast<SDKNodeTypeAlias>(&R);
@@ -966,6 +995,8 @@ static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R)
966995
case SDKNodeKind::TypeWitness:
967996
case SDKNodeKind::DeclImport:
968997
case SDKNodeKind::Root: {
998+
if (isFromExtensionChanged(L, R))
999+
return false;
9691000
return L.getPrintedName() == R.getPrintedName() &&
9701001
L.hasSameChildren(R);
9711002
}
@@ -2621,6 +2652,23 @@ void swift::ide::api::SDKNodeDeclAbstractFunc::diagnose(SDKNode *Right) {
26212652
if (reqNewWitnessTableEntry() != R->reqNewWitnessTableEntry()) {
26222653
emitDiag(Loc, diag::decl_new_witness_table_entry, reqNewWitnessTableEntry());
26232654
}
2655+
2656+
// Diagnose moving a non-final class member to an extension.
2657+
if (hasValidParentPtr(getKind())) {
2658+
while(auto *parent = dyn_cast<SDKNodeDecl>(getParent())) {
2659+
if (parent->getDeclKind() != DeclKind::Class) {
2660+
break;
2661+
}
2662+
if (hasDeclAttribute(DeclAttrKind::DAK_Final)) {
2663+
break;
2664+
}
2665+
auto result = isFromExtensionChanged(*this, *Right);
2666+
if (result.hasValue() && *result) {
2667+
emitDiag(Loc, diag::class_member_moved_to_extension);
2668+
}
2669+
break;
2670+
}
2671+
}
26242672
}
26252673
}
26262674

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// RUN: %target-swift-frontend -emit-module -o %t/Foo.swiftmodule -emit-abi-descriptor-path %t/abi-before.json %s -enable-library-evolution -DBASELINE -emit-tbd-path %t/abi-before.tbd
4+
// RUN: %target-swift-frontend -emit-module -o %t/Foo.swiftmodule -emit-abi-descriptor-path %t/abi-after.json %s -enable-library-evolution -emit-tbd-path %t/abi-after.tbd
5+
// RUN: %api-digester -diagnose-sdk --input-paths %t/abi-before.json -input-paths %t/abi-after.json -abi -o %t/result.txt
6+
// RUN: %FileCheck %s < %t/result.txt
7+
8+
#if BASELINE
9+
10+
public class C {
11+
public func foo() {}
12+
}
13+
14+
#else
15+
16+
public class C {}
17+
extension C {
18+
public func foo() {}
19+
}
20+
21+
#endif
22+
23+
// CHECK: Non-final class member Func C.foo() is moved to extension

0 commit comments

Comments
 (0)