Skip to content

[ConstraintSystem] SE-0326: Temporarily prevent multi-statement closure inference in result builder contexts #40708

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
4 changes: 4 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -5287,6 +5287,10 @@ class ConstraintSystem {
/// part of the constraint system.
void forEachExpr(Expr *expr, llvm::function_ref<Expr *(Expr *)> callback);

/// Determine whether one of the parent closures the given one is nested
/// in (if any) has a result builder applied to its body.
bool isInResultBuilderContext(ClosureExpr *closure) const;

SWIFT_DEBUG_DUMP;
SWIFT_DEBUG_DUMPER(dump(Expr *));

Expand Down
7 changes: 3 additions & 4 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8002,6 +8002,8 @@ namespace {
/// \returns true if any part of the processing fails.
bool processDelayed() {
bool hadError = false;
auto &solution = Rewriter.solution;
auto &cs = solution.getConstraintSystem();

while (!ClosuresToTypeCheck.empty()) {
auto *closure = ClosuresToTypeCheck.pop_back_val();
Expand All @@ -8013,10 +8015,7 @@ namespace {
// as a stack because multi-statement closures could
// have other multi-statement closures in the body.
auto &ctx = closure->getASTContext();
if (ctx.TypeCheckerOpts.EnableMultiStatementClosureInference) {
auto &solution = Rewriter.solution;
auto &cs = solution.getConstraintSystem();

if (cs.participatesInInference(closure)) {
hadError |= cs.applySolutionToBody(
solution, closure, Rewriter.dc,
[&](SolutionApplicationTarget target) {
Expand Down
73 changes: 43 additions & 30 deletions lib/Sema/CSClosure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,28 +512,28 @@ class ClosureConstraintGenerator
}

void visitBreakStmt(BreakStmt *breakStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Break");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Break");
}

void visitContinueStmt(ContinueStmt *continueStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Continue");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Continue");
}

void visitDeferStmt(DeferStmt *deferStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Defer");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Defer");
}

void visitFallthroughStmt(FallthroughStmt *fallthroughStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Fallthrough");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Fallthrough");
}

void visitIfStmt(IfStmt *ifStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: If");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: If");

SmallVector<ElementInfo, 4> elements;

Expand Down Expand Up @@ -562,8 +562,8 @@ class ClosureConstraintGenerator
}

void visitGuardStmt(GuardStmt *guardStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Guard");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Guard");

createConjunction(cs,
{makeElement(guardStmt->getCondPointer(),
Expand All @@ -574,8 +574,8 @@ class ClosureConstraintGenerator
}

void visitWhileStmt(WhileStmt *whileStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Guard");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: While");

createConjunction(cs,
{makeElement(whileStmt->getCondPointer(),
Expand All @@ -586,15 +586,15 @@ class ClosureConstraintGenerator
}

void visitDoStmt(DoStmt *doStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Do");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Do");

visitBraceStmt(doStmt->getBody());
}

void visitRepeatWhileStmt(RepeatWhileStmt *repeatWhileStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: RepeatWhile");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: RepeatWhile");

createConjunction(cs,
{makeElement(repeatWhileStmt->getCond(),
Expand All @@ -606,8 +606,8 @@ class ClosureConstraintGenerator
}

void visitPoundAssertStmt(PoundAssertStmt *poundAssertStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: PoundAssert");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: PoundAssert");

createConjunction(cs,
{makeElement(poundAssertStmt->getCondition(),
Expand All @@ -618,8 +618,8 @@ class ClosureConstraintGenerator
}

void visitThrowStmt(ThrowStmt *throwStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Throw");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Throw");

Type errType =
cs.getASTContext().getErrorDecl()->getDeclaredInterfaceType();
Expand All @@ -641,8 +641,8 @@ class ClosureConstraintGenerator
}

void visitForEachStmt(ForEachStmt *forEachStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: ForEach");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: ForEach");

auto *stmtLoc = cs.getConstraintLocator(locator);

Expand Down Expand Up @@ -679,8 +679,8 @@ class ClosureConstraintGenerator
}

void visitSwitchStmt(SwitchStmt *switchStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Switch");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Switch");

auto *switchLoc = cs.getConstraintLocator(
locator, LocatorPathElt::ClosureBodyElement(switchStmt));
Expand All @@ -705,8 +705,8 @@ class ClosureConstraintGenerator
}

void visitDoCatchStmt(DoCatchStmt *doStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: DoCatch");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: DoCatch");

auto *doLoc = cs.getConstraintLocator(
locator, LocatorPathElt::ClosureBodyElement(doStmt));
Expand All @@ -725,8 +725,8 @@ class ClosureConstraintGenerator
}

void visitCaseStmt(CaseStmt *caseStmt) {
if (!isSupportedMultiStatementClosure())
llvm_unreachable("Unsupported statement: Case");
assert(isSupportedMultiStatementClosure() &&
"Unsupported statement: Case");

Type contextualTy;

Expand Down Expand Up @@ -919,6 +919,19 @@ bool ConstraintSystem::generateConstraints(ClosureExpr *closure) {
return false;
}

bool ConstraintSystem::isInResultBuilderContext(ClosureExpr *closure) const {
if (!closure->hasSingleExpressionBody()) {
auto *DC = closure->getParent();
do {
if (auto *parentClosure = dyn_cast<ClosureExpr>(DC)) {
if (resultBuilderTransformed.count(parentClosure))
return true;
}
} while ((DC = DC->getParent()));
}
return false;
}

bool isConditionOfStmt(ConstraintLocatorBuilder locator) {
auto last = locator.last();
if (!(last && last->is<LocatorPathElt::Condition>()))
Expand Down
9 changes: 7 additions & 2 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5913,8 +5913,13 @@ bool ConstraintSystem::participatesInInference(ClosureExpr *closure) const {
return false;

auto &ctx = closure->getASTContext();
return !closure->hasEmptyBody() &&
ctx.TypeCheckerOpts.EnableMultiStatementClosureInference;
if (closure->hasEmptyBody() ||
!ctx.TypeCheckerOpts.EnableMultiStatementClosureInference)
return false;

// If body is nested in a parent that has a function builder applied,
// let's prevent inference until result builders.
return !isInResultBuilderContext(closure);
}

TypeVarBindingProducer::TypeVarBindingProducer(BindingSet &bindings)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -swift-version 5 -experimental-multi-statement-closures
// REQUIRES: objc_interop
// REQUIRES: OS=macosx

import SwiftUI

enum Status {
case complete
case waiting
}

struct Item : Hashable {
var question: String
var answer: Int
}

func transform(_ v: Int) -> String? { return String(v) }
func transform(_ v: Double) -> String? { return String(v) }

struct MyView : View {
var status: Status

var items: [Item] = []

var currItem: Item {
get { fatalError() }
nonmutating set { }
}

var body: some View {
ZStack {
ItemsView {
EmptyView()
} results: {
switch (status) {
case .complete:
ForEach(items, id: \.self) { item in
if let question = item.question,
let answer = item.answer {
ItemView {
currItem.question = question
currItem.answer = answer
} content: {
AnswerView(title: "",
color: .red,
answer: transform(answer) ?? "",
selected: false)
}
}
}

default:
EmptyView()
}
}
}
}
}

struct AnswerView : View {
var title: String
var color: Color
var answer: String
var selected: Bool

var body: some View {
EmptyView()
}
}

struct ItemsView<Content: View, Results: View>: View {
@ViewBuilder var content: Content
@ViewBuilder var results: Results

var body: some View {
EmptyView()
}
}

struct ItemView<Content: View> : View {
var fn: () -> Void
@ViewBuilder var content: Content

var body: some View {
EmptyView()
}
}