Skip to content

[Migrator] Remove some now unnecessary tuple destructuring #10432

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 1 commit into from
Jun 21, 2017
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
153 changes: 0 additions & 153 deletions lib/Migrator/TupleSplatMigratorPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,161 +176,8 @@ struct TupleSplatMigratorPass : public ASTMigratorPass,
return true;
};

// Handles such kind of cases:
// \code
// func test(_: ((Int, Int)) -> ()) {}
// test({ (x,y) in })
// \endcode
// This compiles fine in Swift 3 but Swift 4 complains with
// error: cannot convert value of type '(_, _) -> ()' to expected
// argument type '((Int, Int)) -> ()'
//
// It will fix the code to "test({ let (x,y) = $0; })".
//
auto handleTupleMapToClosureArgs = [&](const CallExpr *E) -> bool {
auto fnTy = E->getFn()->getType()->getAs<FunctionType>();
if (!fnTy)
return false;
auto fnTy2 = fnTy->getInput()->getAs<FunctionType>();
if (!fnTy2) {
// This may have been a tuple type of one element.
if (auto tuple = fnTy->getInput()->getAs<TupleType>()) {
if (tuple->getNumElements() == 1) {
fnTy2 = tuple->getElement(0).getType()->getAs<FunctionType>();
}
}
}
if (!fnTy2) {
return false;
}
auto parenT = dyn_cast<ParenType>(fnTy2->getInput().getPointer());
if (!parenT)
return false;
auto tupleInFn = parenT->getAs<TupleType>();
if (!tupleInFn)
return false;
if (!E->getArg())
return false;
auto argE = E->getArg()->getSemanticsProvidingExpr();
while (auto *ICE = dyn_cast<ImplicitConversionExpr>(argE))
argE = ICE->getSubExpr();
argE = argE->getSemanticsProvidingExpr();
auto closureE = dyn_cast<ClosureExpr>(argE);
if (!closureE) {
if (auto *FCE = dyn_cast<FunctionConversionExpr>(argE)) {
closureE = dyn_cast<ClosureExpr>(FCE->getSubExpr());
}
}
if (!closureE)
return false;
if (closureE->getInLoc().isInvalid())
return false;
auto paramList = closureE->getParameters();
if (!paramList ||
paramList->getLParenLoc().isInvalid() || paramList->getRParenLoc().isInvalid())
return false;
if (paramList->size() != tupleInFn->getNumElements())
return false;
if (paramList->size() == 0)
return false;

auto hasParamListWithNoTypes = [&]() {
if (closureE->hasExplicitResultType())
return false;
for (auto *param : *paramList) {
auto tyLoc = param->getTypeLoc();
if (!tyLoc.isNull())
return false;
}
return true;
};

if (hasParamListWithNoTypes()) {
// Simpler form depending on type inference.
// Change "(x, y) in " to "let (x, y) = $0;".

Editor.insert(paramList->getLParenLoc(), "let ");
for (auto *param : *paramList) {
// If the argument list is like "(_ x, _ y)", remove the underscores.
if (param->getArgumentNameLoc().isValid()) {
Editor.remove(CharSourceRange(SM, param->getArgumentNameLoc(),
param->getNameLoc()));
}
// If the argument list has type annotations, remove them.
auto tyLoc = param->getTypeLoc();
if (!tyLoc.isNull() && !tyLoc.getSourceRange().isInvalid()) {
auto nameRange = CharSourceRange(param->getNameLoc(),
param->getNameStr().size());
auto tyRange = Lexer::getCharSourceRangeFromSourceRange(SM,
tyLoc.getSourceRange());
Editor.remove(CharSourceRange(SM, nameRange.getEnd(),
tyRange.getEnd()));
}
}

// If the original closure was a single expression without the need
// for a `return` statement, it needs one now, because we've added a new
// assignment statement just above.
if (closureE->hasSingleExpressionBody()) {
Editor.replaceToken(closureE->getInLoc(), "= $0; return");
} else {
Editor.replaceToken(closureE->getInLoc(), "= $0;");
}

return true;
}

// Includes types in the closure signature. The following will do a
// more complicated edit than the above:
// (x: Int, y: Int) -> Int in
// to
// (__val:(Int, Int)) -> Int in let (x,y) = __val;

std::string paramListText;
{
llvm::raw_string_ostream OS(paramListText);
OS << "(__val:(";
for (size_t i = 0, e = paramList->size(); i != e; ++i) {
if (i != 0)
OS << ", ";
auto param = paramList->get(i);
auto tyLoc = param->getTypeLoc();
if (!tyLoc.isNull() && !tyLoc.getSourceRange().isInvalid()) {
OS << SM.extractText(
Lexer::getCharSourceRangeFromSourceRange(SM,
tyLoc.getSourceRange()));
} else {
param->getType().print(OS);
}
}
OS << "))";
}
std::string varBindText;
{
llvm::raw_string_ostream OS(varBindText);
OS << " let (";
for (size_t i = 0, e = paramList->size(); i != e; ++i) {
if (i != 0)
OS << ",";
auto param = paramList->get(i);
OS << param->getNameStr();
}
OS << ") = __val;";

if (closureE->hasSingleExpressionBody()) {
OS << " return";
}
}

Editor.replace(paramList->getSourceRange(), paramListText);
Editor.insertAfterToken(closureE->getInLoc(), varBindText);
return true;
};

