Skip to content

[4.1][CSSolver] Prioritize bindings with fewer default types #14538

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

Merged
merged 1 commit into from
Feb 12, 2018

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Feb 11, 2018

Explanation: With all else equal prioritize bindings with fewer defaultable types,
which always gets us closer to solution since defaultable bindings
mostly come from the collections and could be checked at the end.
This also makes sure that solver not as aggresive at assigning bindings
to trailing closures and allows solver to consider types which come
from inside the closure.
Scope of Issue: Affects logic related to picking bindings for type variables in constraint solver.
Origination: Caused by the performance work related to attempting literal bindings earlier than before which fixes performances related to multiple chained literal array declarations.
Risk: Low risk; Fixes a bug in constraint solver.
Reviewed By: @rudkx
Testing: Compiler regression tests
Radar / SR: rdar://problem/37290898

Resolves: rdar://problem/37290898
(cherry picked from commit aff4974)

With all else equal prioritize bindings with fewer defaultable types,
which always gets us closer to solution since defaultable bindings
mostly come from the collections and could be checked at the end.
This also makes sure that solver not as aggresive at assigning bindings
to trailing closures and allows solver to consider types which come
from inside the closure.

Resolves: rdar://problem/37290898
(cherry picked from commit aff4974)
@xedin xedin requested a review from rudkx February 11, 2018 06:11
@xedin
Copy link
Contributor Author

xedin commented Feb 11, 2018

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Feb 11, 2018

@swift-ci please test source compatibility

Copy link
Contributor

@rudkx rudkx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@xedin
Copy link
Contributor Author

xedin commented Feb 11, 2018

@rudkx Thanks!

@xedin
Copy link
Contributor Author

xedin commented Feb 11, 2018

@swift-ci please nominate

@AnnaZaks AnnaZaks merged commit d82f5c6 into swiftlang:swift-4.1-branch Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants