Skip to content

[4.2] migrator: handle member variables change to global ones. rdar://41658300 #17672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions include/swift/IDE/APIDigesterData.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,10 @@ struct TypeMemberDiffItem: public APIDiffItem {
Optional<uint8_t> removedIndex;
StringRef oldTypeName;
StringRef oldPrintedName;

private:
DeclNameViewer OldNameViewer;
DeclNameViewer NewNameViewer;

std::string NewTypeDot;
public:
TypeMemberDiffItemSubKind Subkind;

Expand All @@ -276,7 +275,9 @@ struct TypeMemberDiffItem: public APIDiffItem {
newTypeName(newTypeName), newPrintedName(newPrintedName),
selfIndex(selfIndex), removedIndex(removedIndex), oldTypeName(oldTypeName),
oldPrintedName(oldPrintedName), OldNameViewer(oldPrintedName),
NewNameViewer(newPrintedName), Subkind(getSubKind()) {}
NewNameViewer(newPrintedName),
NewTypeDot(isNewNameGlobal() ? "" : (llvm::Twine(newTypeName) + ".").str()),
Subkind(getSubKind()) {}
static StringRef head();
static void describe(llvm::raw_ostream &os);
static void undef(llvm::raw_ostream &os);
Expand All @@ -286,9 +287,11 @@ struct TypeMemberDiffItem: public APIDiffItem {
StringRef getKey() const override { return usr; }
const DeclNameViewer &getOldName() const { return OldNameViewer; }
const DeclNameViewer &getNewName() const { return NewNameViewer; }
StringRef getNewTypeAndDot() const { return NewTypeDot; }
APIDiffItemKind getKind() const override {
return APIDiffItemKind::ADK_TypeMemberDiffItem;
}
bool isNewNameGlobal() const { return newTypeName.empty(); }
private:
TypeMemberDiffItemSubKind getSubKind() const;
};
Expand Down
27 changes: 19 additions & 8 deletions lib/Migrator/APIDiffMigratorPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
if (auto *MI = dyn_cast<TypeMemberDiffItem>(Item)) {
if (MI->Subkind == TypeMemberDiffItemSubKind::FuncRename) {
llvm::raw_svector_ostream OS(Buffer);
OS << MI->newTypeName << "." << MI->newPrintedName;
OS << MI->getNewTypeAndDot() << MI->newPrintedName;
return DeclNameViewer(OS.str());
}
}
Expand All @@ -343,8 +343,13 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
if (MD->Subkind == TypeMemberDiffItemSubKind::SimpleReplacement) {
bool NeedNoTypeName = isDotMember &&
MD->oldPrintedName == MD->newPrintedName;
Text = (llvm::Twine(NeedNoTypeName ? "" : MD->newTypeName) + "." +
MD->getNewName().base()).str();
if (NeedNoTypeName) {
Text = (llvm::Twine(MD->isNewNameGlobal() ? "" : ".") +
MD->getNewName().base()).str();
} else {
Text = (llvm::Twine(MD->getNewTypeAndDot()) +
MD->getNewName().base()).str();
}
return true;
}
}
Expand Down Expand Up @@ -540,9 +545,15 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
if (Item->Subkind == TypeMemberDiffItemSubKind::QualifiedReplacement) {
bool NeedNoTypeName = isDotMember(ToReplace) &&
Item->oldPrintedName == Item->newPrintedName;
Editor.replace(ToReplace,
(llvm::Twine(NeedNoTypeName ? "" : Item->newTypeName) + "." +
Item->getNewName().base()).str());
if (NeedNoTypeName) {
Editor.replace(ToReplace,
(llvm::Twine(Item->isNewNameGlobal() ? "" : ".") +
Item->getNewName().base()).str());
} else {
Editor.replace(ToReplace,
(llvm::Twine(Item->getNewTypeAndDot()) +
Item->getNewName().base()).str());
}
return true;
}
}
Expand Down Expand Up @@ -710,8 +721,8 @@ struct APIDiffMigratorPass : public ASTMigratorPass, public SourceEntityWalker {
return false;

if (Item->Subkind == TypeMemberDiffItemSubKind::GlobalFuncToStaticProperty) {
Editor.replace(Call->getSourceRange(), (llvm::Twine(Item->newTypeName) +
"." + Item->getNewName().base()).str());
Editor.replace(Call->getSourceRange(),
(llvm::Twine(Item->getNewTypeAndDot()) + Item->getNewName().base()).str());
return true;
}
if (*Item->selfIndex)
Expand Down
1 change: 1 addition & 0 deletions test/Migrator/Inputs/Cities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ open class Cities {
open func maroochy(x: Int?, y: Int?) {}
public struct CityKind {
public static let Town = 1
public static let Village = 1
}
}

Expand Down
16 changes: 16 additions & 0 deletions test/Migrator/Inputs/qualified.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@
"NewPrintedName": "NewTown",
"NewTypeName": "NewCityKind"
},
{
"DiffItemKind": "TypeMemberDiffItem",
"Usr": "s:6CitiesAAC8CityKindV7VillageSivpZ",
"OldPrintedName": "Village",
"OldTypeName": "Cities.CityKind",
"NewPrintedName": "GlobalCityKindVillage",
"NewTypeName": ""
},
{
"DiffItemKind": "TypeMemberDiffItem",
"Usr": "s:6Cities12ToplevelTypeC",
Expand All @@ -47,4 +55,12 @@
"NewPrintedName": "orderedMemberSame",
"NewTypeName": "NewFooComparisonResult.Nested"
},
{
"DiffItemKind": "TypeMemberDiffItem",
"Usr": "c:@E@FooComparisonResult@FooOrderedMovedToGlobal",
"OldPrintedName": "orderedMovedToGlobal",
"OldTypeName": "FooComparisonResult",
"NewPrintedName": "orderedMovedToGlobal",
"NewTypeName": ""
},
]
1 change: 1 addition & 0 deletions test/Migrator/mock-sdk/Bar.framework/Headers/Bar.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ typedef NS_ENUM(long, FooComparisonResult) {
FooOrderedSame,
FooOrderedDescending,
FooOrderedMemberSame,
FooOrderedMovedToGlobal,
};

