Skip to content

[BuilderTransform] Verify that builder type has at least one accessible build{Partial}Block #64213

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
merged 3 commits into from
Mar 14, 2023
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
16 changes: 15 additions & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -6659,7 +6659,21 @@ ERROR(cannot_declare_computed_var_in_result_builder,none,
ERROR(cannot_convert_result_builder_result_to_return_type,none,
"cannot convert result builder result type %0 to return type %1",
(Type,Type))

ERROR(result_builder_buildblock_not_accessible,none,
"result builder must provide at least one static 'buildBlock' as "
"accessible as result builder type %0 "
"(which is %select{private|fileprivate|internal|package|public|open}1)",
(DeclName, AccessLevel))
ERROR(result_builder_buildpartialblock_first_not_accessible,none,
"result builder must provide at least one static 'buildPartialBlock(first:)' "
"as accessible as result builder type %0 "
"(which is %select{private|fileprivate|internal|package|public|open}1)",
(DeclName, AccessLevel))
ERROR(result_builder_buildpartialblock_accumulated_not_accessible,none,
"result builder must provide at least one static 'buildPartialBlock(accumulated:next:)' "
"as accessible as result builder type %0 "
"(which is %select{private|fileprivate|internal|package|public|open}1)",
(DeclName, AccessLevel))

//------------------------------------------------------------------------------
// MARK: Tuple Shuffle Diagnostics
Expand Down
86 changes: 76 additions & 10 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3938,19 +3938,22 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) {
void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
auto *nominal = dyn_cast<NominalTypeDecl>(D);
auto &ctx = D->getASTContext();
SmallVector<ValueDecl *, 4> potentialMatches;
SmallVector<ValueDecl *, 4> buildBlockMatches;
SmallVector<ValueDecl *, 4> buildPartialBlockFirstMatches;
SmallVector<ValueDecl *, 4> buildPartialBlockAccumulatedMatches;

bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
nominal->getDeclaredType(), nominal, ctx.Id_buildBlock,
/*argLabels=*/{}, &potentialMatches);
/*argLabels=*/{}, &buildBlockMatches);

bool supportsBuildPartialBlock =
TypeChecker::typeSupportsBuilderOp(
nominal->getDeclaredType(), nominal,
ctx.Id_buildPartialBlock,
/*argLabels=*/{ctx.Id_first}, &potentialMatches) &&
nominal->getDeclaredType(), nominal, ctx.Id_buildPartialBlock,
/*argLabels=*/{ctx.Id_first}, &buildPartialBlockFirstMatches) &&
TypeChecker::typeSupportsBuilderOp(
nominal->getDeclaredType(), nominal,
ctx.Id_buildPartialBlock,
/*argLabels=*/{ctx.Id_accumulated, ctx.Id_next}, &potentialMatches);
nominal->getDeclaredType(), nominal, ctx.Id_buildPartialBlock,
/*argLabels=*/{ctx.Id_accumulated, ctx.Id_next},
&buildPartialBlockAccumulatedMatches);