if (handleCallsToEmptyTuple(E))
return;
if (handleTupleMapToClosureArgs(E))
return;
}

bool walkToExprPre(Expr *E) override {
Expand Down
2 changes: 1 addition & 1 deletion test/Migrator/no_extraneous_argument_labels.swift.expected
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ func foo(_ oc: [String]) {
var args: [String] = []
let dictionary: [String: String] = [:]
args.append(contentsOf: oc.map { orderedColumn in
dictionary.first { let (column, value) = $0; return true }!.value
dictionary.first { (column, value) in true }!.value
})
}
22 changes: 11 additions & 11 deletions test/Migrator/tuple-arguments.swift.expected
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,25 @@ func test5(_: (Int, Int, Int) -> ()) {}
test5({ (x,y,z) in })

func test6(_: ((Int, Int)) -> ()) {}
test6({ let (x,y) = $0; })
test6({ (x,y) in })
func test7(_: ((Int, Int, Int)) -> ()) {}
test7({ let (x,y,z) = $0; })
test6({ let (x, y) = $0; })
test6({ let (_, _) = $0; })
test6({ (__val:(Int, Int)) in let (x,y) = __val; })
test6({ (__val:(Int, Int)) ->() in let (_,_) = __val; })
test7({ (x,y,z) in })
test6({ (_ x, _ y) in })
test6({ (_, _) in })
test6({ (x:Int, y:Int) in })
test6({ (_, _) ->() in })

func test8(_: ((Int, Int)) -> Int) {}
test8 { (__val:(Int, Int)) -> Int in let (_,_) = __val; return 2 }
test8 { let (x, y) = $0; return x }
test8 { (_, _) -> Int in 2 }
test8 { (x, y) in x }

func isEven(_ x: Int) -> Bool { return x % 2 == 0 }
let items = Array(zip(0..<10, 0..<10))
_ = items.filter { let (_, x) = $0; return isEven(x) }
_ = items.filter { (_, x) in isEven(x) }
_ = items.filter { _ in true }

func toString(indexes: Int?...) -> String {
let _ = indexes.enumerated().flatMap({ (__val:(Int, Int?)) -> String? in let (i,index) = __val;
let _ = indexes.enumerated().flatMap({ (i: Int, index: Int?) -> String? in
let _: Int = i
if index != nil {}
return ""
Expand All @@ -58,4 +58,4 @@ extension Dictionary {
}

let dictionary: [String: String] = [:]
_ = dictionary.first { let (column, value) = $0; return true }!.value
_ = dictionary.first { (column, value) in true }!.value