Placeholder types: take two

[Jumhyn]

Implements the placeholder types proposal.

[Jumhyn]

cc @xedin @CodaFi @hborla since y'all looked at the last PR on placeholder types 🙂

[xedin]

@Jumhyn This is in my queue, going to try to look asap.

[xedin]

LGTM! I have left some comments inline, but they are mostly minor. I also think it might be worth it to land some part of this PR separately.

[Jumhyn]

@xedin So it seems like things mostly work out just fine. The only additional failure is in test/AutoDiff/Sema/differentiable_func_type.swift, where issues that were previously diagnosed as ambiguities now get more specific error messages because some overloads became unviable (now that their parameter types are ErrorType). Also, we no longer diagnose two layers deep on a non-differentiable function with a non-differentiable function result—we'll only diagnose the innermost issue, looks like. The full diff of the necessary diagnostic changes is below—if you think this is acceptable I'll push up the changes!

-// expected-error @+2 {{result type 'Int' does not conform to 'Differentiable', but the enclosing function type is '@differentiable'}}
-// expected-error @+1 {{result type '@differentiable(reverse) (U) -> Int' does not conform to 'Differentiable', but the enclosing function type is '@differentiable'}}
+// expected-error @+1 {{result type 'Int' does not conform to 'Differentiable', but the enclosing function type is '@differentiable'}}
 func test3<T: Differentiable, U: Differentiable>(_: @differentiable(reverse) (T) -> @differentiable(reverse) (U) -> Int) {}
...
-// expected-note  @+5 2 {{found this candidate}}
...
-// expected-error @+1 {{no exact matches in call to global function 'inferredConformancesGenericLinear'}}
+// expected-error @+2 {{global function 'inferredConformancesGenericLinear' requires that 'Int' conform to 'Differentiable'}}
+// expected-error @+1 {{global function 'inferredConformancesGenericLinear' requires that 'Int' conform to 'Differentiable'}}
 inferredConformancesGenericLinear(nondiff)

-// expected-note @+1 2 {{found this candidate}}
+// expected-note  @+2 2 {{where 'T' = 'Int'}}
+// expected-note  @+1 2 {{where 'U' = 'Int'}}
 func inferredConformancesGenericLinear<T, U>(_: @differentiable(_linear) (Linear<T>) -> Linear<U>) {}
...
-// expected-error @+1 {{no exact matches in call to global function 'inferredConformancesGenericLinear'}}
+// expected-error @+2 {{global function 'inferredConformancesGenericLinear' requires that 'Int' conform to 'Differentiable'}}
+// expected-error @+1 {{global function 'inferredConformancesGenericLinear' requires that 'Int' conform to 'Differentiable'}}
 inferredConformancesGenericLinear(nondiff)

Note: we get the diagnostic twice because the parameter and result type both require that Int conform to Differentiable.

[xedin]

@Jumhyn Sorry it took me a bit to respond, I'm on vacation this week. I think new diagnostics make sense, they are definitely better than ambiguities especially considering that other overload is invalid, so I'd say we should proceed with failing in type resolution + failing on coerceToType.

[Jumhyn]

No worries at all @xedin, hope you're having a nice vacation! Please feel free to ignore me until you're back. :)

For whenever you're back: yes, I agree the diagnostics are improved! Just wanted to make sure I wasn't missing anything. I've rebased to fold the type resolution failure into these changes, so I believe all your comments should be addressed at this point.

[Jumhyn]

Hey @xedin, if you're still on vacation then disregard, just wanted to put this back on your radar in case it slipped off while you were out!

[xedin]

I'm back and it did slip from my radar unfortunately but now it's back in the fold :)

[xedin]

LGTM!

[Jumhyn]

@xedin Alright, updated with clang-format's preferred line breaks for that snippet. Thank you!

[jckarter]

Is the [DNM] marker in this title still relevant? Any changes you still want to make before I merge this?

[Jumhyn]

@jckarter No, assuming that all tests are still passing (🤞) there aren't any changes I intend to make here. Should be merge-ready!

[hborla]

I think the source compatibility failure will be fixed by #38928, which was just merged.

@swift-ci please test source compatibility

[Jumhyn]

Thank you @hborla! @xedin, looks like this is good to go!

[xedin]

@hborla, @rintaro please take a look and we'll land this if everything looks good.

[rintaro]

LGTM. Just a couple of nitpicks

[Jumhyn]

Thanks @rintaro, your comments have been addressed!

[xedin]

@swift-ci please test

[Jumhyn]

@swift-ci please test

[Jumhyn]

@xedin Good to merge?

[rintaro]

🎉 @Jumhyn Could you update CHANGELOG.md too?

[Jumhyn]

@rintaro Already opened the PR here, just waiting for tests to complete after the CI outage :)