Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager. If you do email me directly, please put "SE-0302" somewhere in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at:
This is similar to how C++ type checks auto return types in functions and lambdas. I think this is a good fit for Swift, but I am very interested in whether the return type joining idea is feasable.
I’m confused about the motivation example. Is it supposed to compile correctly without the proposed changes?
Particularly, the map function is supposed to return a type T that conforms to BinaryInteger. However Void, the return type of doSomething, does not conform to BinaryInteger.
It seems to me that there is no correct way to type check the first example. Perhaps doSomething should return a type that conforms to BinaryInteger?
I’ve spent about 20 minutes reading the motivation section of the proposal so maybe I am misunderstanding what the example is supposed to illustrate.
Sorry for the confusion, map in that example shouldn’t have a BinaryInteger requirement on T, I will fix it momentarily. Pitch had doSomething return U but it was mentioned that it’s not very clear because result type would change in multi-statement case, so I changed it to Void instead.
The Source compatibility section is a bit confusing to me. It ends:
There are a couple of ways to mitigate situations like this: [...]
However, it's not clear to me which of the subsequently listed options the proposal actually adopts. Is the behavior adopted by this proposal source-compatible, or not?
The (\$0) almost looks like an interpolation, but that would change the "first reference to an anonymous parameter" location. It might be clearer to remove the (\$0) from those examples.
No, the proposal doesn't adopt any mitigations for the problem, it let's these calls be ambiguous and leaves it for the developers to fix it by providing explicit type.
In that case, has there been an investigation to what the source compatibility impact of this change is, in practice? Were there any issues in the source compat test suite, or do we expect this situation to be exceedingly rare?
Yes to both of your questions - I have ran public source compatibility suite with this change enabled as well as a large number of other projects and there was only one instance of this ambiguity that I found and one of the two overloads there has been deprecated. If that proves to be a problem bigger problem in the future, we can always mitigate that via a ranking rule, but I think the unified semantics make a lot more sense.
That's an amazing improvement. I have one question, thought. Will the following code compile or does it fall into the same case pointed out in Future Directions?
struct Box {
let weight: UInt
}
func heavier_than(boxies: [Box], min: UInt) -> [UInt?] {
let result = boxies.map {
if $0.weight > min {
return $0.weight
}
return nil
}
return result
}
Unfortunately it won't because there is no-join between first and second return statements in the map but if you wrote return boxes.map { ... } because the result type if inferred from the context. I'm thinking about a possible approach for inference across return statements but it's not an easy problem to solve implementation-wise.
I'm super happy and definitely +1 for this, but unfortunately it seems that in this state the proposal would not address what I would consider to be the prime example of the need for multi-statement closure type inference, that is, extracting associated values from enum cases in an ergonomic way.
For example, would the following compile under the current proposal?
enum Either2<A, B> {
case a(A)
case b(B)
}
let eithers: [Either2<Int, String>] = [
.a(42),
.b("yello"),
]
let ints = eithers.compactMap {
guard case .a(let value) = $0 else {
return nil
}
return value
}
// ints should be inferred to be `[Int]`
I guess not (or maybe because compactMap expects a closure to return some Optional it could work, but I'm not sure).
EDIT: I can confirm that it doesn't compile, with Generic parameter 'ElementOfResult' could not be inferred. It can be mitigated with the following:
let x = xs.compactMap {
guard case .a(let value) = $0 else {
return Int?.none
}
return value
}
which is still a major step forward if compared with the current situation
Also, I'm trying to test this with the toolchain linked in the proposal and the flags, but it's not working. I'm not sure if I'm doing it right though.
