Skip to content

Commit ed5984a

Browse files
authored
Merge pull request #64213 from xedin/rdar-104384604
[BuilderTransform] Verify that builder type has at least one accessible build{Partial}Block
2 parents c82d2a0 + 6b323f1 commit ed5984a

File tree

8 files changed

+173
-21
lines changed

8 files changed

+173
-21
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6629,7 +6629,21 @@ ERROR(cannot_declare_computed_var_in_result_builder,none,
66296629
ERROR(cannot_convert_result_builder_result_to_return_type,none,
66306630
"cannot convert result builder result type %0 to return type %1",
66316631
(Type,Type))
6632-
6632+
ERROR(result_builder_buildblock_not_accessible,none,
6633+
"result builder must provide at least one static 'buildBlock' as "
6634+
"accessible as result builder type %0 "
6635+
"(which is %select{private|fileprivate|internal|package|public|open}1)",
6636+
(DeclName, AccessLevel))
6637+
ERROR(result_builder_buildpartialblock_first_not_accessible,none,
6638+
"result builder must provide at least one static 'buildPartialBlock(first:)' "
6639+
"as accessible as result builder type %0 "
6640+
"(which is %select{private|fileprivate|internal|package|public|open}1)",
6641+
(DeclName, AccessLevel))
6642+
ERROR(result_builder_buildpartialblock_accumulated_not_accessible,none,
6643+
"result builder must provide at least one static 'buildPartialBlock(accumulated:next:)' "
6644+
"as accessible as result builder type %0 "
6645+
"(which is %select{private|fileprivate|internal|package|public|open}1)",
6646+
(DeclName, AccessLevel))
66336647

66346648
//------------------------------------------------------------------------------
66356649
// MARK: Tuple Shuffle Diagnostics

lib/Sema/TypeCheckAttr.cpp

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3942,19 +3942,22 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) {
39423942
void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
39433943
auto *nominal = dyn_cast<NominalTypeDecl>(D);
39443944
auto &ctx = D->getASTContext();
3945-
SmallVector<ValueDecl *, 4> potentialMatches;
3945+
SmallVector<ValueDecl *, 4> buildBlockMatches;
3946+
SmallVector<ValueDecl *, 4> buildPartialBlockFirstMatches;
3947+
SmallVector<ValueDecl *, 4> buildPartialBlockAccumulatedMatches;
3948+
39463949
bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
39473950
nominal->getDeclaredType(), nominal, ctx.Id_buildBlock,
3948-
/*argLabels=*/{}, &potentialMatches);
3951+
/*argLabels=*/{}, &buildBlockMatches);
3952+
39493953
bool supportsBuildPartialBlock =
39503954
TypeChecker::typeSupportsBuilderOp(
3951-
nominal->getDeclaredType(), nominal,
3952-
ctx.Id_buildPartialBlock,
3953-
/*argLabels=*/{ctx.Id_first}, &potentialMatches) &&
3955+
nominal->getDeclaredType(), nominal, ctx.Id_buildPartialBlock,
3956+
/*argLabels=*/{ctx.Id_first}, &buildPartialBlockFirstMatches) &&
39543957
TypeChecker::typeSupportsBuilderOp(
3955-
nominal->getDeclaredType(), nominal,
3956-
ctx.Id_buildPartialBlock,
3957-
/*argLabels=*/{ctx.Id_accumulated, ctx.Id_next}, &potentialMatches);
3958+
nominal->getDeclaredType(), nominal, ctx.Id_buildPartialBlock,
3959+
/*argLabels=*/{ctx.Id_accumulated, ctx.Id_next},
3960+
&buildPartialBlockAccumulatedMatches);
39583961

