From 1e012fea056ceabba6356dfcab20ed0d70128d79 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 11 Aug 2025 11:19:14 +0100 Subject: [PATCH 1/4] [CS] Avoid bailing on result builder `return` for completion Make sure we continue to solve the body normally rather than bailing early. --- lib/Sema/BuilderTransform.cpp | 5 +++-- test/IDE/complete_in_result_builder.swift | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 5d9cd6a9b6a47..df55eab9f0940 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1132,8 +1132,9 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType, if (fn.bodyHasExplicitReturnStmt()) { // Diagnostic mode means that solver couldn't reach any viable // solution, so let's diagnose presence of a `return` statement - // in the closure body. - if (shouldAttemptFixes()) { + // in the closure body. Avoid doing this for completion since we need to + // continue solving the body. + if (shouldAttemptFixes() && !isForCodeCompletion()) { if (recordFix(IgnoreResultBuilderWithReturnStmts::create( *this, builderType, getConstraintLocator(fn.getAbstractClosureExpr())))) diff --git a/test/IDE/complete_in_result_builder.swift b/test/IDE/complete_in_result_builder.swift index 368b914b636b1..1d16fbb108c96 100644 --- a/test/IDE/complete_in_result_builder.swift +++ b/test/IDE/complete_in_result_builder.swift @@ -16,7 +16,10 @@ struct TupleBuilder { } } -func buildStringTuple(@TupleBuilder x: () -> Result) {} +@discardableResult +func buildStringTuple(@TupleBuilder x: () -> Result) -> Result { + x() +} enum StringFactory { static func makeString(x: String) -> String { return x } @@ -98,6 +101,21 @@ struct FooStruct { func intGen() -> Int { return 1 } } +func testReturn() { + // Make sure we can still complete even with return. + buildStringTuple { + StringFactory.#^COMPLETE_STATIC_MEMBER_WITH_RETURN^# + // COMPLETE_STATIC_MEMBER_WITH_RETURN: Decl[StaticMethod]/CurrNominal: makeString({#x: String#})[#String#]; + return "" + } + + buildStringTuple { + "" + return FooStruct() + }.#^COMPLETE_INSTANCE_MEMBER_ON_RETURN_BUILDER^# + // COMPLETE_INSTANCE_MEMBER_ON_RETURN_BUILDER: Decl[InstanceVar]/CurrNominal: instanceVar[#Int#]; name=instanceVar +} + func testPatternMatching() { @TupleBuilder var x1 { let x = Letters.b From 931289988ed4ad63bb21e6527409adf5f060c875 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 11 Aug 2025 11:19:14 +0100 Subject: [PATCH 2/4] [CS] NFC: Remove an unused return value `isBuildableIfChainRecursive` never returned anything other than `true`, change it to just be a void call. --- lib/Sema/BuilderTransform.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index df55eab9f0940..92b95d1ec8b23 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -780,8 +780,9 @@ class ResultBuilderTransform #undef UNSUPPORTED_STMT private: - static bool isBuildableIfChainRecursive(IfStmt *ifStmt, unsigned &numPayloads, - bool &isOptional) { + static void checkBuildableIfChainRecursive(IfStmt *ifStmt, + unsigned &numPayloads, + bool &isOptional) { // The 'then' clause contributes a payload. ++numPayloads; @@ -789,7 +790,7 @@ class ResultBuilderTransform if (auto elseStmt = ifStmt->getElseStmt()) { // If it's 'else if', it contributes payloads recursively. if (auto elseIfStmt = dyn_cast(elseStmt)) { - return isBuildableIfChainRecursive(elseIfStmt, numPayloads, isOptional); + checkBuildableIfChainRecursive(elseIfStmt, numPayloads, isOptional); // Otherwise it's just the one. } else { ++numPayloads; @@ -799,8 +800,6 @@ class ResultBuilderTransform } else { isOptional = true; } - - return true; } static bool hasUnconditionalElse(IfStmt *ifStmt) { @@ -815,8 +814,7 @@ class ResultBuilderTransform bool isBuildableIfChain(IfStmt *ifStmt, unsigned &numPayloads, bool &isOptional) { - if (!isBuildableIfChainRecursive(ifStmt, numPayloads, isOptional)) - return false; + checkBuildableIfChainRecursive(ifStmt, numPayloads, isOptional); // If there's a missing 'else', we need 'buildOptional' to exist. if (isOptional && !builder.supportsOptional()) From e0791bb38dd6d95fc1dfafeafc5c0bfe8ad5d826 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 11 Aug 2025 11:19:14 +0100 Subject: [PATCH 3/4] [CS] Fix check in `applyResultBuilderBodyTransform` If `matchResultBuilder` returns `nullopt` we should be bailing, rather than continuing, which would attempt to solve with an empty constraint system. --- lib/Sema/BuilderTransform.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 92b95d1ec8b23..3bd74d9aee4af 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -989,11 +989,11 @@ TypeChecker::applyResultBuilderBodyTransform(FuncDecl *func, Type builderType) { // of this decl; it's not part of the interface type. builderType = func->mapTypeIntoContext(builderType); - if (auto result = cs.matchResultBuilder( - func, builderType, resultContextType, resultConstraintKind, - /*contextualType=*/Type(), - cs.getConstraintLocator(func->getBody()))) { - if (result->isFailure()) + { + auto result = cs.matchResultBuilder( + func, builderType, resultContextType, resultConstraintKind, + /*contextualType=*/Type(), cs.getConstraintLocator(func->getBody())); + if (!result || result->isFailure()) return nullptr; } From 27e2514effc4f86caeaf567942f71884fd1b0c68 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 11 Aug 2025 11:19:14 +0100 Subject: [PATCH 4/4] [CS] Better handle unsupported result builder elements for completion Instead of failing, fall back to solving as a regular function body. rdar://144303920 --- lib/Sema/BuilderTransform.cpp | 22 ++++++---------- test/IDE/complete_in_result_builder.swift | 32 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 3bd74d9aee4af..65e3711e4668e 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -269,12 +269,7 @@ class ResultBuilderTransform for (auto element : braceStmt->getElements()) { if (auto unsupported = transformBraceElement(element, newBody, buildBlockArguments)) { - // When in code completion mode, simply ignore unsported constructs to - // get results for anything that's unrelated to the unsupported - // constructs. - if (!ctx.CompletionCallback) { - return failTransform(*unsupported); - } + return failTransform(*unsupported); } } @@ -1156,18 +1151,17 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType, auto *body = transform.apply(fn.getBody()); if (auto unsupported = transform.getUnsupportedElement()) { - assert(!body || getASTContext().CompletionCallback); - - // If we aren't supposed to attempt fixes, fail. - if (!shouldAttemptFixes()) { - return getTypeMatchFailure(locator); - } + assert(!body); // If we're solving for code completion and the body contains the code - // completion location, skipping it won't get us to a useful solution so - // just bail. + // completion location, fall back to solving as a regular function body. if (isForCodeCompletion() && containsIDEInspectionTarget(fn.getBody())) { + return std::nullopt; + } + + // If we aren't supposed to attempt fixes, fail. + if (!shouldAttemptFixes()) { return getTypeMatchFailure(locator); } diff --git a/test/IDE/complete_in_result_builder.swift b/test/IDE/complete_in_result_builder.swift index 1d16fbb108c96..aa095c1973db3 100644 --- a/test/IDE/complete_in_result_builder.swift +++ b/test/IDE/complete_in_result_builder.swift @@ -53,6 +53,12 @@ func testGlobalLookup() { } } + buildStringTuple { + if { + #^GLOBAL_LOOKUP_IN_IF_BODY_WITHOUT_CONDITION_CLOSURE?check=GLOBAL_LOOKUP_IN_IF_BODY^# + } + } + @TupleBuilder var x4 { guard else { #^GLOBAL_LOOKUP_IN_GUARD_BODY_WITHOUT_CONDITION?check=GLOBAL_LOOKUP_IN_IF_BODY^# @@ -146,6 +152,11 @@ func testPatternMatching() { let x: FooStruct? = FooStruct() guard case .#^GUARD_CASE_PATTERN_2?check=OPTIONAL_FOOSTRUCT^#some() = x {} } + + buildStringTuple { + let x: FooStruct? = FooStruct() + guard case .#^GUARD_CASE_PATTERN_3?check=OPTIONAL_FOOSTRUCT^#some() = x {} + } } func testCompleteFunctionArgumentLabels() { @@ -428,3 +439,24 @@ func testInForLoop(_ x: [Int]) { } } } + +func testUnsupportedForLoop() { + @resultBuilder + struct Builder { + static func buildBlock(_ components: T...) -> T { + components.first! + } + } + + func foo(@Builder fn: () -> Int) {} + + foo { + for i in [0].#^COMPLETE_IN_UNSUPPORTED_FOR^# {} + } + // COMPLETE_IN_UNSUPPORTED_FOR: Decl[InstanceVar]/Super/IsSystem: first[#Int?#]; name=first + + foo { + for i in [0].#^COMPLETE_IN_UNSUPPORTED_FOR_2?check=COMPLETE_IN_UNSUPPORTED_FOR^# {} + return 0 + } +}