[Sema] Fix erroneous partial application errors

[davezarzycki]

After #30097, some valid full applications are diagnosed as partial
application failures. This restores the old behavior while preserving
the super related fixes in #30097.

[davezarzycki]

@swift-ci please smoke test

[xedin]

I'm confused, didn't @slavapestov intentionally break them because they wouldn't work when partial applications are rewritten into closures?

[davezarzycki]

Well, I'm confused too. The tests in question in this pull request all involve full application, albeit via different syntax. Said differently, and at least when we designed Swift, a.foo() was just "sugar" for A.foo(a)().

[xedin]

Yes, I think that's still intentionally considered invalid for mutating methods as @slavapestov mentioned:

bq. This also detects and bans a case we didn't before: unbound references to mutating methods, even with all argument lists applied. Eg, Foo.bar(&self)(). These actually do work today, but they won't with the new curry thunk implementation. Hopefully this won't cause any fallout, if it does, I'll hack something up.

[davezarzycki]

While I don't use the curry-style method application (full or partial), I'm concerned that it's being retroactively turned into an error, and without any links to either swift-evolution or forum discussion, or deprecation warning – and just the promise that it'll be made to work again if there is fall out. This feels wrong.

CC: @DougGregor @tkremenek

[slavapestov]

The issue here is that if 'foo' is mutating, 'A.foo' becomes a function that takes an inout self parameter and returns a function that captures the inout self. This is UB.

My change diagnoses the same for a fully-applied call, which I believe is the right thing to do.

However, this was already a warning instead of an error in -swift-version 4. Perhaps the fully-applied case should be a warning in both language modes, while the partially-applied case can be a warning only in -swift-version 4?

[davezarzycki]

Right. I get why capturing inout is bad.

I think adding a deprecation warning would be helpful. It would have avoided this pull request at the very least. If you intend to add a warning for the fully applied case, I can close this. :-)

[davezarzycki]

Ping. Should I file a bug about the missing warning in the pre-Swift 5 behavior?

[CodaFi]

I think an SR would be appropriate here.

[CodaFi]

@swift-ci test

[CodaFi]

As a matter of paranoia

@swift-ci test source compatibility

[davezarzycki]

Hi @CodaFi – this shouldn't break anything. It just restores the "old" behavior that existed hours before this PR was opened. That being said, I think @slavapestov's goal is fine, I just wish the diagnostics were better (but I don't have time to fix them myself).

[davezarzycki]

Closing now that I've filed a bug:

https://bugs.swift.org/browse/SR-12722