@interface BarBase
Expand Down
3 changes: 3 additions & 0 deletions test/Migrator/qualified-replacement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ func foo() {
_ = FooComparisonResult.orderedSame
let _ : FooComparisonResult = .orderedSame
_ = Cities.CityKind.Town
_ = Cities.CityKind.Village
_ = ToplevelType()
_ = ToplevelType(recordName: "")
bar(.orderedSame)
bar(.orderedMemberSame)
bar(FooComparisonResult.orderedSame)
bar(FooComparisonResult.orderedMemberSame)
bar(FooComparisonResult.orderedMovedToGlobal)
bar(.orderedMovedToGlobal)
}

func foo(_: ToplevelType) {}
Expand Down
3 changes: 3 additions & 0 deletions test/Migrator/qualified-replacement.swift.expected
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ func foo() {
_ = NewFooComparisonResult.NewFooOrderedSame
let _ : FooComparisonResult = NewFooComparisonResult.NewFooOrderedSame
_ = NewCityKind.NewTown
_ = GlobalCityKindVillage
_ = ToplevelWrapper.internalType()
_ = ToplevelWrapper.internalType(recordName: "")
bar(NewFooComparisonResult.NewFooOrderedSame)
bar(.orderedMemberSame)
bar(NewFooComparisonResult.NewFooOrderedSame)
bar(NewFooComparisonResult.Nested.orderedMemberSame)
bar(orderedMovedToGlobal)
bar(orderedMovedToGlobal)
}

func foo(_: ToplevelWrapper.internalType) {}
Expand Down
19 changes: 19 additions & 0 deletions test/api-digester/Outputs/apinotes-migrator-gen-revert.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
[
{
"DiffItemKind": "CommonDiffItem",
"NodeKind": "Var",
"NodeAnnotation": "RevertSimpleStringRepresentableUpdate",
"ChildIndex": "0",
"LeftUsr": "c:@globalAttributeName",
"LeftComment": "AnimalAttributeName",
"RightUsr": "",
"RightComment": "String",
"ModuleName": "APINotesTest"
},
{
"DiffItemKind": "CommonDiffItem",
"NodeKind": "TypeDecl",
Expand Down Expand Up @@ -161,6 +172,14 @@
"NewPrintedName": "oldMember",
"NewTypeName": "OldType"
},
{
"DiffItemKind": "TypeMemberDiffItem",
"Usr": "c:@globalAttributeName",
"OldPrintedName": "globalAttributeName",
"OldTypeName": "AnimalAttributeName",
"NewPrintedName": "globalAttributeName",
"NewTypeName": ""
},
{
"DiffItemKind": "TypeMemberDiffItem",
"Usr": "c:objc(pl)TypeWithMethod(cm)plusPrint",
Expand Down
9 changes: 8 additions & 1 deletion tools/swift-api-digester/swift-api-digester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2544,6 +2544,12 @@ class TypeMemberDiffFinder : public SDKNodeVisitor {
if (nodeParent->getKind() == SDKNodeKind::DeclType &&
diffParent->getKind() == SDKNodeKind::Root)
TypeMemberDiffs.insert({diffNode, node});

// Move from a member variable to global variable.
if (nodeParent->getKind() == SDKNodeKind::Root &&
diffParent->getKind() == SDKNodeKind::DeclType)
TypeMemberDiffs.insert({diffNode, node});

// Move from a member variable to another member variable
if (nodeParent->getKind() == SDKNodeKind::DeclType &&
diffParent->getKind() == SDKNodeKind::DeclType &&
Expand Down Expand Up @@ -3758,7 +3764,8 @@ static void findTypeMemberDiffs(NodePtr leftSDKRoot, NodePtr rightSDKRoot,
// index, old printed name)
TypeMemberDiffItem item = {
right->getAs<SDKNodeDecl>()->getUsr(),
rightParent->getAs<SDKNodeDecl>()->getFullyQualifiedName(),
rightParent->getKind() == SDKNodeKind::Root ?
StringRef() : rightParent->getAs<SDKNodeDecl>()->getFullyQualifiedName(),
right->getPrintedName(), findSelfIndex(right), None,
leftParent->getKind() == SDKNodeKind::Root ?
StringRef() : leftParent->getAs<SDKNodeDecl>()->getFullyQualifiedName(),
Expand Down