[CSSolver] Use correct locator when matching function result types re…

[xedin]

…lated to closures

Currently we always use 'FunctionResult' as a path element when matching
function result types, but closure result type is allowed to be implicitly
converted to Void, which means we need to be careful when to use
'FunctionResult' and 'ClosureResult'.

Resolves: rdar://problem/37790062

[xedin]

/cc @rudkx This is alternative to #14822. It type-checks the example but fails later on in SILGen, I think we need to do something in CSApply if we want to go this way, WDYT?

[rudkx]

fyi, i'm taking a careful look at this

[rudkx]

I think this seems reasonable if slightly precarious. I suspect I am going to end up substantially reworking/replacing getPotentialBindings() in the not-too-distant-future and hopefully as a result of that make something that doesn't have all the strange ordering issues we have today.

[xedin]

@rudkx Ok, sounds good! I'll try to figure out what do we need to do to make this work in CSApply then.

[xedin]

@rjmccall @jckarter Looks like I'd need some help from you here, it seems like ResultPlan doesn't account for conversions from () -> T to () -> Void although we create FunctionConversionExpr to mark that...

I can fix up the assert at https://github.com/apple/swift/blob/master/lib/SILGen/SILGenPoly.cpp#L1880, but when I do so it fails at
https://github.com/apple/swift/blob/master/lib/SILGen/SILGenPoly.cpp#L1940 to cast innerSubstType to tuple (which innerSubstType is not, it's an archetype related to A) but it tries to emit it into empty tuple (which is what Void type is).

So I'm wondering what would be the best approach here? I guess since it's trying to convert function which returns something to function which returns nothing it could be fine to just drop the result type on the floor?

[xedin]

Simpler example:

struct S<T> {
  init(_ a: () -> T, _ b: () -> T) {}
}

func foo() -> Int { return 42 }
func bar() -> Void {}

_ = S({ foo() }, { bar() })

[jckarter]

Dropping a result isn't always an ABI-compatible function conversion if the result is returned indirectly or needs to be released. We ought to be emitting a thunk when this conversion happens. The failures you're seeing suggest to me that isn't happening.

[xedin]

@jckarter Sorry if it wasn’t clear but it fails while trying to emit such a thunk, it seems like it doesn’t support converting result from something to void...

[xedin]

@jckarter What is the best way to express in result planner conversion from something to void?

[jckarter]

If we're converting a result to void, then we need to destroy the result value(s) and produce an empty tuple.

[xedin]

@jckarter Could you please elaborate where I could start to figure out how to do that? :)

[xedin]

/cc @rjmccall Please take a look! I'm pretty sure I've done it incorrectly, but I tried...

[xedin]

@swift-ci please smoke test

[rjmccall]

1. This needs be recursive — we expand tuples recursively, so this needs to do it, too.
2. This needs to follow the inner abstraction pattern, not the inner substituted type. There could be a generic type that's been substituted to a tuple.

I would suggest breaking this out as a method on the planner.

[xedin]

@swift-ci please test

[xedin]

@swift-ci please test source compatibility

[rjmccall]

Thanks, that looks good.