-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[ConstraintSystem] Delay constraint generation for single-statement closure body #28837
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
Conversation
@swift-ci Please Build Toolchain macOS Platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCalleeLocator
changes look good to me!
@swift-ci please test source compatibility |
1 similar comment
@swift-ci please test source compatibility |
TypeMatchOptions flags, ConstraintLocatorBuilder locator) { | ||
closureType = getFixedTypeRecursive(closureType, flags, /*wantRValue=*/true); | ||
|
||
if (closureType->isTypeVariableOrMember()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it makes sense to add hasTypeVariable()
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so because we really want to keep this constraint until the closure is resolved and the type it's bound to can (and probably would) have type variables. This constraint serves dual purpose - source of type for the closure when there is not contextual information available e.g. _ = { $0 + 1 }
and to connect closure type variable to its inferred parameters and result type + any outer parameters used in the body until the closure is resolved. When closure type variable becomes bound to inferred type (closure is "resolved") everything becomes connected, so DefaultClosureType
is obsolete from that moment on. I was thinking instead of DefaultCloureType
to go with InferredClosureType
, maybe that's a better name...
bdd00b7
to
df0ffea
Compare
@swift-ci please test source compatibility |
@swift-ci please smoke test compiler performance |
@swift-ci please test source compatibility |
@swift-ci please test compiler performance |
1 similar comment
@swift-ci please test compiler performance |
@swift-ci please test source compatibility debug |
1 similar comment
@swift-ci please test source compatibility debug |
@swift-ci please test source compatibility release |
Summary for master fullUnexpected test results, excluded stats for RxCocoa, SwifterSwift, Base64CoderSwiftUI Regressions found (see below) Debug-batchdebug-batch briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
debug-batch detailedRegressed (4)
Improved (5)
Unchanged (delta < 1.0% or delta < 100.0ms) (199)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
release detailedRegressed (1)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (17)
|
I'm not sure why but some of the builds where UPASS which I think contributed to the difference in number of constraint scopes, but I think that's okay because it didn't contribute to build time... |
@DougGregor @hborla This is ready for a final review. I'm going to run CI meanwhile. |
@swift-ci please test |
…y to closure result type
…or ranking Instead of trying to hold a "global" set of type variable differences let's use pair-wise comparison instead because in presence of generic overloads such would be more precise. If there are N solutions for a single generic overload we currently relied on "local" comparison to detect the difference, but it's not always possible to split the system to do one. Which means higher level comparisons have to account for "local" (per overload choice) differences as well otherwise ranking would loose precision.
…member overloads Single type of keypath dynamic member lookup could refer to different member overlaods, we have to do a pair-wise comparison in such cases otherwise ranking would miss some viable information e.g. `_ = arr[0..<3]` could refer to subscript through writable or read-only key path and each of them could also pick overload which returns `Slice<T>` or `ArraySlice<T>` (assuming that `arr` is something like `Box<[Int]>`).
…onstraint generation changes
…r (supertypes first) If there is a subtype relationship between N types we have to make sure that type chain is attempted in reverse order because we can't infer bindings transitively through type variables and attempting it in any other way would miss potential bindings across the chain.
@swift-ci please test |
@swift-ci please test source compatibility |
@swift-ci please smoke test compiler performance |
Summary for master smoketestNo regressions above thresholds Debugdebug briefRegressed (0)
Improved (2)
Unchanged (delta < 1.0% or delta < 100.0ms) (1)
debug detailedRegressed (0)
Improved (7)
Unchanged (delta < 1.0% or delta < 100.0ms) (11)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
release detailedRegressed (0)
Improved (1)
Unchanged (delta < 1.0% or delta < 100.0ms) (17)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome!
} | ||
|
||
ConstraintSystem &getConstraintSystem() const { return CS; } | ||
|
||
void enterClosure(ClosureExpr *closure) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love that this goes away!
@swift-ci please smoke test |
@swift-ci please test |
@swift-ci please test source compatibility |
Build failed |
Build failed |
Both multi- and single-statement closures now behave the same way when it comes to constraint generation, because the body is opened only once contextual type becomes available or it's impossible to find one e.g. `_ = { 1 }`.
@swift-ci please test |
@swift-ci please test source compatibility |
Alright, here we go :) |
Attempt to infer a closure type based on its parameters and body
and put it aside until contextual type becomes available or it
has been determined that there is no context.
Once all appropriate conditions are met, generate constraints for
the body of the closure and bind it the previously inferred type.
Doing so makes it possible to pass single-statement closures as
an argument to a function builder parameter.
Resolves: SR-11628
Resolves: rdar://problem/56340587