Skip to content

Commit 19cfe21

Browse files
committed
[Function builders] Add Fix-Its for missing build* members in builders.
When a use of a function builder involves a statement kind that the function builder doesn't support (e.g., if-else), add a note to the diagnostic that specifies what methods need to be added to the function builder to support that statement, including Fix-Its with stub implementations.
1 parent b137dde commit 19cfe21

File tree

3 files changed

+186
-2
lines changed

3 files changed

+186
-2
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5312,6 +5312,16 @@ NOTE(function_builder_buildblock_enum_case, none,
53125312
NOTE(function_builder_buildblock_not_static_method, none,
53135313
"potential match 'buildBlock' is not a static method", ())
53145314

5315+
NOTE(function_builder_missing_build_optional, none,
5316+
"add 'buildOptional(_:)' to the function builder %0 to add support for "
5317+
"'if' statements without an 'else'", (Type))
5318+
NOTE(function_builder_missing_build_either, none,
5319+
"add 'buildEither(first:)' and `buildEither(second:)' to the function "
5320+
"builder %0 to add support for 'if'-'else' and 'switch'", (Type))
5321+
NOTE(function_builder_missing_build_array, none,
5322+
"add 'buildArray(_:)' to the function builder %0 to add support for "
5323+
"'for'..'in' loops", (Type))
5324+
53155325
//------------------------------------------------------------------------------
53165326
// MARK: Tuple Shuffle Diagnostics
53175327
//------------------------------------------------------------------------------

lib/Sema/CSDiagnostics.cpp

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5490,12 +5490,170 @@ SourceLoc SkipUnhandledConstructInFunctionBuilderFailure::getLoc() const {
54905490
return unhandled.get<Decl *>()->getLoc();
54915491
}
54925492

5493+
/// Determine whether the given "if" chain has a missing "else".
5494+
static bool hasMissingElseInChain(IfStmt *ifStmt) {
5495+
if (!ifStmt->getElseStmt())
5496+
return true;
5497+
5498+
if (auto ifElse = dyn_cast<IfStmt>(ifStmt->getElseStmt()))
5499+
return hasMissingElseInChain(ifElse);
5500+
5501+
return false;
5502+
}
5503+
5504+
namespace {
5505+
enum class MissingBuildFunction {
5506+
BuildOptional,
5507+
BuildEitherFirst,
5508+
BuildEitherSecond,
5509+
BuildArray,
5510+
};
5511+
}
5512+
5513+
/// Add code for a missing 'build' function to a function builder.
5514+
static void printMissingBuildFunctionCode(
5515+
SourceLoc buildInsertionLoc, NominalTypeDecl *builder,
5516+
MissingBuildFunction missing, llvm::raw_ostream &out) {
5517+
// Determine the component type from the builder itself, if we can.
5518+
ASTContext &ctx = builder->getASTContext();
5519+
Type componentType;
5520+
bool isPublic = false;
5521+
{
5522+
SmallVector<ValueDecl *, 4> potentialMatches;
5523+
bool supportsBuildBlock = TypeChecker::typeSupportsBuilderOp(
5524+
builder->getDeclaredInterfaceType(), builder, ctx.Id_buildBlock,
5525+
/*argLabels=*/{}, &potentialMatches);
5526+
if (supportsBuildBlock) {
5527+
for (auto decl : potentialMatches) {
5528+
auto func = dyn_cast<FuncDecl>(decl);
5529+
if (!func || !func->isStatic())
5530+
continue;
5531+
5532+
// If we haven't seen a component type before, gather it.
5533+
if (!componentType) {
5534+
componentType = func->getResultInterfaceType();
5535+
isPublic = func->getFormalAccess() == AccessLevel::Public;
5536+
continue;
5537+
}
5538+
5539+
// If there are inconsistent component types, bail out.
5540+
if (!componentType->isEqual(func->getResultInterfaceType())) {
5541+
componentType = Type();
5542+
isPublic = false;
5543+
break;
5544+
}
5545+
}
5546+
}
5547+
}
5548+
5549+
// Render the component type into a string.
5550+
std::string componentTypeString;
5551+
if (componentType)
5552+
componentTypeString = componentType.getString();
5553+
else
5554+
componentTypeString = "<#Component#>";
5555+
5556+
// Render the code.
5557+
StringRef extraIndent;
5558+
StringRef currentIndent = Lexer::getIndentationForLine(
5559+
ctx.SourceMgr, buildInsertionLoc, &extraIndent);
5560+
std::string stubIndent = (currentIndent + extraIndent).str();
5561+
5562+
ExtraIndentStreamPrinter printer(out, stubIndent);
5563+
printer.printNewline();
5564+
5565+
if (isPublic)
5566+
printer << "public ";
5567+
5568+
printer << "static func ";
5569+
switch (missing) {
5570+
case MissingBuildFunction::BuildOptional:
5571+
printer << "buildOptional(_ component: " << componentTypeString << "?)";
5572+
break;
5573+
5574+
case MissingBuildFunction::BuildEitherFirst:
5575+
printer << "buildEither(first component: " << componentTypeString << ")";
5576+
break;
5577+
5578+
case MissingBuildFunction::BuildEitherSecond:
5579+
printer << "buildEither(second component: " << componentTypeString << ")";
5580+
break;
5581+
5582+
case MissingBuildFunction::BuildArray:
5583+
printer << "buildArray(_ components: [" << componentTypeString << "])";
5584+
break;
5585+
}
5586+
5587+
printer << " -> " << componentTypeString << " {";
5588+
printer.printNewline();
5589+
printer << " <#code#>";
5590+
printer.printNewline();
5591+
printer << "}";
5592+
}
5593+
54935594
void SkipUnhandledConstructInFunctionBuilderFailure::diagnosePrimary(
54945595
bool asNote) {
5495-
if (unhandled.is<Stmt *>()) {
5596+
if (auto stmt = unhandled.dyn_cast<Stmt *>()) {
54965597
emitDiagnostic(asNote ? diag::note_function_builder_control_flow
54975598
: diag::function_builder_control_flow,
54985599
builder->getName());
5600+
5601+
// Emit custom notes to help the user introduce the appropriate 'build'
5602+
// functions.
5603+
SourceLoc buildInsertionLoc = builder->getBraces().Start;
5604+
if (buildInsertionLoc.isValid()) {
5605+
buildInsertionLoc = Lexer::getLocForEndOfToken(
5606+
getASTContext().SourceMgr, buildInsertionLoc);
5607+
}
5608+
5609+
if (buildInsertionLoc.isInvalid()) {
5610+
// Do nothing.
5611+
} else if (isa<IfStmt>(stmt) && hasMissingElseInChain(cast<IfStmt>(stmt))) {
5612+
auto diag = emitDiagnosticAt(
5613+
builder->getLoc(), diag::function_builder_missing_build_optional,
5614+
builder->getDeclaredInterfaceType());
5615+
5616+
std::string fixItString;
5617+
{
5618+
llvm::raw_string_ostream out(fixItString);
5619+
printMissingBuildFunctionCode(
5620+
buildInsertionLoc, builder, MissingBuildFunction::BuildOptional,
5621+
out);
5622+
}
5623+
5624+
diag.fixItInsert(buildInsertionLoc, fixItString);
5625+
} else if (isa<SwitchStmt>(stmt) || isa<IfStmt>(stmt)) {
5626+
auto diag = emitDiagnosticAt(
5627+
builder->getLoc(), diag::function_builder_missing_build_either,
5628+
builder->getDeclaredInterfaceType());
5629+
5630+
std::string fixItString;
5631+
{
5632+
llvm::raw_string_ostream out(fixItString);
5633+
printMissingBuildFunctionCode(
5634+
buildInsertionLoc, builder, MissingBuildFunction::BuildEitherFirst,
5635+
out);
5636+
out << '\n';
5637+
printMissingBuildFunctionCode(
5638+
buildInsertionLoc, builder, MissingBuildFunction::BuildEitherSecond,
5639+
out);
5640+
}
5641+
5642+
diag.fixItInsert(buildInsertionLoc, fixItString);
5643+
} else if (isa<ForEachStmt>(stmt)) {
5644+
auto diag = emitDiagnosticAt(
5645+
builder->getLoc(), diag::function_builder_missing_build_array,
5646+
builder->getDeclaredInterfaceType());
5647+
5648+
std::string fixItString;
5649+
{
5650+
llvm::raw_string_ostream out(fixItString);
5651+
printMissingBuildFunctionCode(
5652+
buildInsertionLoc, builder, MissingBuildFunction::BuildArray, out);
5653+
}
5654+
5655+
diag.fixItInsert(buildInsertionLoc, fixItString);
5656+
}
54995657
} else {
55005658
emitDiagnostic(asNote ? diag::note_function_builder_decl
55015659
: diag::function_builder_decl,

test/Constraints/function_builder_diags.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ struct TupleBuilder { // expected-note 2 {{struct 'TupleBuilder' declared here}}
4545
}
4646

4747
@_functionBuilder
48-
struct TupleBuilderWithoutIf { // expected-note {{struct 'TupleBuilderWithoutIf' declared here}}
48+
struct TupleBuilderWithoutIf { // expected-note 3{{struct 'TupleBuilderWithoutIf' declared here}}
49+
// expected-note@-1{{add 'buildOptional(_:)' to the function builder 'TupleBuilderWithoutIf' to add support for 'if' statements without an 'else'}}{{31-31=\n static func buildOptional(_ component: <#Component#>?) -> <#Component#> {\n <#code#>\n \}}}
50+
// expected-note@-2{{add 'buildEither(first:)' and `buildEither(second:)' to the function builder 'TupleBuilderWithoutIf' to add support for 'if'-'else' and 'switch'}}{{31-31=\n static func buildEither(first component: <#Component#>) -> <#Component#> {\n <#code#>\n \}\n\n static func buildEither(second component: <#Component#>) -> <#Component#> {\n <#code#>\n \}}}
51+
// expected-note@-3{{add 'buildArray(_:)' to the function builder 'TupleBuilderWithoutIf' to add support for 'for'..'in' loops}}{{31-31=\n static func buildArray(_ components: [<#Component#>]) -> <#Component#> {\n <#code#>\n \}}}
4952
static func buildBlock() -> () { }
5053

5154
static func buildBlock<T1>(_ t1: T1) -> T1 {
@@ -106,6 +109,19 @@ func testDiags() {
106109
"hello"
107110
}
108111
}
112+
113+
tuplifyWithoutIf(true) {
114+
if $0 { // expected-error{{closure containing control flow statement cannot be used with function builder 'TupleBuilderWithoutIf'}}
115+
"hello"
116+
} else {
117+
}
118+
}
119+
120+
tuplifyWithoutIf(true) { a in
121+
for x in 0..<100 { // expected-error{{closure containing control flow statement cannot be used with function builder 'TupleBuilderWithoutIf'}}
122+
x
123+
}
124+
}
109125
}
110126

111127
struct A { }

0 commit comments

Comments
 (0)