if (!supportsBuildBlock && !supportsBuildPartialBlock) {
{
Expand All @@ -3964,7 +3967,7 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
Type componentType;
std::tie(buildInsertionLoc, stubIndent, componentType) =
determineResultBuilderBuildFixItInfo(nominal);
if (buildInsertionLoc.isValid() && potentialMatches.empty()) {
if (buildInsertionLoc.isValid() && buildBlockMatches.empty()) {
std::string fixItString;
{
llvm::raw_string_ostream out(fixItString);
Expand All @@ -3980,7 +3983,7 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {

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

Expand All @@ -3994,6 +3997,69 @@ void AttributeChecker::visitResultBuilderAttr(ResultBuilderAttr *attr) {
diagnose(member->getLoc(),
diag::result_builder_buildblock_not_static_method);
}

return;
}

// Let's check whether one or more overloads of buildBlock or
// buildPartialBlock are as accessible as the builder type itself.
{
auto isBuildMethodAsAccessibleAsType = [&](ValueDecl *member) {
return !isMemberLessAccessibleThanType(nominal, member);
};

bool hasAccessibleBuildBlock =
llvm::any_of(buildBlockMatches, isBuildMethodAsAccessibleAsType);

bool hasAccessibleBuildPartialBlockFirst = false;
bool hasAccessibleBuildPartialBlockAccumulated = false;

if (supportsBuildPartialBlock) {
DeclName buildPartialBlockFirst(ctx, ctx.Id_buildPartialBlock,
/*argLabels=*/{ctx.Id_first});
DeclName buildPartialBlockAccumulated(
ctx, ctx.Id_buildPartialBlock,
/*argLabels=*/{ctx.Id_accumulated, ctx.Id_next});

buildPartialBlockFirstMatches.clear();
buildPartialBlockAccumulatedMatches.clear();

auto builderType = nominal->getDeclaredType();
nominal->lookupQualified(builderType, DeclNameRef(buildPartialBlockFirst),
NL_QualifiedDefault,
buildPartialBlockFirstMatches);
nominal->lookupQualified(
builderType, DeclNameRef(buildPartialBlockAccumulated),
NL_QualifiedDefault, buildPartialBlockAccumulatedMatches);

hasAccessibleBuildPartialBlockFirst = llvm::any_of(
buildPartialBlockFirstMatches, isBuildMethodAsAccessibleAsType);
hasAccessibleBuildPartialBlockAccumulated = llvm::any_of(
buildPartialBlockAccumulatedMatches, isBuildMethodAsAccessibleAsType);
}

if (!hasAccessibleBuildBlock) {
// No or incomplete `buildPartialBlock` and all overloads of
// `buildBlock(_:)` are less accessible than the type.
if (!supportsBuildPartialBlock) {
diagnose(nominal->getLoc(),
diag::result_builder_buildblock_not_accessible,
nominal->getName(), nominal->getFormalAccess());
} else {
if (!hasAccessibleBuildPartialBlockFirst) {
diagnose(nominal->getLoc(),
diag::result_builder_buildpartialblock_first_not_accessible,
nominal->getName(), nominal->getFormalAccess());
}

if (!hasAccessibleBuildPartialBlockAccumulated) {
diagnose(
nominal->getLoc(),
diag::result_builder_buildpartialblock_accumulated_not_accessible,
nominal->getName(), nominal->getFormalAccess());
}
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/IDE/print_swift_module.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public func returnsAlias() -> Alias<Int> {

@resultBuilder
public struct BridgeBuilder {
static func buildBlock(_: Any...) {}
public static func buildBlock(_: Any...) {}
}

public struct City {
Expand Down
2 changes: 1 addition & 1 deletion test/NameLookup/Inputs/custom_attrs_A.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ public struct Wrapper<Value> {

@resultBuilder
public struct Builder {
static func buildBlock<T>(_ component: T) -> T { component }
public static func buildBlock<T>(_ component: T) -> T { component }
}
2 changes: 1 addition & 1 deletion test/NameLookup/Inputs/custom_attrs_B.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ public struct Wrapper<Value> {

@resultBuilder
public struct Builder {
static func buildBlock<T>(_ component: T) -> T { component }
public static func buildBlock<T>(_ component: T) -> T { component }
}
2 changes: 1 addition & 1 deletion test/Serialization/Inputs/def_macros.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public struct TestMacroArgTypechecking {

@resultBuilder
public struct Builder {
static func buildBlock(_: Int...) -> Void {}
public static func buildBlock(_: Int...) -> Void {}
}
@freestanding(expression)
public macro macroWithBuilderArgs(@Builder _: () -> Void) = #externalMacro(module: "A", type: "B")
6 changes: 0 additions & 6 deletions test/attr/attr_function_builder.swift

This file was deleted.

78 changes: 78 additions & 0 deletions test/attr/attr_result_builder.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// RUN: %target-typecheck-verify-swift

@_functionBuilder // expected-warning{{'@_functionBuilder' has been renamed to '@resultBuilder'}}{{2-18=resultBuilder}}
struct MyBuilder {
static func buildBlock(_: Any...) -> Any { }
}

// rdar://104384604 - empty result builder in swiftinterface file
@resultBuilder
public struct TestInvalidBuildBlock1 {
// expected-error@-1 {{result builder must provide at least one static 'buildBlock' as accessible as result builder type 'TestInvalidBuildBlock1' (which is public)}}
static func buildBlock(_: Int) -> Int { 42 }
}

@resultBuilder
public struct TestInvalidBuildBlock2 { // Ok
static func buildBlock(_: Int) -> Int { 42 }
public static func buildBlock(_: String) -> String { "" }
}

@resultBuilder
public struct TestInvalidBuildPartialBlockFirst1 {
// expected-error@-1 {{result builder must provide at least one static 'buildPartialBlock(first:)' as accessible as result builder type 'TestInvalidBuildPartialBlockFirst1' (which is public)}}
static func buildPartialBlock(first: Int) -> Int { first }
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestInvalidBuildPartialBlockFirst2 { // Ok
static func buildPartialBlock(first: Int) -> Int { first }
public static func buildPartialBlock<T>(first: T) -> T { first }
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestInvalidBuildPartialBlockAccumulated1 {
// expected-error@-1 {{result builder must provide at least one static 'buildPartialBlock(accumulated:next:)' as accessible as result builder type 'TestInvalidBuildPartialBlockAccumulated1' (which is public)}}
public static func buildPartialBlock(first: Int) -> Int { first }
private static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestInvalidBuildPartialBlockAccumulated2 { // Ok
public static func buildPartialBlock<T>(first: T) -> T { first }
public static func buildPartialBlock<T>(accumulated: T, next: T) -> T { fatalError() }

private static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestBuildPartialBlock1 { // Ok
public static func buildPartialBlock(first: Int) -> Int { first }
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestBuildPartialBlock2 { // Ok
private static func buildBlock(_: Int) -> Int { 42 }

public static func buildPartialBlock(first: Int) -> Int { first }
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestBuildPartialBlock3 { // Ok
public static func buildBlock(_: Int) -> Int { 42 }

fileprivate static func buildPartialBlock(first: Int) -> Int { first }
fileprivate static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}

@resultBuilder
public struct TestBuildPartialBlock4 { // Ok
public static func buildBlock(_: Int) -> Int { 42 }

public static func buildPartialBlock(first: Int) -> Int { first }
public static func buildPartialBlock(accumulated: Int, next: Int) -> Int { accumulated + next }
}