In salvage(), there is a point where we're done solving, but we haven't
yet torn down ConstraintSystem::solverState, so the trail is still
active.
It was possible to cause a change to be recorded, because of the
path compression we do in TypeVariableType::getRepresentative().
We have a similar situation where we don't want to do path compression
when we're looking up a type variable's representative in the middle
of undo(). Generalize the existing UndoActive flag to Closed, and set
it after solving in salvage() as well.
While we're here, clean up an existing place where we would check
isUndoActive() to not do that anymore, so now getRepresentative() is
the only place that checks the state of the trail.
A better cleanup would be to try to refactor or eliminate SolverState
entirely, but this fix is pretty clean.
Note that the test cases are somewhat random because the exact scenario
is hard to trigger; you need to set up an invalid expression where the
type variables are set up in just the right way so that path compression
kicks in.
- Fixes rdar://152143989.
- Fixes https://github.com/swiftlang/swift/issues/81801.
- Fixes https://github.com/swiftlang/swift/issues/84884.
interleaveComma is a convenience wrapper that hardcodes ", " as the
separator. Use it in the 2 call sites in CSBindings.cpp where the
separator was being passed explicitly.
A type variable that represents the type of a closure can only be bound
to a function type, but this fact is not directly encoded in the
constraint system.
Check for the appearance of a non-sensical subtype binding on a closure
type variable in reduceBinding(), and promote the binding to exact as
soon as we detect this, since binding the type will always fail; we want
to fail as quickly as possible, before attempting any more disjunctions.
This is a generally good performance optimization, and it also addresses
a performance regression from "Sema: Filter bindings by considering
conformance constraints".
This also speeds up the expression from rdar://59008707, which also uses
Combine and is slow for similar reasons.
This is NFC. All it does is expand out some of the if statements
to run through all combinations of (existing.Kind, binding.Kind)
in order.
This is extremely verbose, but I'm going to factor out the
common parts and also add new checks for various conditions so
I think it will be clearer in the end.
In addition to the generation number, it is important to
recompute bindings when the constraint system transitions
from not doing salvage to salvage.
This fixes diagnostic regressions with an upcoming commit.
It looks like originally, we would only infer bindings from a
KeyPath type, which has invariant generic arguments. However,
nowadays we can infer a keypath root type from a function
type as well, because we allow keypath to function conversions.
So, if we inferred the key path root type through a function type,
then in fact we cannot introduce it as an exact binding, and
we must preserve variance.
This fixes a test failure that appears with a subsequent change
I'm making.
Code with Protocol Composition parameters had not been taking into account
that a parameter would conform to all protocols in the composition on
member resolution and could fail to find a member for one protocol
on static lookups, resulting in a diagnostic that was incorrect even on
correct programs.
For example
```protocol P {
static var boo
}
protocol Q {
}
func t<T: P&Q>(x: T) { }
t(.boo)```
Diagnostic of Q does not have member boo
(Yes, other diagnostics are required in this code)
This occured because P and Q were considered one path at a time.
After the change, the P&Q path is considered together instead.
We should delay binding the conjunction if any of the type variables it
references simplify to types that contain type variables as well.
Otherwise, if we have a result builder with a for-each loop over an Array
with a type variable element type, we might start solving the conjunction
before we bind the element type. This will fail.
If T has no subtypes, we have a constraint `$T0 conv T`, and
another constraint `$T1 optional with object type $T0`, we
must account for the possibility that $T0 is bound to
`@lvalue T`, if $T1 may be bound to an lvalue.
This was previously unsound, because if the type variable accepts
lvalue bindings, every type T has a subtype @lvalue T, and so in
some cases we would bind a type variable to an rvalue type too early.
We would do stuff in the case where the type variable appeared on
the right, but just drop the constraint if the type variable was
on the left. In the latter case, record it in the new
PotentialBindings::LValuesOf vector.
We would sometimes replace a binding whose type contains type
variables with one that does not. This is unsound in the general
case, but we need to to avoid performance problems and ambiguous
solutions in certain pathological cases.
I found two test cases in the test suite that exercised this
specific behavior. Extract them into their own file, and generalize
at the test case. At the same time, tweak the hack to narrow it
down a bit.
Slava Pestov
hectar-glitches
Kathy Gray