Shouldn't it also be stored in the docs directory, for example in TypeChecker.md? It's a bit hard to track down PR descriptions when looking for implementation details.
1 Like
The Swift Evolution proposal template states, with respect to the Detailed Design section:
The detail in this section should be sufficient for someone who is not one of the authors to be able to reasonably implement the feature.
The proposal document serves a key role in explaining why a certain design and not another, which doesn’t have a home elsewhere. In my view, it would certainly be nice to describe how the current detailed design contrasts with a prior attempt particularly in that it unblocks some barrier previously encountered. I don’t know that it has to be as onerous as explaining exactly how the constraint solver works.
To be concrete, I’m saying that I thought @hborla’s reply was really illuminating and literally that text would be a nice addition to the proposal with few changes. By definition, detail sufficient for someone who’s not an author to implement the feature will also likely be more advanced than many readers can understand, but I would argue that that’s explicitly contemplated in the instructions regarding what goes into that section.
4 Likes
This was explicitly considered and rejected in SE-0255. I don’t see that as open for relitigation. Certainly it’s not a part of this proposal.
Ironically, that proposal mentions omitting return type info, and rejects it as not realistic. Things change!
I think this would resolve my misreading. It would help as well if the first example was already two lines or more.
4 Likes
The implementation approach is also documented in the code. If you're working in the implementation, you'll see the documentation for the relevant constraints. Personally, I don't think how the semantics of each kind of expression are represented in the constraint system belongs in TypeChecker.md. That document should instead give folks a model of the backtracking algorithm and an overview of performance heuristics, e.g. connected components. I've been meaning to overhaul this document for a while, because listing out constraint kinds is not going to help people understand the constraint system before diving into the code.
If anyone feels the implementation needs more documentation in the repo, please feel free to leave a code review on the implementation PR.
From the decision:
So I certainly wouldn't consider it ruled out, and if this proposal would make the status quo worse without the corresponding change to return elision in multi-statement closures, I think it's perfectly germane to discuss it here.
That said, I think this particular concern could be addressed with a targeted warning about Void result inference for multi-statement closures, so I don't think that the return elision is necessary, personally.
Mmm, thanks for correcting me! For certain then we should consider how these issues mesh.
1 Like
I have updated the Detailed Design section to point out that contextual type is a primary source of information for the closure.
2 Likes
Technically nothing really, although bodies of the functions are usually bigger than closures so we'd have to figure out whether we need to figure out how to slim down constraint solver state and thresholds.
1 Like
Debates about what proposals are or aren't required to do aside, if this summary wasn't included, how are reviewers supposed to examine the core of the proposal and evaluate whether it makes sense? How are people testing the toolchains for the feature supposed to examine edge cases?
(As an aside, I also disagree that features like this should only be documented at the code level. Unless you're working on the compiler code itself you'd never find it. If we want people who aren't compiler engineers to be able to file proper bugs about inference and other compiler behaviors, it needs to live somewhere visible. IMO, proposals are to document a proposed change, with the full, current behavior documented elsewhere. Whether the project has an appropriate place, I don't know.)
1 Like
Yeah, I'm not suggesting that there be no details at all in the proposal text—I just want to get a handle on what will be considered 'locked in' by this proposal, if accepted. If we decide six months from now that a better heuristic is, say, "look at the first two returns to determine the result type," will it require another proposal to tweak that behavior, or could it be considered a simple 'improvement' to the type inference behavior? (Assuming that such a change could be made in a backwards-compatible way.)
Moderator's note: by request, I have moved three posts about improving the documentation of the constraint system used within the compiler into a new thread; please continue any related discussion there.
Speaking of heuristics…
I suspect (without having gathered any evidence) that a fair number[weasel words] of closures might have their first return statement simply read “return nil”, perhaps in a fail-early guard clause.
That’s enough to indicate the return type is optional, but not what type the wrapped value is. Similarly, “return []” indicates it is an array, but not the element type.
So perhaps a reasonable improvement to the proposal could address those scenarios.
8 Likes
Great point, which builds on my earlier concern regarding return 0.
1 Like
I think this might be a good follow-up just like inout is because it implies inference across statement boundaries, which would be new for the language. The result type of such a closure would have to be a join of all of its return statements.
1 Like
I will add that as a future direction.
What’s the compiler currently doing when a function has an opaque return type and the first return statement has a literal such as this?
The literal type is defaulted. Opaque return type inference also does not join the types of the return values:
func test(value: UInt) -> some ExpressibleByIntegerLiteral {
// ^ error: function declares an opaque return type, but the return statements in its body do not have matching underlying types
if true {
return 0 // note: return statement has underlying type 'Int'
}
return value // note: return statement has underlying type 'UInt'
}
2 Likes
Just to clarify, @xwu, opaque type becomes a type variable that has to conform to a protocol specified after some so it's effectively a situation like <T>(...) -> T. That's why I mentioned that there is no equivalent to this cross-statement inference which would have to happen for return statement to be eluded.