[ConstraintSystem] Return of the new disjunction favoring/selection algorithm #79461
+3,311
−1,766
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test source compatibility |
|
@swift-ci please test |
|
@swift-ci please build toolchain |
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
f785515 to
21a3705
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
xedin
force-pushed
the
solver-perf-frankenstein
branch
2 times, most recently
from
f442ec8 to
6a4b7e7
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
367b059 to
37e3e32
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility |
|
Debug source compatibility failure is UPASS on Doggie which is expected. |
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
37e3e32 to
a7ac64a
Compare
xedin
force-pushed
the
solver-perf-frankenstein
branch
3 times, most recently
from
d593771 to
d8608a3
Compare
|
@swift-ci please test |
2 tasks
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
d8608a3 to
d156df4
Compare
|
@swift-ci please clean test |
|
@swift-ci please test source compatibility |
|
@swift-ci please test Windows platform |
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
d156df4 to
a806a05
Compare
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
a806a05 to
cc06572
Compare
…wiftlang#79128)" This reverts commit 725bd91.
… matching Result type should only be matched if there are matches on arguments or there are no viable candidates.
…ures If there are no-same type requirements and parameters use either concrete types or generic parameter types directly, the optimizer should be able to handle ranking. Currently candidate arguments are considered in isolation which makes it impossible to deal with same-type requirements and complex generic signatures.
… `nil` while ranking `==` and `!=` operators `==` and `!=` operators have special overloads that allow matching `nil` literal on either side even if wrapped type on the other side doesn't conform to `Equatable`.
…t` on `Dictionary` Just like a subscript on `Array` if key type matches the argument type exactly, the result of subscript operator should be a value type wrappend in an `Optional`.
…eneric paameter types For example passing `Int?` to `T` should be considered a match if `T` doesn't have any requirements that block it.
…nt behavior Thanks to `LinkedExprAnalyzer` unary argument hack was able to infer matching based on literals and arithmetic operator chains, let's preserve that behavior in a more principled manner.
…s based speculation New ranking + selection algorithm suffered from over-eagerly selecting operator disjunctions vs. unsupported non-operator ones even if the ranking was based purely on literal candidates. This change introduces a notion of a speculative candidate - one which has a type inferred from a literal or an initializer call that has failable overloads and/or implicit conversions (i.e. Double/CGFloat). `determineBestChoicesInContext` would reset the score of an operator disjunction which was computed based on speculative candidates alone but would preserve favoring information. This way selection algorithm would not be skewed towards operators and at the same time if there is no no choice by to select one we'd still have favoring information available which is important for operator chains that consist purely of literals.
…s are considered Some of the disjunctions are not supported by the optimizers but could still be a better choice than an operator. Using a non-score based preference mechanism first allows us to make sure that operator disjunctions are not selected too eagerly in some situations when i.e. a member (supported or not) could be a better choice. `isPreferable` currently targets only operators in result builder contexts but it could be expanded to more uses in the future.
…ameters and ternary expressions We need to have a notion of "complete" binding set before we can allow inference from generic parameters and ternary, otherwise we'd make a favoring decision that might not be correct i.e. `v ?? (<<cond>> ? nil : o)` where `o` is `Int`. `getBindingsFor` doesn't currently infer transitive bindings which means that for a ternary we'd only have a single binding - `Int` which could lead to favoring overload of `??` and has non-optional parameter on the right-hand side.
The test was slow with hacks but now it's much faster - takes about 63k scopes to solve, it could be improved by introducing new overloads of prefix `-` to stdlib.
…plyExpr`s The original hack never applied to subscripts.
…tors vs. non-operators The problem this is trying to solve is eager selection of operators over unsupported disjunctions, when matching operators let's take speculative information into account because it helps to make better choices in this case.
…arameter types When matching candidate like `[Int]` against `Array<Element>` we need to conservatively assume that if the nominals match the argument is a viable exact match because otherwise it's possible to skip some of the valid matches when other overload choice have generic parameters at the same parameter position.
…ments or variadic overloads This matches the behavior of the old hack where favoring choices were rolled back if `mustConsider` produced `true` which happened only for protocol requirements and variadic overload choice regardless of their viability.
Instead of checking both protocols, check one that matches best depending on where `String` literal candidate came from.
…ices are failable If all of the viable initializer overloads are failable, the only valid inference choice is an optional candidate type.
…hod which is printed at the end
…rator chains Most of them don't have any children but string interpolation does and that marks whole chain as unsupported.
These choices could be better than some other non-disfavored ones in certain situations i.e. when `async` overload is disfavored but appears in async context it's preferrable to a non-async overload choice. Note that the code that mimic old hacks still needs to filter on `@_disfavoredOverload` in few places to maintain source compatibility.
…ing to `isArray()`
…he call instead of callee `calleeFn` now returns the underlying declaration reference looking through `ConstructorRefCallExpr`, which means the downgrade logic needs to check whether the call is using initializer reference before making a decision.
xedin
force-pushed
the
solver-perf-frankenstein
branch
from
cc06572 to
02ab2dd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LinkedExprAnalyzerplaybook_OptionalNilComparisonTypeas candidate fornilwhile analyzing==and!=operatorsInt? -> T