[Pitch] Enable multi-statement closure parameter/result type inference

xwu · 2021-10-06T17:09:57.134Z

soroush:

I know people are split on whether SE-0255 (Implicit returns from single-expression functions) was a good idea, but given that it does exist in the language, and we're removing the restriction that requires a return type, I think it makes sense to remove the explicit return keyword restriction from multi-line functions as well.

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.

soroush · 2021-10-06T17:14:18.124Z

xwu:

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!

xedin:

What if I either make doSomething return Void instead of U or add an explicit contextual type to a let binding in both cases?

I think this would resolve my misreading. It would help as well if the first example was already two lines or more.

hborla · 2021-10-06T17:17:19.649Z

Max_Desiatov:

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.

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.

Jumhyn · 2021-10-06T17:20:29.366Z

xwu:

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.

From the decision:

SE-0255: Implicit Returns from Single-Expression Functions

The core team also acknowledges that accepting this feature may encourage follow-on proposals that generalize it further, such as [...] allowing elision in multi-statement bodies. Accepting this proposal doesn't necessarily endorse or rule out those future directions.

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.

xwu · 2021-10-06T17:21:54.148Z

Jumhyn:

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.

Mmm, thanks for correcting me! For certain then we should consider how these issues mesh.

John_McCall · 2021-10-06T20:44:41.556Z

3 posts were split to a new topic: Improving the documentation of the constraint system

xedin · 2021-10-06T17:47:54.739Z

I have updated the Detailed Design section to point out that contextual type is a primary source of information for the closure.

xedin · 2021-10-06T17:49:50.864Z

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.

Jon_Shier · 2021-10-06T17:51:46.397Z

Jumhyn:

Yeah, I agree, and wonder if we should go even further—need the precise semantics of the inference (based on first use of arguments/first return statement) be explicitly specified at all? Would we need a further proposal if we come up with a better heuristic for determining the type of the closure?

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.)

Jumhyn · 2021-10-06T18:17:03.786Z

Jon_Shier:

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?

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.)

John_McCall · 2021-10-06T20:48:17.874Z

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.

Nevin · 2021-10-06T21:09:20.176Z

Jumhyn:

If we decide six months from now that a better heuristic is, say, "look at the first two return s 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?

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.

xwu · 2021-10-06T21:56:31.551Z

Nevin:

So perhaps a reasonable improvement to the proposal could address those scenarios.

Great point, which builds on my earlier concern regarding return 0.

xedin · 2021-10-07T00:56:14.290Z

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.

xedin · 2021-10-07T00:56:42.338Z

I will add that as a future direction.

xwu · 2021-10-07T02:48:30.325Z

xedin:

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.

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?

hborla · 2021-10-07T02:56:56.133Z

xwu:

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'
}

xedin · 2021-10-07T03:07:52.698Z

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.

xedin · 2021-10-07T16:44:46.704Z

I found a case with source compatibility implications:

func test<T, R>(_: (UnsafePointer<T>) -> R) -> R { fatalError() }
func test<T>(_: (UnsafeRawBufferPointer) -> T) -> T { fatalError() }

let _: Int = test { ptr in
  //assert(ptr[0] == UInt8.max)
  return Int(ptr[0]) << 2
}

Without assert this closure is ambiguous but with it - type-checks just fine, but only if I specify contextual type explicitly. This happens because overload with UnsafeRawBufferPointer doesn't have any unresolved type variables in it (due to supplied contextual result type), so it's the only valid solution for multi-statement closure case without checking the body.

With proposed changes this test-case is going to be ambiguous regardless of assert because it would be possible to infer pointer type from the body of the closure.

I'm going to add this to source compatibility section and it should be possible to mitigate via ranking rules if we wanted but I'd prefer to keep the unified behavior.

Ben_Cohen · 2021-10-07T18:51:19.097Z

soroush:

I think it makes sense to remove the explicit return keyword restriction from multi-line functions as well.

I think some of this is style preference, but I personally would rather not go down this path. I find a bare expression at the end of some other statements being an implied return very unsettling:

_ = vals.map {
  if case .baz(let value) = $0 {
    return value
  }
  nil
}

I know this is how several languages work but I just don't like it. I'd have to think about how to articulate why.

I think something that will go a long way to making it less pressing would be if if/switch statements became expressions. IIRC @Michael_Ilseman went through an analysis of the standard library a while back and found that a huge number of functions would benefit from this.

_ = vals.map {
  if case .baz(let value) = $0 {
    value
  } else {
    nil
  }
}

(this doesn't mesh as well with guard of course, though I feel like guard is a little overused tbh)

Having raised this topic... I'll also say we shouldn't discuss it on this pitch, except in so much as to say this proposal's implementation makes this change very simple. But I suggest we discuss it on a separate thread.