39593962
if (!supportsBuildBlock && !supportsBuildPartialBlock) {
39603963
{
@@ -3968,7 +3971,7 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
39683971
Type componentType;
39693972
std::tie(buildInsertionLoc, stubIndent, componentType) =
39703973
determineResultBuilderBuildFixItInfo(nominal);
3971-
if (buildInsertionLoc.isValid() && potentialMatches.empty()) {
3974+
if (buildInsertionLoc.isValid() && buildBlockMatches.empty()) {
39723975
std::string fixItString;
39733976
{
39743977
llvm::raw_string_ostream out(fixItString);
@@ -3984,7 +3987,7 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
39843987

39853988
// For any close matches, attempt to explain to the user why they aren't
39863989
// valid.
3987-
for (auto *member : potentialMatches) {
3990+
for (auto *member : buildBlockMatches) {
39883991
if (member->isStatic() && isa<FuncDecl>(member))
39893992
continue;
39903993

@@ -3998,6 +4001,69 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
39984001
diagnose(member->getLoc(),
39994002
diag::result_builder_buildblock_not_static_method);
40004003
}
4004+
4005+
return;
4006+
}
4007+
4008+
// Let's check whether one or more overloads of buildBlock or
4009+
// buildPartialBlock are as accessible as the builder type itself.
4010+
{
4011+
auto isBuildMethodAsAccessibleAsType = [&](ValueDecl *member) {
4012+
return !isMemberLessAccessibleThanType(nominal, member);
4013+
};
4014+
4015+
bool hasAccessibleBuildBlock =
4016+
llvm::any_of(buildBlockMatches, isBuildMethodAsAccessibleAsType);
4017+
4018+
bool hasAccessibleBuildPartialBlockFirst = false;
4019+
bool hasAccessibleBuildPartialBlockAccumulated = false;
4020+
4021+
if (supportsBuildPartialBlock) {
4022+
DeclName buildPartialBlockFirst(ctx, ctx.Id_buildPartialBlock,
4023+
/*argLabels=*/{ctx.Id_first});
4024+
DeclName buildPartialBlockAccumulated(
4025+
ctx, ctx.Id_buildPartialBlock,
4026+
/*argLabels=*/{ctx.Id_accumulated, ctx.Id_next});
4027+
4028+
buildPartialBlockFirstMatches.clear();
4029+
buildPartialBlockAccumulatedMatches.clear();
4030+
4031+
auto builderType = nominal->getDeclaredType();
4032+
nominal->lookupQualified(builderType, DeclNameRef(buildPartialBlockFirst),
4033+
NL_QualifiedDefault,
4034+
buildPartialBlockFirstMatches);
4035+
nominal->lookupQualified(
4036+
builderType, DeclNameRef(buildPartialBlockAccumulated),
4037+
NL_QualifiedDefault, buildPartialBlockAccumulatedMatches);
4038+
4039+
hasAccessibleBuildPartialBlockFirst = llvm::any_of(
4040+
buildPartialBlockFirstMatches, isBuildMethodAsAccessibleAsType);
4041+
hasAccessibleBuildPartialBlockAccumulated = llvm::any_of(
4042+
buildPartialBlockAccumulatedMatches, isBuildMethodAsAccessibleAsType);
4043+
}
4044+
4045+
if (!hasAccessibleBuildBlock) {
4046+
// No or incomplete `buildPartialBlock` and all overloads of
4047+
// `buildBlock(_:)` are less accessible than the type.
4048+
if (!supportsBuildPartialBlock) {
4049+
diagnose(nominal->getLoc(),
4050+
diag::result_builder_buildblock_not_accessible,
4051+
nominal->getName(), nominal->getFormalAccess());
4052+
} else {
4053+
if (!hasAccessibleBuildPartialBlockFirst) {
4054+
diagnose(nominal->getLoc(),
4055+
diag::result_builder_buildpartialblock_first_not_accessible,
4056+
nominal->getName(), nominal->getFormalAccess());
4057+
}
4058+
4059+
if (!hasAccessibleBuildPartialBlockAccumulated) {
4060+
diagnose(
4061+
nominal->getLoc(),
4062+
diag::result_builder_buildpartialblock_accumulated_not_accessible,
4063+
nominal->getName(), nominal->getFormalAccess());
4064+
}
4065+
}
4066+
}
40014067
}
40024068
}
40034069

test/IDE/print_swift_module.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public func returnsAlias() -> Alias<Int> {
2929

3030
@resultBuilder
3131
public struct BridgeBuilder {
32-
static func buildBlock(_: Any...) {}
32+
public static func buildBlock(_: Any...) {}
3333
}
3434

3535
public struct City {

test/NameLookup/Inputs/custom_attrs_A.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ public struct Wrapper<Value> {
99

1010
@resultBuilder
1111
public struct Builder {
12-
static func buildBlock<T>(_ component: T) -> T { component }
12+
public static func buildBlock<T>(_ component: T) -> T { component }
1313
}

test/NameLookup/Inputs/custom_attrs_B.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ public struct Wrapper<Value> {
99

1010
@resultBuilder
1111
public struct Builder {
12-
static func buildBlock<T>(_ component: T) -> T { component }
12+
public static func buildBlock<T>(_ component: T) -> T { component }
1313
}

test/Serialization/Inputs/def_macros.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public struct TestMacroArgTypechecking {
2727

2828
@resultBuilder
2929
public struct Builder {
30-
static func buildBlock(_: Int...) -> Void {}
30+
public static func buildBlock(_: Int...) -> Void {}
3131
}
3232
@freestanding(expression)
3333
public macro macroWithBuilderArgs(@Builder _: () -> Void) = #externalMacro(module: "A", type: "B")

test/attr/attr_function_builder.swift

Lines changed: 0 additions & 6 deletions
This file was deleted.

test/attr/attr_result_builder.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
@_functionBuilder // expected-warning{{'@_functionBuilder' has been renamed to '@resultBuilder'}}{{2-18=resultBuilder}}
4+
struct MyBuilder {
5+
static func buildBlock(_: Any...) -> Any { }
6+
}
7+
8+
// rdar://104384604 - empty result builder in swiftinterface file
9+
@resultBuilder
10+
public struct TestInvalidBuildBlock1 {
11+
// expected-error@-1 {{result builder must provide at least one static 'buildBlock' as accessible as result builder type 'TestInvalidBuildBlock1' (which is public)}}
12+
static func buildBlock(_: Int) -> Int { 42 }
13+
}
14+
15+
@resultBuilder
16+
public struct TestInvalidBuildBlock2 { // Ok
17+
static func buildBlock(_: Int) -> Int { 42 }
18+
public static func buildBlock(_: String) -> String { "" }
19+
}
20+
21+
@resultBuilder
22+
public struct TestInvalidBuildPartialBlockFirst1 {
23+
// expected-error@-1 {{result builder must provide at least one static 'buildPartialBlock(first:)' as accessible as result builder type 'TestInvalidBuildPartialBlockFirst1' (which is public)}}
24+
static func buildPartialBlock(first: Int) -> Int { first }
25+
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
26+
}
27+
28+
@resultBuilder
29+
public struct TestInvalidBuildPartialBlockFirst2 { // Ok
30+
static func buildPartialBlock(first: Int) -> Int { first }
31+
public static func buildPartialBlock<T>(first: T) -> T { first }
32+
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
33+
}
34+
35+
@resultBuilder
36+
public struct TestInvalidBuildPartialBlockAccumulated1 {
37+
// expected-error@-1 {{result builder must provide at least one static 'buildPartialBlock(accumulated:next:)' as accessible as result builder type 'TestInvalidBuildPartialBlockAccumulated1' (which is public)}}
38+
public static func buildPartialBlock(first: Int) -> Int { first }
39+
private static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
40+
}
41+
42+
@resultBuilder
43+
public struct TestInvalidBuildPartialBlockAccumulated2 { // Ok
44+
public static func buildPartialBlock<T>(first: T) -> T { first }
45+
public static func buildPartialBlock<T>(accumulated: T, next: T) -> T { fatalError() }
46+
47+
private static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
48+
}
49+
50+
@resultBuilder
51+
public struct TestBuildPartialBlock1 { // Ok
52+
public static func buildPartialBlock(first: Int) -> Int { first }
53+
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
54+
}
55+
56+
@resultBuilder
57+
public struct TestBuildPartialBlock2 { // Ok
58+
private static func buildBlock(_: Int) -> Int { 42 }
59+
60+
public static func buildPartialBlock(first: Int) -> Int { first }
61+
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
62+
}
63+
64+
@resultBuilder
65+
public struct TestBuildPartialBlock3 { // Ok
66+
public static func buildBlock(_: Int) -> Int { 42 }
67+
68+
fileprivate static func buildPartialBlock(first: Int) -> Int { first }
69+
fileprivate static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
70+
}
71+
72+
@resultBuilder
73+
public struct TestBuildPartialBlock4 { // Ok
74+
public static func buildBlock(_: Int) -> Int { 42 }
75+
76+
public static func buildPartialBlock(first: Int) -> Int { first }
77+
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
78+
}

0 commit comments

Comments
 (0)