My experiment to improve source compatibility by also performing a
backward scan removed the SE-0286 heuristic that skipped binding
the unlabeled trailing closure to a defaulted parameter when that
would fail. Reinstate that heuristic, which makes more existing code
work with the forward-scan behavior.
This makes my source-compatibility improvements a quality-of-implementation
Whenever we form a call that relies on the deprecated "backward" scan,
produce a warning to note the deprecation along with a Fix-It to label
the parameter appropriately (and suppress the warning). For example:
warning: backward matching of the unlabeled trailing closure is
deprecated; label the argument with 'g' to suppress this warning
trailingClosureEitherDirection { $0 * $1 }
^
(g: )
To better preserve source compatibility, teach the constraint
solver to try both the new forward scanning rule as well as the
backward scanning rule when matching a single, unlabeled trailing
closure. In the extreme case, where the unlabeled trailing closure
matches different parameters with the different rules, and yet both
produce a potential match, introduce a disjunction to explore both
possibilities.
Prefer solutions that involve forward scans to those that involve
backward scans, so we only use the backward scan as a fallback.
SE-0248 changes the backward-scan matching behavior for the unlabeled
trailing closure into a forward scan. In circumstances where this
could silently change the meaning of a call to a particular
function, i.e., when there are two defaulted closure parameters such
that a given closure to match either one of them, produce an warning
that describes the change in behavior. For example:
t4.swift:2:24: warning: since Swift 5.3, unlabeled trailing
closure argument matches parameter 'x' rather than parameter 'z'
trailingClosureSingle2 { $0 }
^
t4.swift:2:24: note: label the argument with 'z' to retain the
pre-Swift 5.3 behavior
trailingClosureSingle2 { $0 }
^
(z: )
t4.swift:2:24: note: label the argument with 'x' to silence this
warning for Swift 5.3 and newer
trailingClosureSingle2 { $0 }
^
(x: )
t4.swift:1:6: note: 'trailingClosureSingle2(x:y:z:)' contains
defaulted closure parameters 'x' and 'z'
func trailingClosureSingle2(x: (Int) -> Int = { $0 } , y: (Int) ->
Int = { $0 }, z: (Int) -> Int = { $0 }) {}
^ ~
This explains the (rare) case where SE-0286 silently changes the
meaning of a program, offering Fix-Its to either restore the
pre-SE-0286 behavior or silence the warning, as appropriate.
The change to the forward-scanning rule regressed some diagnostics,
because we no longer generated the special "trailing closure mismatch"
diagnostic. Reinstate the special-case "trailing closure mismatch"
diagnostic, but this time do so as part of the normal argument
mismatch diagnostics so it is based on type information.
While here, clean up the handling of missing-argument diagnostics to
deal with (multiple) trailing closures properly, so that we can (e.g)
suggest adding a new labeled trailing closure at the end, rather than
producing nonsensical Fix-Its.
And, note that SR-12291 is broken (again) by the forward-scan matching
rules.
Once the first argument for a variadic function-typed parameter has been
matched, allow an unlabeled trailing closure to match, rather than
banning all uses of the unlabeled trailing closure with variadic
parameters.
The "fuzzy" forward scan matching algorithm was only applied when there
was a single, unlabeled trailing closure, but was disabled in the
presence of multiple trailing closures. Extend the "fuzzy" match to
account for multiple trailing closures, by restricting the search for
"a later parameter that needs an argument" to stop when we find a
parameter that matches the first (labeled) trailing closure.
Introsuce a new "forward" algorithm for trailing closures where
the unlabeled trailing closure argument matches the next parameter in
the parameter list that can accept an unlabeled trailing closure.
The "can accept an unlabeled trailing closure" criteria looks at the
parameter itself. The parameter accepts an unlabeled trailing closure
if all of the following are true:
* The parameter is not 'inout'
* The adjusted type of the parameter (defined below) is a function type
The adjusted type of the parameter is the parameter's type as
declared, after performing two adjustments:
* If the parameter is an @autoclosure, use the result type of the
parameter's declared (function) type, before performing the second
adjustment.
* Remove all outer "optional" types.
For example, the following function illustrates both adjustments to
determine that the parameter "body" accepts an unlabeled trailing
closure:
func doSomething(body: @autoclosure () -> (((Int) -> String)?))
This is a source-breaking change. However, there is a "fuzzy" matching
rule that that addresses the source break we've observed in practice,
where a defaulted closure parameter precedes a non-defaulted closure
parameter:
func doSomethingElse(
onError: ((Error) -> Void)? = nil,
onCompletion: (Int) -> Void
) { }
doSomethingElse { x in
print(x)
}
With the existing "backward" scan rule, the trailing closure matches
onCompletion, and onError is given the default of "nil". With the
forward scanning rule, the trailing closure matches onError, and there
is no "onCompletion" argument, so the call fails.
The fuzzy matching rule proceeds as follows:
* if the call has a single, unlabeled trailing closure argument, and
* the parameter that would match the unlabeled trailing closure
argument has a default, and
* there are parameters *after* that parameter that require an argument
(i.e., they are not variadic and do not have a default argument)
then the forward scan skips this parameter and considers the next
parameter that could accept the unlabeled trailing closure.
Note that APIs like doSomethingElse(onError:onCompletion:) above
should probably be reworked to put the defaulted parameters at the
end, which works better with the forward scan and with multiple
trailing closures:
func doSomethingElseBetter(
onCompletion: (Int) -> Void,
onError: ((Error) -> Void)? = nil
) { }
doSomethingElseBetter { x in
print(x)
}
doSomethingElseBetter { x in
print(x)
} onError: { error in
throw error
}
* [CSDiagnostics] Adjusting MemberAccessOnOptionalBaseFailure to be able to handle key path component member base types
* [tests] Adding regression tests for SR-5688
* [CSDiagnostics] Adjusting source range to diagnose/insert the fixes in correct location
* [tests] Adjusting regression tests to handle the fixits
* [AST] Creating an helper getSourceRange function for KeyPathExpr::Component
* [ConstraintSystem] Store member base type when recording UnwrapOptionalBase fix
* [AST] Creating a new diagnostic note for removing optional from written type
* [CSDiagnostics] Adjusting logic around MemberAccessOnOptionalBaseFailure to emit the correct diagnostics and fixes
* [tests] Adjusting regression tests to add subscript and key path root cases with respective diagnostics
* [Diagnostics] Adjusting message to mention base type
* [CSDiagnostics] Better naming for method/variable that represents base source range
* [CSDiagnostics] Adjusting to use the stored base member only when member is a key path component.
* [Diagnostics] Adjusting minor typos and code
* [AST] Adjusting keypath root diagnostic note message for use unwrapped type
* [CSDiagnostics] Adjusments in MemberAccessOnOptionalBaseFailure diagnostics as per suggestion
* [tests] Adding more test cases for SR-5688
* [CSDiagnostics] Adjusting fixits for key path root and range for diagnostics
* [CSSimplify] Attempt to diagnose InsertExplicitCall for optional function return types when possible on simplifyOptionalObjectConstraint
* [tests] Adding TODO to improve the diagnostics refering to key path root infered as optional types
* [CSDiagnostics] Adjusting comments
* [CSSimplify] Adjusting logic on simplifyOptionalObjectConstraint to attempt InsertCall fix before remove unwrap
* [CSDiagnostics] Adjust the logic to use resolveType on MemberAccessOnOptionalBaseFailure construction
Currently it's possible to have a type conflict between different
requirements deduced as the same type which leads to incorrect
diagnostics. To mitigate that let's adjust how "fixed" requirements
are stored - instead of using resolved type for the left-hand side,
let's use originating generic parameter type.
Instead of requiring sub-classes of `ContextualMismatch` to implement
`diagnoseForAmbiguity` let's implement it directly on `ContextualMismatch`
itself and check whether all of the aggregated fixes have same types on
both sides and if so, diagnose as-if it was a single fix.
We currently leave a key path constraint unsolved
if one of its components hasn't yet had its
overload resolved. However, for e.g a missing
member component, the overload type variable will
be bound to a hole and an overload will never be
resolved.
Tweak the logic to consider the key path constraint
trivially solved if one of its components has been
marked as a hole, which will allow the key path
type itself to be marked as a hole.
Resolves SR-12437 & SR-12823.
Resolves rdar://62201037.
Detect situation when it's impossible to determine types for
closure parameters used in the body from the context. E.g.
when a call closure is associated with refers to a missing
member.
```swift
struct S {
}
S.foo { a, b in } // `S` doesn't have static member `foo`
let _ = { v in } // not enough context to infer type of `v`
_ = .foo { v in } // base type for `.foo` couldn't be determined
```
Resolves: [SR-12815](https://bugs.swift.org/browse/SR-12815)
Resolves: rdar://problem/63230293
If a Swift key path has root type inferred but does not have a leading dot,
then diagnose it, because it's not valid.
For example:
```swift
struct Foo {
let property: [Int] = []
let kp: KeyPath<Foo, Int> = \property.count // error
}
```
Resolves SR-12290
Resolves rdar://problem/59874355
Previously we were bailing early on encountering
an optional chain in the key path. However this
could cause us to miss invalid components further
down the line. Instead, set a flag and force the
key path to be read-only if we encountered an
optional chain.
Resolves SR-12519.
Previously we could skip default literal or
supertype bindings if we had already found a solution
with fixes, which could lead us to miss bindings
that produce better diagnostics.
Tweak the logic such that we continue exploring if
we're in diagnostic mode.
Resolves SR-12399.
`_` or discard assignment expression should only be used on the left-hand
side of the assignment expression. Incorrect uses are easy to detect during
constraint generation which also allows us to avoid complications related
to other diagnostics when `_` is used incorrectly.
Reverts apple/swift#30006. It caused a regression that we'd like to address before re-landing:
```swift
struct X {
var cgf: CGFloat
}
func test(x: X?) {
let _ = (x?.cgf ?? 0) <= 0.5
}
```
This reverts commit 0a6b444b49.
This reverts commit ed255596a6.
This reverts commit 3e01160a2f.
This reverts commit 96297b7e39.
Resolves: rdar://problem/60185506
Prioritize type mismatches over conformance failures when stdlib
types are involved because it wouldn't be appropriate to suggest
to add such a conformance, so the problem is most likely related
to something else e.g. other overload choice has a better fix.
Consider following example:
```swift
struct S {
init(_: Double) {}
init<T: BinaryInteger>(_: T) {}
}
_ = S(Double("0"))
```
In cases like that it's better to prefer failable initializer
which takes a `String` and returns `Double?` and diagnose a
problem related to missing optional unwrap instead of missing
conformances related to a `String` argument of other `Double`
initializer just because it returns a concrete type.
