Skip to content

Sema: Implicit conversion for single-expression closures of Never type (3.0) #4951

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
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
19 changes: 1 addition & 18 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,8 @@ class alignas(8) Expr {
/// True if closure parameters were synthesized from anonymous closure
/// variables.
unsigned HasAnonymousClosureVars : 1;

/// True if this is a closure created as a result of a void contextual
/// conversion.
unsigned IsVoidConversionClosure : 1;
};
enum { NumClosureExprBits = NumAbstractClosureExprBits + 2 };
enum { NumClosureExprBits = NumAbstractClosureExprBits + 1 };
static_assert(NumClosureExprBits <= 32, "fits in an unsigned");

class BindOptionalExprBitfields {
Expand Down Expand Up @@ -3393,7 +3389,6 @@ class ClosureExpr : public AbstractClosureExpr {
Body(nullptr) {
setParameterList(params);
ClosureExprBits.HasAnonymousClosureVars = false;
ClosureExprBits.IsVoidConversionClosure = false;
}

SourceRange getSourceRange() const;
Expand All @@ -3419,18 +3414,6 @@ class ClosureExpr : public AbstractClosureExpr {
ClosureExprBits.HasAnonymousClosureVars = true;
}

/// \brief Determine if this closure was created to satisfy a contextual
/// conversion to a void function type.
bool isVoidConversionClosure() const {
return ClosureExprBits.IsVoidConversionClosure;
}

/// \brief Indicate that this closure was created to satisfy a contextual
/// conversion to a void function type.
void setIsVoidConversionClosure() {
ClosureExprBits.IsVoidConversionClosure = true;
}

/// \brief Determine whether this closure expression has an
/// explicitly-specified result type.
bool hasExplicitResultType() const { return ArrowLoc.isValid(); }
Expand Down
2 changes: 0 additions & 2 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2080,8 +2080,6 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
printClosure(E, "closure_expr");
if (E->hasSingleExpressionBody())
OS << " single-expression";
if (E->isVoidConversionClosure())
OS << " void-conversion";

if (E->getParameters()) {
OS << '\n';
Expand Down
20 changes: 9 additions & 11 deletions lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1827,22 +1827,20 @@ FORWARD_SOURCE_LOCS_TO(ClosureExpr, Body.getPointer())

Expr *ClosureExpr::getSingleExpressionBody() const {
assert(hasSingleExpressionBody() && "Not a single-expression body");
if (!isVoidConversionClosure()) {
return cast<ReturnStmt>(getBody()->getElement(0).get<Stmt *>())
->getResult();
} else {
return getBody()->getElement(0).get<Expr *>();
}
auto body = getBody()->getElement(0);
if (body.is<Stmt *>())
return cast<ReturnStmt>(body.get<Stmt *>())->getResult();
return body.get<Expr *>();
}

void ClosureExpr::setSingleExpressionBody(Expr *NewBody) {
assert(hasSingleExpressionBody() && "Not a single-expression body");
if (!isVoidConversionClosure()) {
cast<ReturnStmt>(getBody()->getElement(0).get<Stmt *>())
->setResult(NewBody);
} else {
return getBody()->setElement(0, NewBody);
auto body = getBody()->getElement(0);
if (body.is<Stmt *>()) {
cast<ReturnStmt>(body.get<Stmt *>())->setResult(NewBody);
return;
}
getBody()->setElement(0, NewBody);
}

FORWARD_SOURCE_LOCS_TO(AutoClosureExpr, Body)
Expand Down
74 changes: 60 additions & 14 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1198,10 +1198,10 @@ namespace {
}

public:
/// \brief Coerce a closure expression with a non-void return type to a
/// contextual function type with a void return type.


/// \brief Coerce a closure expression with a non-Void return type to a
/// contextual function type with a Void return type.
///
/// This operation cannot fail.
///
Expand All @@ -1210,6 +1210,17 @@ namespace {
/// \returns The coerced closure expression.
///
ClosureExpr *coerceClosureExprToVoid(ClosureExpr *expr);

/// \brief Coerce a closure expression with a Never return type to a
/// contextual function type with some other return type.
///
/// This operation cannot fail.
///
/// \param expr The closure expression to coerce.
///
/// \returns The coerced closure expression.
///
ClosureExpr *coerceClosureExprFromNever(ClosureExpr *expr);

/// \brief Coerce the given expression to the given type.
///
Expand Down Expand Up @@ -5017,14 +5028,12 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {

// Re-write the single-expression closure to return '()'
assert(closureExpr->hasSingleExpressionBody());

// Transform the ClosureExpr representation into the "expr + return ()" rep
// if it isn't already.
if (!closureExpr->isVoidConversionClosure()) {

auto member = closureExpr->getBody()->getElement(0);

// A single-expression body contains a single return statement.

// A single-expression body contains a single return statement
// prior to this transformation.
auto member = closureExpr->getBody()->getElement(0);

if (member.is<Stmt *>()) {
auto returnStmt = cast<ReturnStmt>(member.get<Stmt *>());
auto singleExpr = returnStmt->getResult();
auto voidExpr = TupleExpr::createEmpty(tc.Context,
Expand All @@ -5051,7 +5060,6 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {
/*implicit*/true);

closureExpr->setImplicit();
closureExpr->setIsVoidConversionClosure();
closureExpr->setBody(braceStmt, /*isSingleExpression*/true);
}

Expand All @@ -5065,6 +5073,38 @@ ClosureExpr *ExprRewriter::coerceClosureExprToVoid(ClosureExpr *closureExpr) {
return closureExpr;
}

ClosureExpr *ExprRewriter::coerceClosureExprFromNever(ClosureExpr *closureExpr) {
auto &tc = cs.getTypeChecker();

// Re-write the single-expression closure to drop the 'return'.
assert(closureExpr->hasSingleExpressionBody());

// A single-expression body contains a single return statement
// prior to this transformation.
auto member = closureExpr->getBody()->getElement(0);

if (member.is<Stmt *>()) {
auto returnStmt = cast<ReturnStmt>(member.get<Stmt *>());
auto singleExpr = returnStmt->getResult();

tc.checkIgnoredExpr(singleExpr);

SmallVector<ASTNode, 1> elements;
elements.push_back(singleExpr);

auto braceStmt = BraceStmt::create(tc.Context,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of AST hackery feels gross, but... okay. I don't see a better localized fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just copy and pasted from the old code (and I know, that's a HORRIBLE excuse)

closureExpr->getStartLoc(),
elements,
closureExpr->getEndLoc(),
/*implicit*/true);

closureExpr->setImplicit();
closureExpr->setBody(braceStmt, /*isSingleExpression*/true);
}

return closureExpr;
}

static void
maybeDiagnoseUnsupportedFunctionConversion(TypeChecker &tc, Expr *expr,
AnyFunctionType *toType) {
Expand Down Expand Up @@ -6471,9 +6511,15 @@ namespace {
closure->setSingleExpressionBody(body);

if (body) {

// A single-expression closure with a non-Void expression type
// coerces to a Void-returning function type.
if (fnType->getResult()->isVoid() && !body->getType()->isVoid()) {
closure = Rewriter.coerceClosureExprToVoid(closure);
// A single-expression closure with a Never expression type
// coerces to any other function type.
} else if (!fnType->getResult()->isUninhabited() &&
body->getType()->isUninhabited()) {
closure = Rewriter.coerceClosureExprFromNever(closure);
} else {

body = Rewriter.coerceToType(body,
Expand Down
7 changes: 4 additions & 3 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1959,10 +1959,11 @@ ConstraintSystem::matchTypes(Type type1, Type type2, TypeMatchKind kind,
}
}

// If the types disagree, but we're comparing a non-void, single-expression
// closure result type to a void function result type, allow the conversion.
// Allow '() -> T' to '() -> ()' and '() -> Never' to '() -> T' for closure
// literals.
{
if (concrete && kind >= TypeMatchKind::Subtype && type2->isVoid()) {
if (concrete && kind >= TypeMatchKind::Subtype &&
(type1->isUninhabited() || type2->isVoid())) {
SmallVector<LocatorPathElt, 2> parts;
locator.getLocatorParts(parts);

Expand Down
52 changes: 0 additions & 52 deletions test/Misc/single_expr_closure_conversion.swift

This file was deleted.

57 changes: 57 additions & 0 deletions test/expr/closure/single_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,60 @@ func expect<T>(_ expression: @autoclosure () -> T) -> Expectation<T> {
}
func describe(_ closure: () -> ()) {}
func f() { describe { _ = expect("what") } }

struct Blob {}

func withBlob(block: (Blob) -> ()) {}

protocol Binding {}
extension Int: Binding {}
extension Double: Binding {}
extension String: Binding {}
extension Blob: Binding {}

struct Stmt {
@discardableResult
func bind(_ values: Binding?...) -> Stmt {
return self
}

@discardableResult
func bind(_ values: [Binding?]) -> Stmt {
return self
}

@discardableResult
func bind(_ values: [String: Binding?]) -> Stmt {
return self
}
}

let stmt = Stmt()
withBlob { stmt.bind(1, 2.0, "3", $0) }
withBlob { stmt.bind([1, 2.0, "3", $0]) }
withBlob { stmt.bind(["1": 1, "2": 2.0, "3": "3", "4": $0]) }

// <rdar://problem/19840785>
// We shouldn't crash on the call to 'a.dispatch' below.
class A {
func dispatch(_ f : () -> Void) {
f()
}
}

class C {
var prop = 0
var a = A()

func act() {
a.dispatch({() -> Void in
self.prop // expected-warning {{expression of type 'Int' is unused}}
})
}
}

// Never-returning expressions
func haltAndCatchFire() -> Never { while true { } }
let backupPlan: () -> Int = { haltAndCatchFire() }
func missionCritical(storage: () -> String) {}
missionCritical(storage: { haltAndCatchFire() })