From b1f020bf3846b92c7d2f8bcecc5daf1a67c5cfe5 Mon Sep 17 00:00:00 2001 From: Richard Wei Date: Fri, 25 Feb 2022 22:04:47 -0800 Subject: [PATCH] [ResultBuilders] `buildPartialBlock` support This PR implements support for `buildPartialBlock` as proposed in https://forums.swift.org/t/pitch-buildpartialblock-for-result-builders/55561. This is similar to the existing support for `buildBlock(combining:into:)` except that it also checks for availability when deciding whether to fall back to plain old `buildBlock`. > In the result builder transform, the compiler will look for static members `buildPartialBlock(first:)` and `buildPartialBlock(accumulated:next:)` in the builder type. If the following conditions are met: > - Both methods `buildPartialBlock(first:)` and `buildPartialBlock(accumulated:next:)` exist. > - The availability of the enclosing declaration is greater than or equal to the availability of `buildPartialBlock(first:)` and `buildPartialBlock(accumulated:next:)`. When there's no available `buildPartialBlock` to call and there's no `buildBlock`, emit a diagnostic: ```console result builder 'Builder' does not implement any 'buildBlock' or a combination of 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)' with sufficient availability for this call site ``` --- include/swift/AST/DiagnosticsSema.def | 9 + include/swift/AST/KnownIdentifiers.def | 2 + lib/Sema/BuilderTransform.cpp | 74 ++++- lib/Sema/TypeCheckAttr.cpp | 23 +- lib/Sema/TypeChecker.h | 3 +- .../result_builder_buildpartialblock.swift | 257 ++++++++++++++++++ .../result_builder_pairwise_build_block.swift | 3 + .../result_builders_buildpartialblock.swift | 38 +++ ...lders_buildpartialblock_availability.swift | 28 ++ test/lit.cfg | 6 + 10 files changed, 428 insertions(+), 15 deletions(-) create mode 100644 test/Constraints/result_builder_buildpartialblock.swift create mode 100644 test/decl/var/result_builders_buildpartialblock.swift create mode 100644 test/decl/var/result_builders_buildpartialblock_availability.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 28a1b9ed17ba1..b6268d014dc01 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -6095,6 +6095,15 @@ WARNING(result_builder_missing_limited_availability, none, ERROR(result_builder_static_buildblock, none, "result builder must provide at least one static 'buildBlock' " "method", ()) +ERROR(result_builder_static_buildblock_or_buildpartialblock, none, + "result builder must provide at least one static 'buildBlock' " + "method, or both 'buildPartialBlock(first:)' and " + "'buildPartialBlock(accumulated:next:)'", ()) +ERROR(result_builder_missing_available_buildpartialblock, none, + "result builder %0 does not implement any 'buildBlock' or a combination " + "of 'buildPartialBlock(first:)' and " + "'buildPartialBlock(accumulated:next:)' with sufficient availability for " + "this call site", (Type)) NOTE(result_builder_non_static_buildblock, none, "did you mean to make instance method 'buildBlock' static?", ()) NOTE(result_builder_buildblock_enum_case, none, diff --git a/include/swift/AST/KnownIdentifiers.def b/include/swift/AST/KnownIdentifiers.def index c348e367f97c8..7ae9fb928ac24 100644 --- a/include/swift/AST/KnownIdentifiers.def +++ b/include/swift/AST/KnownIdentifiers.def @@ -28,6 +28,7 @@ IDENTIFIER(allKeys) IDENTIFIER(alloc) IDENTIFIER(allocWithZone) IDENTIFIER(allZeros) +IDENTIFIER(accumulated) IDENTIFIER(ActorType) IDENTIFIER(Any) IDENTIFIER(ArrayLiteralElement) @@ -41,6 +42,7 @@ IDENTIFIER(buildFinalResult) IDENTIFIER(buildIf) IDENTIFIER(buildLimitedAvailability) IDENTIFIER(buildOptional) +IDENTIFIER(buildPartialBlock) IDENTIFIER(callAsFunction) IDENTIFIER(Change) IDENTIFIER_WITH_NAME(code_, "_code") diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index c10ece132de62..a0503dd8c1ad6 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -71,7 +71,7 @@ class BuilderClosureVisitor Type builderType; NominalTypeDecl *builder = nullptr; Identifier buildOptionalId; - llvm::SmallDenseMap supportedOps; + llvm::SmallDenseMap supportedOps; SkipUnhandledConstructInResultBuilder::UnhandledNode unhandledNode; @@ -141,15 +141,18 @@ class BuilderClosureVisitor } /// Check whether the builder supports the given operation. - bool builderSupports(Identifier fnName, - ArrayRef argLabels = {}) { - auto known = supportedOps.find(fnName); + bool builderSupports(Identifier fnBaseName, + ArrayRef argLabels = {}, + bool checkAvailability = false) { + DeclName name(dc->getASTContext(), fnBaseName, argLabels); + auto known = supportedOps.find(name); if (known != supportedOps.end()) { return known->second; } - return supportedOps[fnName] = TypeChecker::typeSupportsBuilderOp( - builderType, dc, fnName, argLabels); + return supportedOps[name] = TypeChecker::typeSupportsBuilderOp( + builderType, dc, fnBaseName, argLabels, /*allResults*/ {}, + checkAvailability); } /// Build an implicit variable in this context. @@ -368,11 +371,39 @@ class BuilderClosureVisitor return nullptr; Expr *call = nullptr; - // If the builder supports `buildBlock(combining:into:)`, use this to - // combine subexpressions pairwise. + // If the builder supports `buildPartialBlock(first:)` and + // `buildPartialBlock(accumulated:next:)`, use this to combine + // subexpressions pairwise. if (ctx.LangOpts.EnableExperimentalPairwiseBuildBlock && !expressions.empty() && - builderSupports(ctx.Id_buildBlock, {ctx.Id_combining, ctx.Id_into})) { + builderSupports(ctx.Id_buildPartialBlock, {ctx.Id_first}, + /*checkAvailability*/ true) && + builderSupports(ctx.Id_buildPartialBlock, + {ctx.Id_accumulated, ctx.Id_next}, + /*checkAvailability*/ true)) { + // NOTE: The current implementation uses one-way constraints in between + // subexpressions. It's functionally equivalent to the following: + // let v0 = Builder.buildPartialBlock(first: arg_0) + // let v1 = Builder.buildPartialBlock(accumulated: arg_1, next: v0) + // ... + // return Builder.buildPartialBlock(accumulated: arg_n, next: ...) + call = buildCallIfWanted(braceStmt->getStartLoc(), + ctx.Id_buildPartialBlock, + {expressions.front()}, + /*argLabels=*/{ctx.Id_first}); + for (auto *expr : llvm::drop_begin(expressions)) { + call = buildCallIfWanted(braceStmt->getStartLoc(), + ctx.Id_buildPartialBlock, + {new (ctx) OneWayExpr(call), expr}, + {ctx.Id_accumulated, ctx.Id_next}); + } + } + // TODO: Remove support for the old method name, + // `buildBlock(combining:into:)`. + else if (ctx.LangOpts.EnableExperimentalPairwiseBuildBlock && + !expressions.empty() && + builderSupports(ctx.Id_buildBlock, + {ctx.Id_combining, ctx.Id_into})) { // NOTE: The current implementation uses one-way constraints in between // subexpressions. It's functionally equivalent to the following: // let v0 = Builder.buildBlock(arg_0) @@ -387,6 +418,17 @@ class BuilderClosureVisitor {ctx.Id_combining, ctx.Id_into}); } } + // If `buildBlock` does not exist at this point, it could be the case that + // `buildPartialBlock` did not have the sufficient availability for this + // call site. Diagnose it. + else if (ctx.LangOpts.EnableExperimentalPairwiseBuildBlock && + !builderSupports(ctx.Id_buildBlock)) { + ctx.Diags.diagnose( + braceStmt->getStartLoc(), + diag::result_builder_missing_available_buildpartialblock, + builderType); + return nullptr; + } // Otherwise, call `buildBlock` on all subexpressions. else { // Call Builder.buildBlock(... args ...) @@ -2016,7 +2058,8 @@ std::vector TypeChecker::findReturnStatements(AnyFunctionRef fn) { bool TypeChecker::typeSupportsBuilderOp( Type builderType, DeclContext *dc, Identifier fnName, - ArrayRef argLabels, SmallVectorImpl *allResults) { + ArrayRef argLabels, SmallVectorImpl *allResults, + bool checkAvailability) { bool foundMatch = false; SmallVector foundDecls; dc->lookupQualified( @@ -2036,6 +2079,17 @@ bool TypeChecker::typeSupportsBuilderOp( continue; } + // If we are checking availability, the candidate must have enough + // availability in the calling context. + if (checkAvailability) { + if (AvailableAttr::isUnavailable(func)) + continue; + if (TypeChecker::checkDeclarationAvailability( + func, ExportContext::forFunctionBody( + dc, extractNearestSourceLoc(dc)))) + continue; + } + foundMatch = true; break; } diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 7a0dbcc96df28..35e00e77ca72d 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -3267,15 +3267,30 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) { void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) { auto *nominal = dyn_cast(D); + auto &ctx = D->getASTContext(); SmallVector potentialMatches; bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp( - nominal->getDeclaredType(), nominal, D->getASTContext().Id_buildBlock, + nominal->getDeclaredType(), nominal, ctx.Id_buildBlock, /*argLabels=*/{}, &potentialMatches); - - if (!supportsBuildBlock) { + bool isBuildPartialBlockFeatureEnabled = + ctx.LangOpts.EnableExperimentalPairwiseBuildBlock; + bool supportsBuildPartialBlock = isBuildPartialBlockFeatureEnabled && + TypeChecker::typeSupportsBuilderOp( + nominal->getDeclaredType(), nominal, + ctx.Id_buildPartialBlock, + /*argLabels=*/{ctx.Id_first}, &potentialMatches) && + TypeChecker::typeSupportsBuilderOp( + nominal->getDeclaredType(), nominal, + ctx.Id_buildPartialBlock, + /*argLabels=*/{ctx.Id_accumulated, ctx.Id_next}, &potentialMatches); + + if (!supportsBuildBlock && !supportsBuildPartialBlock) { { auto diag = diagnose( - nominal->getLoc(), diag::result_builder_static_buildblock); + nominal->getLoc(), + isBuildPartialBlockFeatureEnabled + ? diag::result_builder_static_buildblock_or_buildpartialblock + : diag::result_builder_static_buildblock); // If there were no close matches, propose adding a stub. SourceLoc buildInsertionLoc; diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index bec4144f2324a..04964f2ea6009 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1179,7 +1179,8 @@ UnresolvedMemberExpr *getUnresolvedMemberChainBase(Expr *expr); /// are verified against any candidates. bool typeSupportsBuilderOp(Type builderType, DeclContext *dc, Identifier fnName, ArrayRef argLabels = {}, - SmallVectorImpl *allResults = nullptr); + SmallVectorImpl *allResults = nullptr, + bool checkAvailability = false); /// Forces all changes specified by the module's access notes file to be /// applied to this declaration. It is safe to call this function more than diff --git a/test/Constraints/result_builder_buildpartialblock.swift b/test/Constraints/result_builder_buildpartialblock.swift new file mode 100644 index 0000000000000..1329ffe158e7c --- /dev/null +++ b/test/Constraints/result_builder_buildpartialblock.swift @@ -0,0 +1,257 @@ +// RUN: %target-run-simple-swift(-Xfrontend -enable-experimental-pairwise-build-block) | %FileCheck %s +// REQUIRES: executable_test + +struct Values { + var values: T + + init(values: T) { + self.values = values + } + + func map(_ f: (T) -> R) -> Values { + .init(values: f(values)) + } +} + +@resultBuilder +enum NestedTupleBuilder { + static func buildPartialBlock(first x: T) -> Values { + .init(values: x) + } + + static func buildPartialBlock( + accumulated: Values, next: U + ) -> Values<(T, U)> { + .init(values: (accumulated.values, next)) + } +} + +extension Values { + init(@NestedTupleBuilder nested values: () -> Self) { + self = values() + } +} + +let nestedValues = Values(nested: { + 1 + "2" + 3.0 + "yes" +}) +print(nestedValues) + +@resultBuilder +enum NestedTupleBuilder_Not { + @available(*, unavailable) + static func buildPartialBlock(first x: T) -> Values { + .init(values: x) + } + + @available(*, unavailable) + static func buildPartialBlock( + accumulated: Values, next: U + ) -> Values<(T, U)> { + .init(values: (accumulated.values, next)) + } + +#if os(macOS) + @available(macOS 9999, *) + static func buildPartialBlock(first x: Never) -> Values { + fatalError() + } + + @available(macOS 9999, *) + static func buildPartialBlock( + accumulated: Values, next: Never + ) -> Values { + fatalError() + } +#endif + + // This one will be called because no `buildPartialBlock` is available. + static func buildBlock(_ x: Any...) -> Values<[Any]> { + .init(values: x) + } +} + +extension Values { + init(@NestedTupleBuilder_Not nested_not values: () -> Self) { + self = values() + } +} + +let nestedValues_not = Values(nested_not: { + 1 + "2" + 3.0 + "yes" +}) +print(nestedValues_not) + +// CHECK: Values>(values: [1, "2", 3.0, "yes"]) + +@resultBuilder +enum FlatTupleBuilder { + static func buildExpression(_ x: T) -> Values { + .init(values: x) + } + + static func buildPartialBlock(first x: Values) -> Values { + .init(values: x.values) + } + + static func buildPartialBlock( + accumulated: Values, + next: Values + ) -> Values<(T, N)> { + .init(values: (accumulated.values, next.values)) + } + + static func buildPartialBlock( + accumulated: Values<(T0, T1)>, + next: Values + ) -> Values<(T0, T1, N)> { + .init(values: (accumulated.values.0, accumulated.values.1, next.values)) + } + + static func buildPartialBlock( + accumulated: Values<(T0, T1, T2)>, + next: Values + ) -> Values<(T0, T1, T2, N)> { + .init(values: (accumulated.values.0, accumulated.values.1, accumulated.values.2, next.values)) + } + + static func buildPartialBlock( + accumulated: Values<(T0, T1, T2, T3)>, + next: Values + ) -> Values<(T0, T1, T2, T3, N)> { + .init(values: (accumulated.values.0, accumulated.values.1, accumulated.values.2, accumulated.values.3, next.values)) + } + + static func buildBlock(_ x: Never...) -> Values<()> { + assert(x.isEmpty, "I should never be called unless it's nullary") + return .init(values: ()) + } + + static func buildEither(first: T) -> T { + first + } + + static func buildEither(second: T) -> T { + second + } + + static func buildOptional(_ x: Values?) -> Values { + x?.map { $0 } ?? .init(values: nil) + } + + static func buildLimitedAvailability(_ x: Values) -> Values { + x + } +} + +extension Values { + init(@FlatTupleBuilder flat values: () -> Self) { + self = values() + } +} + +let flatValues0 = Values(flat: {}) +print(flatValues0) +// CHECK: Values<()>(values: ()) + +let flatValues1 = Values(flat: { + 1 + "2" + 3.0 +}) +print(flatValues1) +// CHECK: Values<(Int, String, Double)>(values: (1, "2", 3.0)) + +let flatValues2 = Values(flat: { + 1 + "2" + let y = 3.0 + 4.0 + #if false + "not gonna happen" + #endif + if true { + "yes" + } else { + "no" + } + #warning("Beware of pairwise block building") + #if true + if false { + "nah" + } + if #available(*) { + 5.0 + } + #endif +}) +print(flatValues2) + +// CHECK: Values<(Int, String, String, Optional, Optional)>(values: (1, "2", "yes", nil, Optional(5.0))) + +struct Nil: CustomStringConvertible { + var description: String { + "nil" + } +} +struct Cons: CustomStringConvertible { + var head: Head + var tail: Tail + + var description: String { + "(cons \(String(reflecting: head)) \(tail))" + } +} + +@resultBuilder +enum ListBuilder { + static func buildBlock() -> Nil { + Nil() + } + + static func buildPartialBlock(first x: T) -> Cons { + .init(head: x, tail: Nil()) + } + + static func buildPartialBlock(accumulated: T, next: New) -> Cons { + .init(head: next, tail: accumulated) + } + + static func buildBlock(_ x: T...) -> [T] { + fatalError("I should never be called!") + } +} + +func list(@ListBuilder f: () -> T) -> T { + f() +} + +let list0 = list {} +print(list0) +// CHECK: nil + +let list1 = list { "1" } +print(list1) +// Check: (cons 1 nil) + +let list2 = list { + 1 + 2 +} +print(list2) +// CHECK: (cons 2 (cons 1 nil)) +let list3 = list { + 1 + list { + 2.0 + "3" + } + "4" +} +print(list3) +// CHECK: (cons "4" (cons (cons "3" (cons 2.0 nil)) (cons 1 nil))) diff --git a/test/Constraints/result_builder_pairwise_build_block.swift b/test/Constraints/result_builder_pairwise_build_block.swift index a965f46b9ff51..82bc3bf13b93b 100644 --- a/test/Constraints/result_builder_pairwise_build_block.swift +++ b/test/Constraints/result_builder_pairwise_build_block.swift @@ -1,6 +1,9 @@ // RUN: %target-run-simple-swift(-Xfrontend -enable-experimental-pairwise-build-block) | %FileCheck %s // REQUIRES: executable_test +// TODO: This test is for the old method name `buildBlock(combining:into:)`. Delete this file once +// clients have moved to `buildPartialBlock`. + struct Values { var values: T diff --git a/test/decl/var/result_builders_buildpartialblock.swift b/test/decl/var/result_builders_buildpartialblock.swift new file mode 100644 index 0000000000000..2df474278f2d2 --- /dev/null +++ b/test/decl/var/result_builders_buildpartialblock.swift @@ -0,0 +1,38 @@ +// RUN: %target-typecheck-buildpartialblock-verify-swift + +@resultBuilder +struct ValidBuilder1 { + static func buildPartialBlock(first: Int) -> Int { fatalError() } + static func buildPartialBlock(accumulated: Int, next: Int) -> Int { fatalError() } +} + +@resultBuilder +struct ValidBuilder2 { + @available(*, unavailable) + static func buildPartialBlock(first: Int) -> Int { fatalError() } + @available(*, unavailable) + static func buildPartialBlock(accumulated: Int, next: Int) -> Int { fatalError() } + // available + // static func buildBlock(_: Int...) -> Int { fatalError() } +} + +func caller1_ValidBuilder2() { + // expected-error @+1 {{result builder 'ValidBuilder2' does not implement any 'buildBlock' or a combination of 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)' with sufficient availability for this call site}} + @ValidBuilder2 var x: Int { + 1 + 1 + 1 + } +} + +@resultBuilder +struct InvalidBuilder1 {} // expected-error {{result builder must provide at least one static 'buildBlock' method, or both 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)'}} + +@resultBuilder +struct InvalidBuilder2 { // expected-error {{result builder must provide at least one static 'buildBlock' method, or both 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)'}} + static func buildPartialBlock(first: Any) -> Any { fatalError() } +} +@resultBuilder +struct InvalidBuilder3 { // expected-error {{result builder must provide at least one static 'buildBlock' method, or both 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)'}} + static func buildPartialBlock(accumulated: Any, next: Any) -> Any { fatalError() } +} diff --git a/test/decl/var/result_builders_buildpartialblock_availability.swift b/test/decl/var/result_builders_buildpartialblock_availability.swift new file mode 100644 index 0000000000000..2ba893036416a --- /dev/null +++ b/test/decl/var/result_builders_buildpartialblock_availability.swift @@ -0,0 +1,28 @@ +// RUN: %target-typecheck-buildpartialblock-verify-swift + +// REQUIRES: macosx + +@resultBuilder +struct ValidBuilder2 { + @available(SwiftStdlib 5.6, *) + static func buildPartialBlock(first: Int) -> Int { fatalError() } + @available(SwiftStdlib 5.6, *) + static func buildPartialBlock(accumulated: Int, next: Int) -> Int { fatalError() } +} + +@available(SwiftStdlib 5.5, *) +func caller1_ValidBuilder2() { + // expected-error @+1 {{result builder 'ValidBuilder2' does not implement any 'buildBlock' or a combination of 'buildPartialBlock(first:)' and 'buildPartialBlock(accumulated:next:)' with sufficient availability for this call site}} + @ValidBuilder2 var x: Int { + 1 + 1 + 1 + } + if #available(SwiftStdlib 5.6, *) { + @ValidBuilder2 var y: Int { + 1 + 1 + 1 + } + } +} diff --git a/test/lit.cfg b/test/lit.cfg index e3a1189c1c94b..6612e65acce07 100644 --- a/test/lit.cfg +++ b/test/lit.cfg @@ -1968,6 +1968,10 @@ if 'swift_repl' in config.available_features: config.target_parse_verify_swift = ( '%s -typecheck -verify -disable-objc-attr-requires-foundation-module %%s' % (config.target_swift_frontend, )) +# Remove once `-enable-experimental-pairwise-build-block` is removed. +config.target_parse_buildpartialblock_verify_swift = ( + '%s -enable-experimental-pairwise-build-block -typecheck -verify -disable-objc-attr-requires-foundation-module %%s' + % (config.target_swift_frontend, )) config.target_sil_func_extractor = ( '%s -target %s %s' @@ -2211,6 +2215,8 @@ config.substitutions.append(('%target-msvc-runtime-opt', config.target_msvc_runtime_opt)) config.substitutions.append(('%target-typecheck-verify-swift', config.target_parse_verify_swift)) +# Remove once `-enable-experimental-pairwise-build-block` is removed. +config.substitutions.append(('%target-typecheck-buildpartialblock-verify-swift', config.target_parse_buildpartialblock_verify_swift)) config.substitutions.append(('%target-swift-emit-silgen\(mock-sdk:([^)]+)\)', SubstituteCaptures(r'%target-swift-frontend(mock-sdk:\1) -emit-silgen -verify-syntax-tree')))