Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
CodaFi opened this issue Jan 13, 2021 · 8 comments
Open

Comments

@CodaFi
Copy link
Member

@CodaFi CodaFi commented Jan 13, 2021

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
Copy link
Collaborator

@swift-ci swift-ci commented Jan 18, 2021

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
Copy link
Collaborator

@swift-ci swift-ci commented Jan 19, 2021

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
Copy link
Member Author

@CodaFi CodaFi commented Jan 19, 2021

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
Copy link
Collaborator

@swift-ci swift-ci commented Jan 20, 2021

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
Copy link
Member Author

@CodaFi CodaFi commented Jan 20, 2021

Yes, that's right.

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Jan 27, 2021

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
Copy link
Collaborator

@swift-ci swift-ci commented Feb 22, 2021

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
Copy link
Member Author

@CodaFi CodaFi commented Feb 23, 2021

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.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants