Skip to content

Commit 2196eb3

Browse files
authored
Merge pull request #29409 from DougGregor/function-builder-multi-if
[Function builders] Support multiple Boolean conditions in 'if' statements
2 parents 1576ab7 + 6238923 commit 2196eb3

File tree

3 files changed

+77
-32
lines changed

3 files changed

+77
-32
lines changed

lib/Sema/BuilderTransform.cpp

Lines changed: 61 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -324,18 +324,27 @@ class BuilderClosureVisitor
324324
CONTROL_FLOW_STMT(Yield)
325325
CONTROL_FLOW_STMT(Defer)
326326

327-
static Expr *getTrivialBooleanCondition(StmtCondition condition) {
328-
if (condition.size() != 1)
329-
return nullptr;
327+
/// Whether we can handle all of the conditions for this statement.
328+
static bool canHandleStmtConditions(StmtCondition condition) {
329+
for (const auto &element : condition) {
330+
switch (element.getKind()) {
331+
case StmtConditionElement::CK_Boolean:
332+
continue;
330333

331-
return condition.front().getBooleanOrNull();
334+
case StmtConditionElement::CK_PatternBinding:
335+
case StmtConditionElement::CK_Availability:
336+
return false;
337+
}
338+
}
339+
340+
return true;
332341
}
333342

334343
static bool isBuildableIfChainRecursive(IfStmt *ifStmt,
335344
unsigned &numPayloads,
336345
bool &isOptional) {
337-
// The conditional must be trivial.
338-
if (!getTrivialBooleanCondition(ifStmt->getCond()))
346+
// Check whether we can handle the conditional.
347+
if (!canHandleStmtConditions(ifStmt->getCond()))
339348
return false;
340349

341350
// The 'then' clause contributes a payload.
@@ -461,26 +470,37 @@ class BuilderClosureVisitor
461470
payloadIndex + 1, numPayloads, isOptional);
462471
}
463472

464-
// Generate constraints for the various subexpressions.
465-
auto condExpr = getTrivialBooleanCondition(ifStmt->getCond());
466-
assert(condExpr && "Cannot get here without a trivial Boolean condition");
467-
condExpr = cs->generateConstraints(condExpr, dc);
468-
if (!condExpr) {
469-
hadError = true;
470-
return nullptr;
471-
}
472-
473473
// Condition must convert to Bool.
474474
// FIXME: This should be folded into constraint generation for conditions.
475475
auto boolDecl = ctx.getBoolDecl();
476476
if (!boolDecl) {
477477
hadError = true;
478478
return nullptr;
479479
}
480-
cs->addConstraint(ConstraintKind::Conversion,
481-
cs->getType(condExpr),
482-
boolDecl->getDeclaredType(),
483-
cs->getConstraintLocator(condExpr));
480+
481+
// Generate constraints for the conditions.
482+
for (const auto &condElement : ifStmt->getCond()) {
483+
switch (condElement.getKind()) {
484+
case StmtConditionElement::CK_Boolean: {
485+
Expr *condExpr = condElement.getBoolean();
486+
condExpr = cs->generateConstraints(condExpr, dc);
487+
if (!condExpr) {
488+
hadError = true;
489+
return nullptr;
490+
}
491+
492+
cs->addConstraint(ConstraintKind::Conversion,
493+
cs->getType(condExpr),
494+
boolDecl->getDeclaredType(),
495+
cs->getConstraintLocator(condExpr));
496+
continue;
497+
}
498+
499+
case StmtConditionElement::CK_PatternBinding:
500+
case StmtConditionElement::CK_Availability:
501+
llvm_unreachable("unhandled statement condition");
502+
}
503+
}
484504

485505
// The operand should have optional type if we had optional results,
486506
// so we just need to call `buildIf` now, since we're at the top level.
@@ -850,19 +870,29 @@ class BuilderClosureRewriter
850870

851871
Stmt *visitIfStmt(IfStmt *ifStmt, FunctionBuilderTarget target) {
852872
// Rewrite the condition.
853-
// FIXME: We should handle the whole condition within the type system.
854-
auto cond = ifStmt->getCond();
855-
auto condExpr = cond.front().getBoolean();
856-
auto finalCondExpr = rewriteExpr(condExpr);
857-
858-
// Load the condition if needed.
859-
if (finalCondExpr->getType()->is<LValueType>()) {
860-
auto &cs = solution.getConstraintSystem();
861-
finalCondExpr = cs.addImplicitLoadExpr(finalCondExpr);
862-
}
873+
auto condition = ifStmt->getCond();
874+
for (auto &condElement : condition) {
875+
switch (condElement.getKind()) {
876+
case StmtConditionElement::CK_Boolean: {
877+
auto condExpr = condElement.getBoolean();
878+
auto finalCondExpr = rewriteExpr(condExpr);
879+
880+
// Load the condition if needed.
881+
if (finalCondExpr->getType()->is<LValueType>()) {
882+
auto &cs = solution.getConstraintSystem();
883+
finalCondExpr = cs.addImplicitLoadExpr(finalCondExpr);
884+
}
885+
886+
condElement.setBoolean(finalCondExpr);
887+
continue;
888+
}
863889

864-
cond.front().setBoolean(finalCondExpr);
865-
ifStmt->setCond(cond);
890+
case StmtConditionElement::CK_PatternBinding:
891+
case StmtConditionElement::CK_Availability:
892+
llvm_unreachable("unhandled statement condition");
893+
}
894+
}
895+
ifStmt->setCond(condition);
866896

867897
assert(target.kind == FunctionBuilderTarget::TemporaryVar);
868898
auto temporaryVar = target.captured.first;

stdlib/public/Darwin/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ set(all_overlays
4444
MetalKit
4545
ModelIO
4646
NaturalLanguage
47-
Network
47+
# Network
4848
ObjectiveC
4949
OpenCL
5050
os

test/Constraints/function_builder.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,3 +461,18 @@ func testNestedClosuresWithDependencies(cond: Bool) {
461461
}
462462
}
463463
}
464+
465+
// Check that we can handle multiple conditions in an 'if' statement.
466+
func testIfConditions(cond: Bool, c1: Bool, i1: Int, i2: Int) {
467+
tuplify(cond) { x in
468+
"testIfConditions"
469+
if i1 == i2, c1, x {
470+
1
471+
"hello"
472+
}
473+
3.14159
474+
}
475+
}
476+
testIfConditions(cond: true, c1: true, i1: 1, i2: 1)
477+
// CHECK: testIfConditions
478+
// CHECK-SAME: hello

0 commit comments

Comments
 (0)