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

Jump to bottom

[SR-9800] Ambiguity with calling UnsafePointer(someMutablePointer) #52224

Closed
lilyball mannequin opened this issue Jan 29, 2019 · 13 comments
Closed

[SR-9800] Ambiguity with calling UnsafePointer(someMutablePointer) #52224

lilyball mannequin opened this issue Jan 29, 2019 · 13 comments
Assignees
@lilyball
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression swift 5.0 type checker Area → compiler: Semantic analysis

Comments

@lilyball
Copy link
Mannequin

lilyball mannequin commented Jan 29, 2019

Previous ID SR-9800
Radar rdar://problem/47706090
Original Reporter @lilyball
Type Bug
Status Resolved
Resolution Done
Environment

Apple Swift version 5.0 (swiftlang-1001.0.45.7 clang-1001.0.37.7)
Target: x86_64-apple-darwin18.2.0
ABI version: 0.6

Xcode 10.2 beta (10P82s)

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, 5.0Regression, TypeChecker
Assignee @lilyball
Priority Medium

md5: 65cb31e64243a6f3b139ee42c7599890

Issue Description:

If I have code that converts an UnsafeMutablePointer to an UnsafePointer and passes the result to an overloaded function that takes multiple types of UnsafePointer (such as String.init(cString:)) this throws an ambiguity error with the Swift 5 compiler. This seems to be a straight-up bug in method resolution because one of the two methods it thinks is ambiguous is UnsafePointer.init(_ other: Self), even though that can't possibly apply as the argument is an UnsafeMutablePointer.

Reproduction:

let buf = UnsafeMutablePointer<CChar>.allocate(capacity: 1)
func foo(_: UnsafePointer<CChar>) {}
func foo(_: UnsafePointer<UInt8>) {}
_ = foo(UnsafePointer(buf))

This compiles with the Swift 4.2 compiler, but with the Swift 5 compiler (even in Swift 4 mode) it throws an error:

unnamed.swift:4:9: error: ambiguous use of 'init(_:)'
_ = foo(UnsafePointer(buf))
        ^
Swift._Pointer:6:12: note: found this candidate
    public init(_ other: Self)
           ^
Swift._Pointer:8:12: note: found this candidate
    public init<T>(_ other: UnsafeMutablePointer<T>)
           ^
@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Jan 29, 2019

Explicitly specifying foo(UnsafePointer<CChar>(buf)) works around this error but it shouldn't be necessary.

@belkadan
Copy link
Contributor

Ugh, I think it's trying to pass a pointer to the pointer value, per SE-0205. @xedin, @jckarter, any ideas?

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Jan 30, 2019

SE-0205 doesn't look related to me; that introduces overloads of withUnsafePointer and withUnsafeBytes but neither is being called here.

@moiseev
Copy link
Mannequin

moiseev mannequin commented Jan 31, 2019

`public init(_ other: Self)` part is not clear to me, but the expression seems ambiguous. Since there is an initializer that converts any `UnsafeMutablePointer<T>` into any other `UnsafePointer<U>`, and multiple functions accepting various UnsafePointer's which one should be chosen? Maybe an invalid diagnostic, but the behavior looks correct to me.

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Jan 31, 2019

Oh you're right, the real issue here is that there now exists an UnsafePointer.init<T>(_ other: UnsafeMutablePointer<T>) that can convert any UnsafeMutablePointer into the receiver.

I think that's incorrect. This completely backtracks on the whole memory-rebinding stuff we introduced before. And from the docstring it seems to have been a mistake:

/// Creates a new pointer from the given mutable pointer.
///
/// Use this initializer to explicitly convert `other` to an `UnsafeRawPointer`
/// instance. This initializer creates a new pointer to the same address as
/// `other` and performs no allocation or copying.
///
/// - Parameter other: The typed pointer to convert.

This docstring suggests that the initializer was moved from UnsafeRawPointer into the _Pointer protocol without actually thinking about it. The _Pointer protocol should have an init(_ other: UnsafeMutablePointer<Pointee>) but it definitely should not have a generic initializer.

@xedin
Copy link
Member

xedin commented Jan 31, 2019

I think this is something which has been fixed in 5 to be ambiguous and previous behavior was due to the solver trying overloads in the different order (init before foo) which incorrectly filtered out one of the partial solutions.

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Jan 31, 2019

Among other things, this initializer lets me write stuff like

var s = "foo"
let x = UnsafePointer<Int>(&s)

which is clearly wrong.

@xedin
Copy link
Member

xedin commented Jan 31, 2019

Or there is a new overload which makes this ambiguous 🙂

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Jan 31, 2019

Looks like this was introduced in #17952

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Jan 31, 2019

CC @airspeedswift

@moiseev
Copy link
Mannequin

moiseev mannequin commented Feb 1, 2019

#22288

@LucianoPAlmeida
Copy link
Collaborator

Ping PR is merged but this still open, should this be resolved? 🙂

@xedin
Copy link
Member

xedin commented Jun 15, 2020

Indeed this should be resolved, thanks, @LucianoPAlmeida! Please verify using a snapshot of master and close.

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

@lilyball lilyball

Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression swift 5.0 type checker Area → compiler: Semantic analysis
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@xedin @belkadan @LucianoPAlmeida @AnthonyLatsis