Skip to content

Commit 824aaa1

Browse files
committed
swift-api-digester: detect the move of static members only.
This patch restricts the detection of moved members to be static members, since only in this case we need to update qualified access to them. The move of instance members will be either handled by rename or we don't need to update anything at all. Additionally, this patch introduces a sub-kind of type member diff item called qualified replacement to describe the aforementioned case. However, the migrator part has not started to honor this sub-kind yet. rdar://32466196
1 parent 62784f5 commit 824aaa1

File tree

9 files changed

+60
-28
lines changed

9 files changed

+60
-28
lines changed

include/swift/IDE/APIDigesterData.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ struct CommonDiffItem: public APIDiffItem {
219219
//
220220
enum class TypeMemberDiffItemSubKind {
221221
SimpleReplacement,
222+
QualifiedReplacement,
222223
GlobalFuncToStaticProperty,
223224
HoistSelfOnly,
224225
HoistSelfAndRemoveParam,

lib/IDE/APIDigesterData.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,12 @@ swift::ide::api::TypeMemberDiffItem::getSubKind() const {
134134
} else if (ToProperty) {
135135
assert(OldName.argSize() == 1);
136136
return TypeMemberDiffItemSubKind::HoistSelfAndUseProperty;
137-
} else {
137+
} else if (oldTypeName.empty()) {
138138
assert(NewName.argSize() + 1 == OldName.argSize());
139139
return TypeMemberDiffItemSubKind::HoistSelfOnly;
140+
} else {
141+
assert(NewName.argSize() == OldName.argSize());
142+
return TypeMemberDiffItemSubKind::QualifiedReplacement;
140143
}
141144
} else if (ToProperty) {
142145
assert(OldName.argSize() == 0);

lib/Migrator/APIDiffMigratorPass.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
439439
}
440440
if (!Item)
441441
return false;
442-
if (Item->Subkind == TypeMemberDiffItemSubKind::SimpleReplacement)
442+
if (Item->Subkind == TypeMemberDiffItemSubKind::SimpleReplacement ||
443+
Item->Subkind == TypeMemberDiffItemSubKind::QualifiedReplacement)
443444
return false;
444445

445446
if (Item->Subkind == TypeMemberDiffItemSubKind::GlobalFuncToStaticProperty) {
@@ -490,6 +491,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
490491
switch (Item->Subkind) {
491492
case TypeMemberDiffItemSubKind::GlobalFuncToStaticProperty:
492493
case TypeMemberDiffItemSubKind::SimpleReplacement:
494+
case TypeMemberDiffItemSubKind::QualifiedReplacement:
493495
llvm_unreachable("should be handled elsewhere");
494496
case TypeMemberDiffItemSubKind::HoistSelfOnly:
495497
// we are done here.

test/api-digester/Inputs/APINotesLeft/APINotesTest.apinotes

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,6 @@ Name: APINotesTest
22
Globals:
33
- Name: ANTGlobalValue
44
SwiftName: OldType.oldMember
5+
Protocols:
6+
- Name: TypeWithMethod
7+
SwiftName: SwiftTypeWithMethodLeft

test/api-digester/Inputs/APINotesLeft/APINotesTest.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,8 @@ extern int ANTGlobalValue;
44
@end
55
@interface OldType
66
@end
7+
8+
@protocol TypeWithMethod
9+
-(void) minusPrint;
10+
+(void) plusPrint;
11+
@end

test/api-digester/Inputs/APINotesRight/APINotesTest.apinotes

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,6 @@ Name: APINotesTest
22
Globals:
33
- Name: ANTGlobalValue
44
SwiftName: NewType.newMember
5+
Protocols:
6+
- Name: TypeWithMethod
7+
SwiftName: SwiftTypeWithMethodRight

test/api-digester/Inputs/APINotesRight/APINotesTest.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,9 @@ extern int ANTGlobalValue;
33
@interface NewType
44
@end
55
@interface OldType
6-
@end
6+
@end
7+
8+
@protocol TypeWithMethod
9+
-(void) minusPrint;
10+
+(void) plusPrint;
11+
@end

test/api-digester/Outputs/apinotes-migrator-gen.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,13 @@
66
"OldTypeName": "OldType",
77
"NewPrintedName": "newMember",
88
"NewTypeName": "NewType"
9+
},
10+
{
11+
"DiffItemKind": "TypeMemberDiffItem",
12+
"Usr": "c:objc(pl)TypeWithMethod(cm)plusPrint",
13+
"OldPrintedName": "plusPrint()",
14+
"OldTypeName": "SwiftTypeWithMethodLeft",
15+
"NewPrintedName": "plusPrint()",
16+
"NewTypeName": "SwiftTypeWithMethodRight"
917
}
1018
]

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

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,7 +1937,7 @@ class SDKTreeDiffPass {
19371937
virtual ~SDKTreeDiffPass() {}
19381938
};
19391939

1940-
using NodePairVector = std::vector<std::pair<NodePtr, NodePtr>>;
1940+
using NodePairVector = llvm::MapVector<NodePtr, NodePtr>;
19411941

19421942
// This map keeps track of updated nodes; thus we can conveniently find out what
19431943
// is the counterpart of a node before or after being updated.
@@ -1947,7 +1947,7 @@ class UpdatedNodesMap : public MatchedNodeListener {
19471947
public:
19481948
void foundMatch(NodePtr Left, NodePtr Right) override {
19491949
assert(Left && Right && "Not update operation.");
1950-
MapImpl.push_back(std::make_pair(Left, Right));
1950+
MapImpl.insert({Left, Right});
19511951
}
19521952

19531953
NodePtr findUpdateCounterpart(const SDKNode *Node) const {
@@ -2133,6 +2133,26 @@ class MapUSRToNode : public SDKNodeVisitor {
21332133
MapUSRToNode &operator=(MapUSRToNode &) = delete;
21342134
};
21352135

2136+
static StringRef constructFullTypeName(NodePtr Node) {
2137+
assert(Node->getKind() == SDKNodeKind::TypeDecl);
2138+
std::vector<NodePtr> TypeChain;
2139+
for (auto C = Node; C->getKind() == SDKNodeKind::TypeDecl; C = C->getParent()) {
2140+
TypeChain.insert(TypeChain.begin(), C);
2141+
}
2142+
assert(TypeChain.front()->getParent()->getKind() == SDKNodeKind::Root);
2143+
llvm::SmallString<64> Buffer;
2144+
bool First = true;
2145+
for (auto N : TypeChain) {
2146+
if (First) {
2147+
First = false;
2148+
} else {
2149+
Buffer.append(".");
2150+
}
2151+
Buffer.append(N->getName());
2152+
}
2153+
return Node->getSDKContext().buffer(Buffer.str());
2154+
}
2155+
21362156
// Class to build up a diff of structurally different nodes, based on the given
21372157
// USR map for the left (original) side of the diff, based on parent types.
21382158
class TypeMemberDiffFinder : public SDKNodeVisitor {
@@ -2162,11 +2182,13 @@ class TypeMemberDiffFinder : public SDKNodeVisitor {
21622182
// Move from global variable to a member variable.
21632183
if (nodeParent->getKind() == SDKNodeKind::TypeDecl &&
21642184
diffParent->getKind() == SDKNodeKind::Root)
2165-
TypeMemberDiffs.push_back({diffNode, node});
2185+
TypeMemberDiffs.insert({diffNode, node});
21662186
// Move from a member variable to another member variable
21672187
if (nodeParent->getKind() == SDKNodeKind::TypeDecl &&
2168-
diffParent->getKind() == SDKNodeKind::TypeDecl)
2169-
TypeMemberDiffs.push_back({diffNode, node});
2188+
diffParent->getKind() == SDKNodeKind::TypeDecl &&
2189+
declNode->isStatic() &&
2190+
constructFullTypeName(nodeParent) != constructFullTypeName(diffParent))
2191+
TypeMemberDiffs.insert({diffNode, node});
21702192
// Move from a getter/setter function to a property
21712193
else if (node->getKind() == SDKNodeKind::Getter &&
21722194
diffNode->getKind() == SDKNodeKind::Function &&
@@ -2943,26 +2965,6 @@ class OverloadMemberFunctionEmitter : public SDKNodeVisitor {
29432965
namespace fs = llvm::sys::fs;
29442966
namespace path = llvm::sys::path;
29452967

2946-
static StringRef constructFullTypeName(NodePtr Node) {
2947-
assert(Node->getKind() == SDKNodeKind::TypeDecl);
2948-
std::vector<NodePtr> TypeChain;
2949-
for (auto C = Node; C->getKind() == SDKNodeKind::TypeDecl; C = C->getParent()) {
2950-
TypeChain.insert(TypeChain.begin(), C);
2951-
}
2952-
assert(TypeChain.front()->getParent()->getKind() == SDKNodeKind::Root);
2953-
llvm::SmallString<64> Buffer;
2954-
bool First = true;
2955-
for (auto N : TypeChain) {
2956-
if (First) {
2957-
First = false;
2958-
} else {
2959-
Buffer.append(".");
2960-
}
2961-
Buffer.append(N->getName());
2962-
}
2963-
return Node->getSDKContext().buffer(Buffer.str());
2964-
}
2965-
29662968
struct RenameDetectorForMemberDiff : public MatchedNodeListener {
29672969
void foundMatch(NodePtr Left, NodePtr Right) override {
29682970
detectRename(Left, Right);

0 commit comments

Comments
 (0)