Skip to content

[ConstraintSystem] Implement disjunction favoring algorithm behind a flag #82574

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 52 commits into from
Jun 30, 2025

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Jun 27, 2025

  • Partially revert [ConstraintSystem] Revert new disjunction favoring algorithm #79128 (all of the old hacks are left in place)
  • Include fixes from [CSOptimizer] A few improvements to inference and ranking #79309
  • Drop the score if analysis was done based on "speculative" candidate, to avoid over-eager selection of operators
  • Bring back analysis of subscripts on Dictionary type but only if the argument's type is known
  • Strengthen unary argument favoring behavior by taking a page from LinkedExprAnalyzer playbook
  • Introduce _OptionalNilComparisonType as candidate for nil while analyzing == and != operators
  • Fix a bug that didn't let optional types be matched against generic parameter types i.e. Int? -> T
  • Account for whether candidates are speculative or not when selecting disjunctions
  • Expand operator chain inference to cover previously resolved declaration references
  • Make collection subscript result type inference more principled
  • Enable new favoring algorithm by default
  • Introduce a new flag -enable-constraint-solver-performance-hacks to re-enable old behavior
  • Add a new block list action to enable the old behavior on a per-module basis

@xedin
Copy link
Contributor Author

xedin commented Jun 27, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Jun 27, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Jun 27, 2025

@swift-ci please test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jun 27, 2025

@swift-ci please test Linux platform

xedin added 14 commits June 27, 2025 23:43
…ings (v2)

The original attempt to do this was reverted by swiftlang#77653

The problem is that the fix was too broad, I narrowed it down to
non-exact uses of stdlib collections that support upcasts.

(cherry picked from commit b7e7493)
…injected into an optional

If result of `CGFloat` -> `Double` conversion is injected into an optional
it should be ranked based on depth just like when locator is fully simplified.

For example:

```swift
func test(v: CGFloat?) {
   _ = v ?? 2.0 / 3.0
}
```

In this expression division should be performed on `Double` and result
narrowed down (based on the rule that narrowing conversion should always
be delayed) but `Double` -> `CGFloat?` was given an incorrect score and
instead of picking `?? (_: T?, _: T) -> T` overload, the solver would
use `?? (_: T?, _: T?) -> T?`.

(cherry picked from commit cb876cb)
…ase is opaque result type

Previously only nominal type and existential where allowed but
that's outdated, it's possible to reference a member of an opaque
result type as well, the concrete type in this case is going to
be deduced by the solver.
This is currently unused because current mechanism set favored choices
directly but it would be utilized by the disjunction optimizer.

(cherry picked from commit e404ed7)
…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.
This also changes the flag to `-enable-constraint-solver-performance-hacks`.
…hen some of the arguments are number literals

Don't attempt this optimization if call has number literals.
This is intended to narrowly fix situations like:

```swift
func test<T: FloatingPoint>(_: T) { ... }
func test<T: Numeric>(_: T) { ... }

test(42)
```

The call should use `<T: Numeric>` overload even though the
`<T: FloatingPoint>` is a more specialized version because
selecting `<T: Numeric>` doesn't introduce non-default literal
types.

(cherry picked from commit 8d5cb11)
… matching

Result type should only be matched if there are matches on arguments
or there are no viable candidates.
For example, `??` operator could produce an optional type
so `test(<<something>> ?? 0) could result in an optional
argument that wraps a type variable. It should be possible
to infer bindings from underlying type variable and restore
optionality.
This used to be limited to Double/CGFloat and operator arguments
but it's safe to do in general.
xedin added 22 commits June 27, 2025 23:43
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.
…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.
This is helpful in situations when all of the chained operators
have literal arguments because it would make sure that every
operator has the same score if there is no contextual type.
… status

If the scores are the same and both disjunctions are operators
they could be ranked purely based on whether the candidates
were speculative or not. The one with more context always wins.

Consider the following situation:

```swift
func test(_: Int) { ... }
func test(_: String) { ... }

test("a" + "b" + "c")
```

In this case we should always prefer ... + "c" over "a" + "b"
because it would fail and prune the other overload if parameter
type (aka contextual type) is `Int`.
- Expand the inference to include prefix and postfix unary operators
- Recognize previously resolved declaration and member references
  in argument positions and record their types.
- Expand reconciliation logic from Double<->Int to include other
  floating-point types and `CGFloat`.
Infer result type of a subscript with Array or Dictionary base type
if argument type matches the key type exactly or it's a supported
literal type.

This helps to maintain the existing behavior without having to resort
to "favored type" computation.
Instead of checking `EnableConstraintSolverPerformanceHacks`
directly, let's use an option which is easier to set i.e.
when a block list is implemented.
…izer

Allow enabling performance hacks via a block list action - `ShouldUseTypeCheckerPerfHacks`
@xedin xedin force-pushed the solver-perf-behind-a-flag branch from 777a203 to 4591884 Compare June 28, 2025 07:19
@xedin
Copy link
Contributor Author

xedin commented Jun 28, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Jun 28, 2025

@swift-ci please test macOS platform

@xedin
Copy link
Contributor Author

xedin commented Jun 28, 2025

@swift-ci please test Windows platform

@xedin
Copy link
Contributor Author

xedin commented Jun 30, 2025

@swift-ci please smoke test macOS platform

@xedin xedin merged commit bec4ebd into swiftlang:main Jun 30, 2025
5 checks passed
xedin added a commit to xedin/swift-source-compat-suite that referenced this pull request Jun 30, 2025
Type-checking performance issue is fixed by swiftlang/swift#82574
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.

1 participant