SR-1485: Change Unmanaged to use UnsafePointer

Author: Russ Bishop

stdlib/private/SwiftPrivatePthreadExtras/SwiftPrivatePthreadExtras.swift

@@ -52,8 +52,8 @@ internal func invokeBlockContext(
   _ contextAsVoidPointer: UnsafeMutablePointer<Void>?
 ) -> UnsafeMutablePointer<Void>! {
   // The context is passed in +1; we're responsible for releasing it.
-  let contextAsOpaque = OpaquePointer(contextAsVoidPointer!)
-  let context = Unmanaged<PthreadBlockContext>.fromOpaque(contextAsOpaque)
+  let context = Unmanaged<PthreadBlockContext>
+    .fromOpaque(contextAsVoidPointer!)
     .takeRetainedValue()
 
   return context.run()
@@ -68,8 +68,7 @@ public func _stdlib_pthread_create_block<Argument, Result>(
   let context = PthreadBlockContextImpl(block: start_routine, arg: arg)
   // We hand ownership off to `invokeBlockContext` through its void context
   // argument.
-  let contextAsOpaque = OpaquePointer(bitPattern: Unmanaged.passRetained(context))
-  let contextAsVoidPointer = UnsafeMutablePointer<Void>(contextAsOpaque)
+  let contextAsVoidPointer = Unmanaged.passRetained(context).toOpaque()
 
   var threadID = _make_pthread_t()
   let result = pthread_create(&threadID, attr,

stdlib/public/core/CTypes.swift

@@ -130,19 +130,6 @@ public struct OpaquePointer : Equatable, Hashable {
     self.init(unwrapped)
   }
 
-  /// Unsafely convert an unmanaged class reference to an opaque
-  /// C pointer.
-  ///
-  /// This operation does not change reference counts.
-  ///
-  ///     let str0: CFString = "boxcar"
-  ///     let bits = OpaquePointer(bitPattern: Unmanaged(withoutRetaining: str0))
-  ///     let str1 = Unmanaged<CFString>(bitPattern: bits).object
-  @_transparent
-  public init<T>(bitPattern bits: Unmanaged<T>) {
-    self = unsafeBitCast(bits._value, to: OpaquePointer.self)
-  }
-
   /// The hash value.
   ///
   /// **Axiom:** `x == y` implies `x.hashValue == y.hashValue`.
@@ -212,3 +199,11 @@ func _memcpy(
 
 @available(*, unavailable, renamed: "OpaquePointer")
 public struct COpaquePointer {}
+
+extension OpaquePointer {
+  @available(*, unavailable, 
+    message:"use 'Unmanaged.toOpaque()' instead")
+  public init<T>(bitPattern bits: Unmanaged<T>) {
+    Builtin.unreachable()
+  }
+}

stdlib/public/core/Runtime.swift.gyb

@@ -23,16 +23,16 @@ import SwiftShims
 @_transparent
 public // @testable
 func _stdlib_atomicCompareExchangeStrongPtrImpl(
-  object target: UnsafeMutablePointer<OpaquePointer?>,
-  expected: UnsafeMutablePointer<OpaquePointer?>,
-  desired: OpaquePointer?) -> Bool {
+  object target: UnsafeMutablePointer<UnsafeMutablePointer<Void>?>,
+  expected: UnsafeMutablePointer<UnsafeMutablePointer<Void>?>,
+  desired: UnsafeMutablePointer<Void>?) -> Bool {
 
   // We use Builtin.Word here because Builtin.RawPointer can't be nil.
   let (oldValue, won) = Builtin.cmpxchg_seqcst_seqcst_Word(
     target._rawValue,
     UInt(bitPattern: expected.pointee)._builtinWordValue,
     UInt(bitPattern: desired)._builtinWordValue)
-  expected.pointee = OpaquePointer(bitPattern: Int(oldValue))
+  expected.pointee = UnsafeMutablePointer(bitPattern: Int(oldValue))
   return Bool(won)
 }
 
@@ -72,7 +72,7 @@ func _stdlib_atomicCompareExchangeStrongPtr<T>(
   return _stdlib_atomicCompareExchangeStrongPtrImpl(
     object: UnsafeMutablePointer(target),
     expected: UnsafeMutablePointer(expected),
-    desired: OpaquePointer(desired))
+    desired: UnsafeMutablePointer(desired))
 }
 
 @_transparent
@@ -81,8 +81,8 @@ public // @testable
 func _stdlib_atomicInitializeARCRef(
   object target: UnsafeMutablePointer<AnyObject?>,
   desired: AnyObject) -> Bool {
-  var expected: OpaquePointer? = nil
-  let desiredPtr = OpaquePointer(bitPattern: Unmanaged.passRetained(desired))
+  var expected: UnsafeMutablePointer<Void>? = nil
+  let desiredPtr = Unmanaged.passRetained(desired).toOpaque()
   let wonRace = _stdlib_atomicCompareExchangeStrongPtrImpl(
     object: UnsafeMutablePointer(target),
     expected: &expected,
@@ -243,7 +243,8 @@ func _stdlib_atomicLoadARCRef(
   let result = _swift_stdlib_atomicLoadPtrImpl(
     object: UnsafeMutablePointer(target))
   if let unwrapped = result {
-    return Unmanaged<AnyObject>.fromOpaque(unwrapped).takeUnretainedValue()
+    return Unmanaged<AnyObject>.fromOpaque(
+      UnsafePointer(unwrapped)).takeUnretainedValue()
   }
   return nil
 }

stdlib/public/core/Unmanaged.swift

@@ -29,10 +29,22 @@ public struct Unmanaged<Instance : AnyObject> {
   ///
   ///     let str: CFString = Unmanaged.fromOpaque(ptr).takeUnretainedValue()
   @_transparent
-  public static func fromOpaque(_ value: OpaquePointer) -> Unmanaged {
+  public static func fromOpaque(_ value: UnsafePointer<Void>) -> Unmanaged {
     return Unmanaged(_private: unsafeBitCast(value, to: Instance.self))
   }
 
+  /// Unsafely convert an unmanaged class reference to a pointer
+  ///
+  /// This operation does not change reference counts.
+  ///
+  ///     let str0: CFString = "boxcar"
+  ///     let bits = Unmanaged.passUnretained(str0)
+  ///     let str1 = Unmanaged<CFString>(bits).object
+  @_transparent
+  public func toOpaque() -> UnsafeMutablePointer<Void> {
+    return unsafeBitCast(_value, to: UnsafeMutablePointer<Void>.self)
+  }
+
   /// Create an unmanaged reference with an unbalanced retain.
   /// The object will leak if nothing eventually balances the retain.
   ///
@@ -201,3 +213,17 @@ public struct Unmanaged<Instance : AnyObject> {
   }
 #endif
 }
+
+extension Unmanaged {
+  @available(*, unavailable, 
+    message:"use 'fromOpaque(_: UnsafePointer<Void>)' instead")
+  public static func fromOpaque(_ value: OpaquePointer) -> Unmanaged {
+    Builtin.unreachable()
+  }
+  
+  @available(*, unavailable, 
+    message:"use 'toOpaque() -> UnsafePointer<Void>' instead")
+  public func toOpaque() -> OpaquePointer {
+    Builtin.unreachable()
+  }
+}

test/1_stdlib/Unmanaged.swift

@@ -54,6 +54,21 @@ UnmanagedTests.test("_withUnsafeGuaranteedRef/return") {
   }
 }
 
+UnmanagedTests.test("Opaque") {
+  var ref = Foobar()
+  let opaquePtr = Unmanaged.passUnretained(ref).toOpaque()
+  
+  let unknownPtr = Int(bitPattern: opaquePtr)
+  let voidPtr = UnsafePointer<Void>(bitPattern: unknownPtr)
+  expectNotEmpty(voidPtr, "toOpaque must not return null pointer")
+  
+  let unmanaged = Unmanaged<Foobar>.fromOpaque(voidPtr!)
+  expectEqual(
+    ref === unmanaged.takeUnretainedValue(),
+    true,
+    "fromOpaque must return the same reference")
+  
+  _fixLifetime(ref)
+}
 
 runAllTests()
-