[Sema] Don't look through CoerceExprs in markDirectCallee

[hamishknight]

Doing so prevented the coercion of a function with argument labels to a user-written function type, as the reference would keep its argument labels due to being marked as "directly applied", but this would then prevent its coercion as the written function type cannot contain argument labels:

func foo(x: Int) {}
(foo as (Int) -> ())(5)
// error: Cannot convert value of type '(Swift.Int) -> ()' to type '(Swift.Int) -> ()'
// in coercion

This PR changes the behaviour such that the referenced function is considered to be unapplied, meaning it has its argument labels stripped, therefore allowing the coercion.

I'm unsure if this change requires any Swift evolution discussion – it is technically source breaking, as it's currently possible to use a generic type alias to make the compiler infer a function type with argument labels:

typealias Magic<T> = T

func foo(x: Int) {}
(foo as Magic)(x: 5)

But I would hope nobody is actually writing code like this, and I can't think of any other compelling examples that would break because of this change.

Resolves SR-11429.

[hamishknight]

@swift-ci please test

[hamishknight]

@swift-ci please test source compatibility

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 1cc1e58

[hamishknight]

@swift-ci please test OS X platform

[jrose-apple]

This seems right to me but @xedin would be a better person to check with. I think someone from the core team should probably say this is okay but it does seem obviously correct. That comment about disambiguation does seem possible, though.

[hamishknight]

@jrose-apple I believe the comment about disambiguation was made back when function types could have argument labels, so you could do things like:

func foo(x: Int) {}
func foo(y: Int) {}

(foo as (x: Int) -> Void)(x: 5)
(foo as (y: Int) -> Void)(y: 5)

But AFAIK it's no longer possible to perform such disambiguation using an intermediate coercion.

[jrose-apple]

I thought it's still potentially relevant for things like

func foo(x: Int8) {}
func foo(x: UInt8) {}
(foo as (Int8) -> Void)(5)

…except that that already doesn't work

<stdin>:3:2: error: ambiguous reference to member 'foo(x:)'
(foo as (Int8) -> Void)(5)
 ^~~
<stdin>:1:6: note: found this candidate
func foo(x: Int8) {}
     ^
<stdin>:2:6: note: found this candidate
func foo(x: UInt8) {}
     ^

and you'll note I left out the label

<stdin>:3:24: error: extraneous argument label 'x:' in call
(foo as (Int8) -> Void)(x: 5)
                       ^~~~

So maybe we're in the clear.

[hamishknight]

@jrose-apple Yeah, because we mark (overloaded) decl ref foo as being directly applied, when attempting overloads we don't strip the argument labels from the type, meaning that we're then unable to perform a coercion to a function type without arg labels. This PR would allow (foo as (Int8) -> Void)(5) to compile.

[slavapestov]

Function types with argument labels are definitely not meant to be part of the type system, and it's mostly a historical artifact that this can be represented at all.

[slavapestov]

I wonder if rdar://36209605 is related:

struct S {
  init() {}

  static func create(x: Int) -> S {
    return S()
  }
}

let s1: S = S.create(x:)(12) // OK
let s2: S = .create(x:)(12) // error: missing argument label 'x:' in call

[hamishknight]

@slavapestov That's indeed a similar issue, though an UnresolvedMemberExpr doesn't have its FunctionRefKind determined by markDirectCallee, instead it's decided when generating constraints:

https://github.com/apple/swift/blob/c79214c3c6f7d2a9880c76b93cae1ec3659dbc8f/lib/Sema/CSGen.cpp#L1469-L1471

We ought to be treating it as FunctionRefKind::Compound if its decl name is compound, even if it has an argument expr. Though unfortunately this would also be source breaking for IUO returns, for example the following would no longer compile:

struct S {
  static func foo(_ x: Int) -> S! { S() }
}

let s: S = .foo(_:)(5)

Unless we change the rules such that IUO unwraps are allowed through compound references which are otherwise directly applied.

[xedin]

This does seem reasonable to me and I'm be fine breaking the case mentioned in the description to make the behavior consistent, but I think we still need a core team sign-off on that. @DougGregor could you please take a look as well?

[DougGregor]

Sorry for the delay; I'll bring this up with the core team

[DougGregor]

The Core Team discussed this today and has approved the change. Thanks for your patience!

[hamishknight]

@swift-ci please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 1cc1e58

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 1cc1e58

[hamishknight]

@swift-ci please smoke test

[jrose-apple]

Worth mentioning in the CHANGELOG, since it both fixes a source-level issue and breaks obscure code?

[hamishknight]

@jrose-apple Sure thing! I'll try and open a PR tomorrow.