[Diagnostics] Strip off unrelated optionals from generic parameter di…

[xedin]

…agnostics

In situations where left-hand side requires value-to-optional
promotion which ends up in type mismatch let's not mention
optionals in the diagnostic because they are unrelated e.g.

func test(_: UnsafePointer<Int>??) {}

var value: Float = 0
test(&value)

In this example value gets implicitly wrapped into a double optional
before UnsafePointer<Float> could be matched against UnsafePointer<Int>
associated with the parameter.

Diagnostic is about generic argument mismatch Float vs. Int
and shouldn't mention any optionals.

[xedin]

@swift-ci please smoke test

[hborla]

Hmm, I'm not sure how I feel about these diagnostics changes. I understand that Optional is not part of the mismatch, but it is important in identifying where that type is in the source code. This error message seems wrong because the argument type is [String]?, not [String].

Could we instead add a note similar to the one we emit for same-type generic requirements?

[xedin]

All of the diagnostics here were wrong because they mentioned optional only on one side and produced a note which mentioned either Element or Pointee which is confusing. So this is at least an improvement on status quo.

Because it's allowed to implicitly load value into an optional, the problem is that either we produce a diagnostic where everything is optional - e.g. [Int]? and [String]? and a note which would have to say that Wrapped is a problem (which is incorrect), or we ignore optionals and produce diagnostic about actual mismatch which is better.

[LucianoPAlmeida]

This error message seems wrong because the argument type is [String]?, not [String].

I thought the same when added this in #30814, because that is the argument type as the user declared :)
So to me, it seemed like the type in the message was not right in some way

[xedin]

It seems like before doing anything else we need to figure out diagnostic format that allows to mention complete types as well as their portions e.g. tuple mismatches which have to mention whole tuple as well as narrowed down message(s) about mismatched bits.

[hborla]

I think it's fine that the diagnostic only has optional on one side - I think that's how users think about value-to-optional conversions anyway. I was suggesting adding a new note about the Element types expected to be the same (in this case), not Wrapped

[xedin]

@hborla What do you mean by new note? There is a note about Element already associated with that error.

[hborla]

Oh whoops, you're right, I didn't even notice the note because it wasn't part of the change

[xedin]

I think if we wanted to start mentioning types as written we'd end up with diagnostics like this:

func takesConstPointer(_ x: UnsafePointer<Int>??) -> Character { return "x" }

var ff: [Float] = [0, 1, 2]
takesConstPointer(ff)

error:  cannot convert value of type '[Float]' to expected argument type 'UnsafePointer<Int>??'

note: arguments to generic parameter 'Pointee' ('Float' and 'Int') are expected to be equal
takesConstPointer(ff)
                  ^

Which is not very informative, but maybe okay?

[hborla]

I think that case in particular might benefit from a tailored argument-to-parameter error message that somehow mentions the array-to-pointer conversion so that Pointee in the note makes more sense... I do think the note is very informative because it tells you exactly which part of the type that's wrong (Float vs Int)

[xedin]

We currently produce a diagnostic which mentions UnsafePointer<Float> instead of [Float] and doesn't have optional on the right-hand side, that's why it makes sense to have Pointee in the note.

If we wanted to start diagnosing chains like [Float] -> [Float]? -> [Float]?? -> UnsafePointer<Float>?? which then matched vs. UnsafePointer<Int>?? we'd probably need to get some kind of "nested" error representation first...

[hborla]

Pavel and I discussed this offline and we decided to take this change for now

[LucianoPAlmeida]

I think if we wanted to start mentioning types as written we'd end up with diagnostics like this:

func takesConstPointer(_ x: UnsafePointer<Int>??) -> Character { return "x" }

var ff: [Float] = [0, 1, 2]
takesConstPointer(ff)

error:  cannot convert value of type '[Float]' to expected argument type 'UnsafePointer<Int>??'

note: arguments to generic parameter 'Pointee' ('Float' and 'Int') are expected to be equal
takesConstPointer(ff)
                  ^

@xedin @hborla I wonder if cases like this ☝️ you mentioned already exists on the suit?
If not, maybe it is worth adding? :)

[xedin]

Yeah, that’s the test case from the suite :)

[xedin]

@swift-ci please smoke test macOS platform