Reimplement function builders as statement transformations. #29133
Conversation
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
This is going to conflict with #28837, which should be merged first and I'll rebase/cleanup once that happens. This also needs some serious stress-testing on SwiftUI apps, so we can get some more complicated function builder uses. |
DougGregor
changed the title
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
A couple of trivial fixes as part of the builder transform refactoring: * When recording captured expressions, record the source entity appropriately * Make sure to check (and skip) #if declarations in application * Check and diagnostic #warning/#error as part of application (not generation) * Simplify the body result type for return coercion during application * When checking for applicability of a function builder, don't do any constraint generation.
When generating constraints in the context of the "old" diagnostic path that type-checks subexpressions independently, we might not have types for closure parameters. Cope with the case with a narrower form of the original hack.
If we encountered an error, just skip it; there's nothing more to do.
… applied. We used to get this contextualization "for free" because closures that had function builders applied to them would get translated into single-expression closures. Now, we need to check for this explicitly.
The right solution is to extend the notion of the "anchor" of a locator to also cover statements (and TypeReprs, and Patterns, and more), so this is a stop-gap.
DougGregor
force-pushed
the
generlize-function-builders
branch
from
05dda38 to
0a7f040
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test compiler performance |
| }; | ||
|
|
||
| /// A mapping from expressions whose values are captured by the builder | ||
| /// to information about the temporary variable capturing the |
There was a problem hiding this comment.
Nit: looks like the rest of the comment is missing here...
| // FIXME: Need a locator for the "if" statement. | ||
| Type resultType = cs->addJoinConstraint(nullptr, | ||
| { | ||
| { cs->getType(thenExpr), cs->getConstraintLocator(thenExpr) }, |
There was a problem hiding this comment.
using TernaryBranch(bool) locator element would help to get better diagnostics in there is a mismatch here just like in ConstraintGenerator::visitIfExpr.
There was a problem hiding this comment.
Okay, thank you! I'll do that in a follow-up PR.
There was a problem hiding this comment.
Bah, I still need an "anchor", which pretty much has to be the "if" statement for this to make sense. I'll leave it as-is for the moment.
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test |
|
Bah, I broke something. Reverting! |
Refactor the implementation of function builders so they maintain the statement structure of the closure/function body to which they are applied. This replaces the previous "fold everything into a single expression" implementation with one that is better in several regards:
buildBlock/buildExpression/buildIf/etc. invocation. The result is easier to reason about.if let, although each of these will require work.This refactoring is a major step toward generalizing function builders (rdar://problem/50426203).