Skip to content

Fix some issues in handling of withoutActuallyEscaping. #9695

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
May 18, 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
56 changes: 36 additions & 20 deletions lib/AST/ASTVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1839,35 +1839,51 @@ class Verifier : public ASTWalker {
Out << "MakeTemporarilyEscapableExpr type does not match subexpression";
abort();
}

// Closure and opaque value should both be functions, with the closure
// noescape and the opaque value escapable but otherwise matching.
auto closureFnTy = E->getNonescapingClosureValue()->getType()
->getAs<FunctionType>();
if (!closureFnTy) {
Out << "MakeTemporarilyEscapableExpr closure type is not a closure";

auto call = dyn_cast<CallExpr>(E->getSubExpr());
if (!call) {
Out << "MakeTemporarilyEscapableExpr subexpression is not a call\n";
abort();
}
auto opaqueValueFnTy = E->getOpaqueValue()->getType()
->getAs<FunctionType>();
if (!opaqueValueFnTy) {
Out<<"MakeTemporarilyEscapableExpr opaque value type is not a closure";

auto callFnTy = call->getFn()->getType()->getAs<FunctionType>();
if (!callFnTy) {
Out << "MakeTemporarilyEscapableExpr call does not call function\n";
abort();
}
if (!closureFnTy->isNoEscape()) {
Out << "MakeTemporarilyEscapableExpr closure type should be noescape";
if (!callFnTy->getExtInfo().isNoEscape()) {
Out << "MakeTemporarilyEscapableExpr called function is not noescape\n";
abort();
}
if (opaqueValueFnTy->isNoEscape()) {
Out << "MakeTemporarilyEscapableExpr opaque value type should be "
"escaping";

auto callArgTy = call->getArg()->getType()->getAs<FunctionType>();
if (!callArgTy) {
Out << "MakeTemporarilyEscapableExpr call argument is not a function\n";
abort();
}

// Closure and opaque value should both be functions, with the closure
// noescape and the opaque value escapable but otherwise matching.
auto closureFnTy =
E->getNonescapingClosureValue()->getType()->getAs<FunctionType>();
if (!closureFnTy) {
Out << "MakeTemporarilyEscapableExpr closure type is not a closure\n";
abort();
}
auto opaqueValueFnTy =
E->getOpaqueValue()->getType()->getAs<FunctionType>();
if (!opaqueValueFnTy) {
Out << "MakeTemporarilyEscapableExpr opaque value type is not a "
"closure\n";
abort();
}
if (!closureFnTy->isEqual(
opaqueValueFnTy->withExtInfo(opaqueValueFnTy->getExtInfo()
.withNoEscape()))) {
auto closureFnNoEscape =
closureFnTy->withExtInfo(closureFnTy->getExtInfo().withNoEscape());
auto opaqueValueNoEscape = opaqueValueFnTy->withExtInfo(
opaqueValueFnTy->getExtInfo().withNoEscape());
if (!closureFnNoEscape->isEqual(opaqueValueNoEscape)) {
Out << "MakeTemporarilyEscapableExpr closure and opaque value type "
"don't match";
"don't match\n";
abort();
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6745,7 +6745,8 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
assert(arg->getNumElements() == 2 && "should have two arguments");
auto nonescaping = arg->getElements()[0];
auto body = arg->getElements()[1];
auto bodyFnTy = cs.getType(body)->castTo<FunctionType>();
auto bodyTy = cs.getType(body)->getLValueOrInOutObjectType();
auto bodyFnTy = bodyTy->castTo<FunctionType>();
auto escapableType = bodyFnTy->getInput();
auto resultType = bodyFnTy->getResult();

Expand Down
3 changes: 0 additions & 3 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3682,9 +3682,6 @@ ConstraintSystem::simplifyEscapableFunctionOfConstraint(

type2 = getFixedTypeRecursive(type2, flags, /*wantRValue=*/true);
if (auto fn2 = type2->getAs<FunctionType>()) {
// We should have the noescape end of the relation.
if (!fn2->getExtInfo().isNoEscape())
return SolutionKind::Error;
// Solve forward by binding the other type variable to the escapable
// variation of this type.
auto fn1 = fn2->withExtInfo(fn2->getExtInfo().withNoEscape(false));
Expand Down
34 changes: 34 additions & 0 deletions test/Constraints/without_actually_escaping_no_errors.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// RUN: %target-swift-frontend -module-name main -typecheck -swift-version 3 %s
// RUN: %target-swift-frontend -module-name main -typecheck -swift-version 4 %s

// These tests are split out to ensure that we run the AST verifier's
// special post-type-checked verifications, which don't currently
// happen if any errors occur anywhere during compilation.

func rdar32239354_1(_ fn: () -> Void) {
var other: (() -> Void) -> Void = { _ in }

withoutActuallyEscaping(fn, do: other)
// Reassign to avoid warning about changing this to a let, since we
// need this to be a var to trigger the original issue.
other = { _ in }
}

func rdar32239354_2(_ fn: () -> Void, other: inout (() -> Void) -> Void) {
withoutActuallyEscaping(fn, do: other)
}

func testVariations(
_ no_escape: () -> (),
_ escape: @escaping () -> (),
_ takesFn: (()->()) -> ()->()
) -> () -> () {
withoutActuallyEscaping(no_escape) { _ in }
withoutActuallyEscaping({}) { _ in }
withoutActuallyEscaping(escape) { _ in }
_ = withoutActuallyEscaping(no_escape, do: takesFn)
_ = withoutActuallyEscaping(escape, do: takesFn)
_ = withoutActuallyEscaping(no_escape) {
return takesFn($0)
}
}