[Diagnostics] Improve diagnostic when attempting to pass an Array to a variadic argument

[owenv]

• Give a more specific diagnostic which indicates the parameter is variadic
• If the argument is an Array literal, offer to drop the brackets

Example:

/Users/owenvoorhees/Desktop/hello.swift:3:3: error: cannot pass an array of type '[Int]' as variadic arguments of type 'Int'
f([1,2,3])
  ^
/Users/owenvoorhees/Desktop/hello.swift:3:3: note: remove brackets to pass array elements directly
f([1,2,3])
  ^     ~

This cherry-picks the standalone diagnostics improvements from #25997 . That PR has a bit of additional context.

In the short term, adding this fix might exacerbate SR-11104 slightly.

Resolves SR-6623

cc @xedin , @hamishknight

[xedin]

Unfortunately we are not yet there with argument-to-parameter conversion mismatches, because different overloads could fail in a different way and we need to produce a diagnostic which includes all reasonable mismatches. For example, in this case there could be an overload of function which accepts an array as an argument but places a requirement on it's type:

func foo<T: P>(_ x: [T]) {}
func foo(_ x: Int...) {}

foo([1, 2, 3])

I think this should produce a diagnostic which mentions that candidate foo<T>([T]) would match if Element type of the Array conforms to P as well as candidate foo(Int...) would match if arguments are unpacked from array.

To get there we need to first provide a way to associate a fix with each overload, figure out which overloads are completely irrelevant and produce an "ambiguity" diagnostic which mentions all of of the fixable cases.

I like the idea of adding parameter flags to locator, I think it could be merged separately.

[owenv]

Ah, ok, makes sense. I'm going to go ahead and close this then.

[xedin]

@owenv Once the infra is there we should be able to merge this PR.

[xedin]

@owenv I think we can re-open this PR now since generic argument conversions have landed and the case I was talking about should be covered.

[xedin]

It just needs a rebase...

[owenv]

@xedin Sounds good! This might need a little more work, I'm not sure all the locator changes are still needed. I'll try and take a look and fix it up sometime in the next few days.

[xedin]

Sounds good! Let me know if you need any help with that!

[owenv]

@xedin I've brought this PR up to date, and for the most part things are working nicely with the other argument mismatch fixes. I am running into a couple of problematic cases that I was hoping I could get your opinion on though.

The first is your example from above,

protocol P {}
func f<T: P>(x: [T]) {}
func f(x: Int...) {}
f(x: [1,2,3])

Ideally I think this would diagnose the missing conformance for the first overload and the bad varargs conversion for the second. Right now, it only diagnoses the varargs conversion because that solution ranks higher since its overload isn't generic.

The other case is:

protocol P {}
func f<T: P>(x: [T]) {}
func f<T: P>(x: T...) {}
f(x: [1,2,3])

In this case, right now only the missing conformance for the first overload is diagnosed, because solutions involving overloads with a fixed number of params rank higher than ones with variadic params when both are generic.

I'm not sure if solution ranking should be more lenient here when all of the solutions require fixes, or if there would be a better way to improve this. Sorry if I'm just missing something obvious!

[xedin]

@owenv I agree that eventually cases like this should not be ranked and produce "ambiguous" erro with multiple notes, but we still rely on ranking for some cases to produce "best" solution. I'd suggest you not to worry about it for now and leave a TODO(diagnostics): near test-case for posterity as well as implement diagnoseAsNote so when ranking behavior changes everything would just work.

[owenv]

@xedin Cool, I kind of figured trying to change ranking right now would be problematic. I've updated this to make the changes you suggested, I think it should be ready for a review now.

[xedin]

Looks great! Thank you! Going to be even better after a small follow-up I mentioned inline.

[hamishknight]

Thanks for sticking with this!

[xedin]

@swift-ci please test

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 153b4808e612e2e69962ee54cd8f061daad25312

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 153b4808e612e2e69962ee54cd8f061daad25312

[xedin]

@swift-ci please smoke test