Skip to content

Commit 51c833f

Browse files
xedinxedin
authored
Merge pull request #79617 from xedin/isolate-buildBlock-in-if-and-case-statements
[BuilderTransform] Type-check `buildBlock` of each condition body separately from "join" operation
2 parents f387219 + d70f14e commit 51c833f

File tree

4 files changed

+70
-11
lines changed

4 files changed

+70
-11
lines changed

‎lib/Sema/BuilderTransform.cpp

+16-4
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,8 @@ class ResultBuilderTransform
261261
}
262262

263263
std::pair<NullablePtr<Expr>, std::optional<UnsupportedElt>>
264-
transform(BraceStmt *braceStmt, SmallVectorImpl<ASTNode> &newBody) {
264+
transform(BraceStmt *braceStmt, SmallVectorImpl<ASTNode> &newBody,
265+
bool isolateBuildBlock = false) {
265266
SmallVector<Expr *, 4> buildBlockArguments;
266267

267268
auto failTransform = [&](UnsupportedElt unsupported) {
@@ -326,6 +327,14 @@ class ResultBuilderTransform
326327
auto *buildBlock = builder.buildCall(
327328
braceStmt->getStartLoc(), ctx.Id_buildBlock, buildBlockArguments,
328329
/*argLabels=*/{});
330+
331+
if (isolateBuildBlock) {
332+
auto *buildBlockVar = captureExpr(buildBlock, newBody);
333+
return std::make_pair(
334+
builder.buildVarRef(buildBlockVar, braceStmt->getStartLoc()),
335+
std::nullopt);
336+
}
337+
329338
return std::make_pair(buildBlock, std::nullopt);
330339
}
331340
}
@@ -502,7 +511,8 @@ class ResultBuilderTransform
502511

503512
auto *ifBraceStmt = cast<BraceStmt>(ifStmt->getThenStmt());
504513

505-
std::tie(thenVarRef, unsupported) = transform(ifBraceStmt, thenBody);
514+
std::tie(thenVarRef, unsupported) =
515+
transform(ifBraceStmt, thenBody, /*isolateBuildBlock=*/true);
506516
if (unsupported) {
507517
recordUnsupported(*unsupported);
508518
return nullptr;
@@ -537,7 +547,8 @@ class ResultBuilderTransform
537547
auto *elseBraceStmt = cast<BraceStmt>(elseStmt);
538548
SmallVector<ASTNode> elseBody;
539549

540-
std::tie(elseVarRef, unsupported) = transform(elseBraceStmt, elseBody);
550+
std::tie(elseVarRef, unsupported) = transform(
551+
elseBraceStmt, elseBody, /*isolateBuildBlock=*/true);
541552
if (unsupported) {
542553
recordUnsupported(*unsupported);
543554
return nullptr;
@@ -634,7 +645,8 @@ class ResultBuilderTransform
634645
std::optional<UnsupportedElt> unsupported;
635646
SmallVector<ASTNode, 4> newBody;
636647

637-
std::tie(caseVarRef, unsupported) = transform(body, newBody);
648+
std::tie(caseVarRef, unsupported) =
649+
transform(body, newBody, /*isolateBuildBlock=*/true);
638650

639651
if (unsupported) {
640652
recordUnsupported(*unsupported);

‎test/ConstExtraction/ExtractResultBuilders.swift

+19-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public enum FooBuilder {
5757
public static func buildBlock(_ components: Component...) -> Component {
5858
return components.flatMap { $0 }
5959
}
60-
60+
6161
public static func buildLimitedAvailability(_ component: Component) -> Component {
6262
return component
6363
}
@@ -359,6 +359,9 @@ public struct MyFooProviderInferredWithArrayReturn: FooProvider {
359359
// CHECK-NEXT: ]
360360
// CHECK-NEXT: }
361361
// CHECK-NEXT: }
362+
// CHECK-NEXT: },
363+
// CHECK-NEXT: {
364+
// CHECK-NEXT: "element": {}
362365
// CHECK-NEXT: }
363366
// CHECK-NEXT: ],
364367
// CHECK-NEXT: "elseElements": [
@@ -379,6 +382,9 @@ public struct MyFooProviderInferredWithArrayReturn: FooProvider {
379382
// CHECK-NEXT: ]
380383
// CHECK-NEXT: }
381384
// CHECK-NEXT: }
385+
// CHECK-NEXT: },
386+
// CHECK-NEXT: {
387+
// CHECK-NEXT: "element": {}
382388
// CHECK-NEXT: }
383389
// CHECK-NEXT: ],
384390
// CHECK-NEXT: "elseElements": [
@@ -397,6 +403,9 @@ public struct MyFooProviderInferredWithArrayReturn: FooProvider {
397403
// CHECK-NEXT: ]
398404
// CHECK-NEXT: }
399405
// CHECK-NEXT: }
406+
// CHECK-NEXT: },
407+
// CHECK-NEXT: {
408+
// CHECK-NEXT: "element": {}
400409
// CHECK-NEXT: }
401410
// CHECK-NEXT: ]
402411
// CHECK-NEXT: }
@@ -455,6 +464,9 @@ public struct MyFooProviderInferredWithArrayReturn: FooProvider {
455464
// CHECK-NEXT: ]
456465
// CHECK-NEXT: }
457466
// CHECK-NEXT: }
467+
// CHECK-NEXT: },
468+
// CHECK-NEXT: {
469+
// CHECK-NEXT: "element": {}
458470
// CHECK-NEXT: }
459471
// CHECK-NEXT: ],
460472
// CHECK-NEXT: "elseElements": []
@@ -518,6 +530,9 @@ public struct MyFooProviderInferredWithArrayReturn: FooProvider {
518530
// CHECK-NEXT: },
519531
// CHECK-NEXT: {
520532
// CHECK-NEXT: "element": {}
533+
// CHECK-NEXT: },
534+
// CHECK-NEXT: {
535+
// CHECK-NEXT: "element": {}
521536
// CHECK-NEXT: }
522537
// CHECK-NEXT: ],
523538
// CHECK-NEXT: "elseElements": [
@@ -536,6 +551,9 @@ public struct MyFooProviderInferredWithArrayReturn: FooProvider {
536551
// CHECK-NEXT: ]
537552
// CHECK-NEXT: }
538553
// CHECK-NEXT: }
554+
// CHECK-NEXT: },
555+
// CHECK-NEXT: {
556+
// CHECK-NEXT: "element": {}
539557
// CHECK-NEXT: }
540558
// CHECK-NEXT: ]
541559
// CHECK-NEXT: }

‎test/Constraints/result_builder_switch_with_vars.swift

+30
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,33 @@ tuplify {
9090
value.a
9191
}
9292
}
93+
94+
tuplify { _ in
95+
switch true {
96+
// CHECK: (case_stmt {{.*}}
97+
// CHECK: (pattern_named implicit type="Int" "$__builder2")
98+
// CHECK: (declref_expr implicit type="(TupleBuilder.Type) -> (Int) -> Int" location={{.*}} range={{.*}} decl="{{.*}}buildBlock@{{.*}}" function_ref=single apply)
99+
100+
// CHECK: (call_expr implicit type="Either<Int, Int>" {{.*}}
101+
// CHECK-NEXT: (dot_syntax_call_expr implicit type="(Int) -> Either<Int, Int>" {{.*}}
102+
// CHECK-NEXT: (declref_expr implicit type="(TupleBuilder.Type) -> (Int) -> Either<Int, Int>" location={{.*}} range={{.*}} decl="{{.*}}buildEither(first:)@{{.*}}" function_ref=single apply)
103+
// CHECK: (argument_list implicit labels="first:"
104+
// CHECK-NEXT: (argument label="first"
105+
// CHECK-NEXT: (load_expr implicit type="Int" {{.*}}
106+
// CHECK-NEXT: (declref_expr implicit type="@lvalue Int" location={{.*}} range={{.*}} decl="{{.*}}.$__builder2@{{.*}}" function_ref=unapplied))))))))
107+
case true: 0
108+
109+
// CHECK: (case_stmt {{.*}}
110+
// CHECK: (pattern_named implicit type="Int" "$__builder4")
111+
// CHECK: (declref_expr implicit type="(TupleBuilder.Type) -> (Int) -> Int" location={{.*}} range={{.*}} decl="{{.*}}buildBlock@{{.*}}" function_ref=single apply)
112+
113+
// CHECK: (call_expr implicit type="Either<Int, Int>" {{.*}}
114+
// CHECK-NEXT: (dot_syntax_call_expr implicit type="(Int) -> Either<Int, Int>" {{.*}}
115+
// CHECK-NEXT: (declref_expr implicit type="(TupleBuilder.Type) -> (Int) -> Either<Int, Int>" location={{.*}} range={{.*}} decl="{{.*}}buildEither(second:)@{{.*}}" function_ref=single apply)
116+
// CHECK: (argument_list implicit labels="second:"
117+
// CHECK-NEXT: (argument label="second"
118+
// CHECK-NEXT: (load_expr implicit type="Int" {{.*}}
119+
// CHECK-NEXT: (declref_expr implicit type="@lvalue Int" location={{.*}} range={{.*}} decl="{{.*}}.$__builder4@{{.*}}" function_ref=unapplied))))))))
120+
case false: 1
121+
}
122+
}

‎validation-test/Sema/result_builder_buildBlock_resolution.swift

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
// RUN: %target-typecheck-verify-swift
22
// RUN: %target-typecheck-verify-swift -I %t
33

4-
// This test verifies that `buildBlock` is type-checked together with enclosing context,
5-
// which means that it's not captured into separate variable but rather used directly and
6-
// contextual information can impact overload resolution.
4+
// ! - Based on the result builders proposal `buildBlock` shouldn't be type-checked together with `buildOptional`.
75

86
protocol ActionIdentifier: Hashable {
97
}
@@ -14,11 +12,11 @@ struct ActionLookup<Identifier: ActionIdentifier> {
1412

1513
@resultBuilder
1614
enum ActionLookupBuilder<Identifier: ActionIdentifier> { // expected-note 3{{'Identifier' previously declared here}}
17-
static func buildBlock<Identifier: ActionIdentifier>(_ components: [ActionLookup<Identifier>]...) -> ActionLookup<Identifier> { // expected-warning {{generic parameter 'Identifier' shadows generic parameter from outer scope with the same name; this is an error in the Swift 6 language mode}}
15+
static func buildBlock<Identifier: ActionIdentifier>(_ components: [ActionLookup<Identifier>]...) -> ActionLookup<Identifier> { // expected-warning {{generic parameter 'Identifier' shadows generic parameter from outer scope with the same name; this is an error in the Swift 6 language mode}} expected-note {{found this candidate}}
1816
fatalError()
1917
}
2018

21-
static func buildBlock<Identifier: ActionIdentifier>(_ components: [ActionLookup<Identifier>]...) -> [ActionLookup<Identifier>] { // expected-warning {{generic parameter 'Identifier' shadows generic parameter from outer scope with the same name; this is an error in the Swift 6 language mode}}
19+
static func buildBlock<Identifier: ActionIdentifier>(_ components: [ActionLookup<Identifier>]...) -> [ActionLookup<Identifier>] { // expected-warning {{generic parameter 'Identifier' shadows generic parameter from outer scope with the same name; this is an error in the Swift 6 language mode}} expected-note {{found this candidate}}
2220
[]
2321
}
2422

@@ -43,7 +41,8 @@ enum ActionType: String, ActionIdentifier, CaseIterable {
4341
ActionTypeLookup(
4442
.download
4543
)
46-
if true { // If condition is needed to make sure that `buildOptional` affects `buildBlock` resolution.
44+
if true { // If condition without else is needed to make sure that `buildOptional` affects `buildBlock` resolution.
45+
// expected-error@-1 {{ambiguous use of 'buildBlock'}}
4746
ActionTypeLookup(
4847
.upload
4948
)

0 commit comments

Comments
 (0)