[SR-14044] Contextual Usage of UnsafeMutablePointer<Void> In API is Mis-Diagnosed

[CodaFi]

Previous ID	SR-14044
Radar	rdar://43580528
Original Reporter	@CodaFi
Type	Improvement

Additional Detail from JIRA

Votes	0
Component/s	Compiler
Labels	Improvement, StarterBug, TypeChecker
Assignee	None
Priority	Medium

md5: 29439788662577468de9cf1437c1e59a

Issue Description:

import Foundation

var data = Data(count:0)
try data.withUnsafeMutableBytes { (ptr: UnsafeMutablePointer<Void>) throws -> Void in
}

There is a warning here because TypeCheckType only knows how to unconditionally warn about UnsafeMutablePointer<Void>. It needs to be taught that this contextual usage is okay - because the alternative is the compiler emitting a fixit that introduces a contextual mismatch by rewriting the parameter type to UnsafeMutableRawPointer.

[swift-ci]

Comment by Shreya Sharma (JIRA)

Hello,
I want to work on this, but I'm new to Swift. Can you please describe it further?

[swift-ci]

Comment by Saidhon Orifov (JIRA)

Hey @CodaFi, question about this: since you mentioned that TypeCheckType unconditionally just emits a warning with a fixit, what condition should it not emit those warnings? Is the difference in this case, using UnsafeMutablePointer in the context of closure vs using it in like a function parameter or just literally if it's Void: `(ptr: UnsafeMutablePointer<Void>) throws -> Void in`

[CodaFi]

I think we just need to not emit the warning at all if we're in a situation where we have some kind of contextual mismatch. That's kind of a tricky thing - my first instinct was certainly to suppress it whenever it appears in parameter position, but then code like this

let f = { (ptr: UnsafeMutablePointer<Void>) throws -> Void in }

or

func foo(ptr: UnsafeMutablePointer<Void>) {}

which should emit the warning no longer does. It's a tricky problem, and I fear we may have to give up a class of otherwise sound migrations to close this little loophole. The right fix would be a refactoring that moves this diagnostic into the expression checker, where you could actually determine if emitting it would cause a contextual mismatch.

[swift-ci]

Comment by Saidhon Orifov (JIRA)

Okay, so if I understand correctly based on the response, code like this (generated with warnings & fixit):

 func foo(ptr : UnsafeMutablePointer<()>) {} // warning: UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer
 func foo2(ptr: UnsafeMutablePointer<Void>) {} // warning: UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer

should emit the warning, which is expected, because that's what I am seeing as of Xcode 12.3 at least, and fixit changes it to UnsafeMutableRawPointer, which doesn't cause any contextual mismatch. However, the code in description results in contextual mismatch with the error:

 Cannot convert value of type '(UnsafeMutableRawPointer) throws -> Void' to expected argument type '(UnsafeMutableRawBufferPointer) throws -> Void'

after fixit, which must always be checked before emitting a warning with a fixit, am I correct in this analogy?

[CodaFi]

Yes, that's right.

[swift-ci]

Comment by Saidhon Orifov (JIRA)

@CodaFi I have also worked on SR-13848 along this ticket and PR has been updated (apologies for posting that here, can you please re-review it when you get a chance?) Thanks in advance: PR.

[swift-ci]

Comment by Saidhon Orifov (JIRA)

@CodaFi So I did a little digging on this issue, and what I found were a couple of classes such as ContextualMismatch, ContextualFailure, TypeExpr, CSDiagnostics, and TypeChecker that I supposed could be relevant to helping diagnose this for a possible contextual mismatch. When you mean that this diagnostic needs to be moved in expression checker, do you mean that current diagnostic that adds a FixIt needs to be passed to TypeChecker, where essentially the fixit would be unpacked or am I misunderstanding something here?

[CodaFi]

What I'm saying is that generally the declaration checker and its associated machinery is the wrong place to be diagnosing this kind of thing. The expression type checker is the right judge of whether a particular type annotation is being written by the user (and therefore needs to be diagnosed) or matches the one we can infer from context (in which case we need to leave it alone). The idea should be that we can apply some kind of analysis in the expression checker to distinguish the two cases.