[Sema] Implement basic type checking for pack expansion expressions. #61678
Conversation
|
Heads up, I merged #61657 so you can drop your cherry pick |
During prechecking, postfix '...' expressions are rewritten to pack expansion expressions if the operand contains pack references. References to packs are replaced with opaque values, and the pack expansion stores the opaque value bindings to decl refs.
hborla
force-pushed
the
pack-expansion-expression-checking
branch
2 times, most recently
from
8c4ef65 to
4488556
Compare
hborla
force-pushed
the
pack-expansion-expression-checking
branch
from
4488556 to
cf38d55
Compare
…ment inference. Previously, only the pattern type was added as a requirement inference source, but the pack expansion type is necessary for inferring same-shape requirements for packs that are expanded in parallel.
…s for pack expansion expressions.
|
|
||
| // CHECK-LABEL: expandedParameters(_:transform:) | ||
| // CHECK-NEXT: Generic signature: <T..., Result... where T.shape == Result.shape> | ||
| func expandedParameters<T..., Result...>(_ t: T..., transform: ((T) -> Result)...) -> (Result...) { |
There was a problem hiding this comment.
Can you add a third generic parameter and change the outer return type, to ensure that we don't equate the shapes of all three generic parameters?
|
@swift-ci please smoke test |
…Type::get when creating the generic signature for an opened pack element.
…ened element pattern types to pattern archetypes until after type variables are resolved.
|
@swift-ci please smoke test |
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS |
| @@ -2874,7 +2881,31 @@ namespace { | |||
| } | |||
|
|
|||
| Type visitPackExpansionExpr(PackExpansionExpr *expr) { | |||
| llvm_unreachable("not implemented for PackExpansionExpr"); | |||
| for (auto *binding : expr->getBindings()) { | |||
| auto type = visit(binding); | |||
There was a problem hiding this comment.
This might be an oversight - Traversal::visitPackExpansionExpr in ASTWalker.cpp needs to be updated to walk into bindings and re-set them, same applies to ASTDumper and ASTPrinter most likely.
There was a problem hiding this comment.
No, not visiting these in ASTWalker was intentional. This follows the same pattern as OpenExistentialExpr which only visits the subexpression containing opaque values, and the bindings are handled separately where needed. These aren't actually separate values in the AST, but we do need the types in the constraint system.
There was a problem hiding this comment.
Ok, we probably need a comment about this then... ASTDumper still needs to be updated to print bindings and opaque values just like OpenExistentialExpr does.
| llvm_unreachable("not implemented for PackExpansionExpr"); | ||
| for (unsigned i = 0; i < expr->getNumBindings(); ++i) { | ||
| auto *binding = expr->getBindings()[i]; | ||
| expr->setBinding(i, visit(binding)); |
There was a problem hiding this comment.
Can visit add any implicit AST nodes for binding? I think it might be safer to add cs.cacheTypes(<result>) in this case.
|
|
||
| // A ParenExpr can end up with a tuple type if it contains | ||
| // a pack expansion. Rewrite it to a TupleExpr. | ||
| if (dyn_cast<PackExpansionExpr>(expr->getSubExpr())) { |
There was a problem hiding this comment.
Nit: isa, or even better isExpr
There was a problem hiding this comment.
Thanks! I forgot about isExpr 🙂
| TypeMatchOptions flags, | ||
| ConstraintLocatorBuilder locator) { | ||
| auto elementType = simplifyType(first, flags); | ||
| auto *loc = getConstraintLocator(locator); |
There was a problem hiding this comment.
Nit: Might be better to materialize locator only when we are avoid to add unsolved constraint since the other case takes a builder anyway...
This change rewrites postfix
...operator expressions toPackExpansionExprwhen the operand contains pack references. Pack references are replaced withOpaqueValueExprs, and thePackExpansionExprstores the bindings from opaque values to pack references.This is currently done while pre-checking an expression before constraint generation, but this could later be moved into the solver by rewriting the postfix operator expression to an
UnresolvedExpansionOperatoror similar that generates a disjunction between the between postfix...and expansion operator, and delaying constraint generation for the operand until after a disjunction choice is attempted.