[swift-evolution] Pitch: Remove `NonObjectiveCBase` and replace `isUniquelyReferenced` by `isUniquelyReferencedUnsafe`

[Arnold Schwaighofer]

Thank you Dmitri for your feedback. Updated draft below.

https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md

Remove NonObjectiveCBase and replace isUniquelyReferenced by isUniquelyReferencedUnsafe

Proposal: SE-0000 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md>
Author: Arnold Schwaighofer <https://github.com/aschwaighofer>
Status: Pitch
Review manager: TBD
 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md#introduction>Introduction

Remove NonObjectiveCBase and replace isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T) byisUniquelyReferencedUnsafe<T: AnyObject>(_ object: T). This will remove surface API. Instead of a type check dynamically check the non- at objc constraint under -Onone.

Swift-evolution thread: Pitch <https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160711/024515.html>
Swift bug: SR-1962 <http://bugs.swift.org/browse/SR-1962>
Branch with change to stdlib: remove_nonobjectivecbase <https://github.com/aschwaighofer/swift/tree/remove_nonobjectivecbase>
 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md#motivation>Motivation

Today we have isUniquelyReferenced which only works on subclasses of NonObjectiveCBase, and we have isUniquelyReferencedNonObjC which also works on @objc classes.

class SwiftKlazz : NonObjectiveCBase {}
class ObjcKlazz : NSObject {}

expectTrue(isUniquelyReferenced(SwiftKlazz()))
expectFalse(isUniquelyReferencedNonObjC(ObjcKlazz()))

// Would not compile:
expectFalse(isUniquelyReferenced(ObjcKlazz()))
In most cases we expect developers to be using the ManagedBufferPointer type. In cases where they want to use a custom class they would use isUniquelyReferenced today and can use isUniquelyReferencedUnsafe in the future.

class SwiftKlazz : NonObjectiveCBase {}
class ObjcKlazz : NSObject {}

expectTrue(isUniquelyReferencedUnsafe(SwiftKlazz()))
// Would trap under -Onone:
expectFalse(isUniquelyReferencedUnsafe(ObjcKlazz()))
Replacing isUniquelyReferenced<T : NonObjectiveCBase> by isUniquelyReferencedUnsafe<T: AnyObject> will allow us to remove the NonObjectiveCBase class from the standard library thereby shrink API surface. We argue that trading type safety for less API surface is a good trade-off to make with this low-level API.

 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md#proposed-solution>Proposed solution

Replace isUniquelyReferenced<T : NonObjectiveCBase> by isUniquelyReferencedUnsafe<T: AnyObject> and remove the NonObjectiveCBase class from the standard library.

 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md#detailed-design>Detailed design

Todays APIs that can be used to check uniqueness is the family of isUniquelyReferenced functions.

/// Returns `true` iff `object` is a non-`@objc` class instance with
/// a single strong reference.
///
/// * Does *not* modify `object`; the use of `inout` is an
///   implementation artifact.
/// * If `object` is an Objective-C class instance, returns `false`.
/// * Weak references do not affect the result of this function.
///
/// Useful for implementing the copy-on-write optimization for the
/// deep storage of value types:
///
///     mutating func modifyMe(_ arg: X) {
///       if isUniquelyReferencedNonObjC(&myStorage) {
///         myStorage.modifyInPlace(arg)
///       }
///       else {
///         myStorage = self.createModified(myStorage, arg)
///       }
///     }
public func isUniquelyReferencedNonObjC<T : AnyObject>(_ object: inout T) -> Bool
public func isUniquelyReferencedNonObjC<T : AnyObject>(_ object: inout T?) -> Bool

/// A common base class for classes that need to be non-`@objc`,
/// recognizably in the type system.
public class NonObjectiveCBase {
  public init() {}
}

public func isUniquelyReferenced<T : NonObjectiveCBase>(
  _ object: inout T
) -> Bool
And the somewhat higher level APIs that can be used to model a storage with several elements ManagedBufferPointer.

/// Contains a buffer object, and provides access to an instance of
/// `Header` and contiguous storage for an arbitrary number of
/// `Element` instances stored in that buffer.
///
/// For most purposes, the `ManagedBuffer` class works fine for this
/// purpose, and can simply be used on its own.  However, in cases
/// where objects of various different classes must serve as storage,
/// `ManagedBufferPointer` is needed.
///
/// A valid buffer class is non-`@objc`, with no declared stored
///   properties.  Its `deinit` must destroy its
///   stored `Header` and any constructed `Element`s.
/// `Header` and contiguous storage for an arbitrary number of
/// `Element` instances stored in that buffer.
public struct ManagedBufferPointer<Header, Element> : Equatable {
  /// Create with new storage containing an initial `Header` and space
  /// for at least `minimumCapacity` `element`s.
  ///
  /// - parameter bufferClass: The class of the object used for storage.
  /// - parameter minimumCapacity: The minimum number of `Element`s that
  ///   must be able to be stored in the new buffer.
  /// - parameter initialHeader: A function that produces the initial
  ///   `Header` instance stored in the buffer, given the `buffer`
  ///   object and a function that can be called on it to get the actual
  ///   number of allocated elements.
  ///
  /// - Precondition: `minimumCapacity >= 0`, and the type indicated by
  ///   `bufferClass` is a non-`@objc` class with no declared stored
  ///   properties.  The `deinit` of `bufferClass` must destroy its
  ///   stored `Header` and any constructed `Element`s.
  public init(
    bufferClass: AnyClass,
    minimumCapacity: Int,
    initialHeader: @noescape (buffer: AnyObject, capacity: @noescape (AnyObject) -> Int) throws -> Header
  ) rethrows

  /// Returns `true` iff `self` holds the only strong reference to its buffer.
  ///
  /// See `isUniquelyReferenced` for details.
  public mutating func holdsUniqueReference() -> Bool

  /// Returns `true` iff either `self` holds the only strong reference
  /// to its buffer or the pinned has been 'pinned'.
  ///
  /// See `isUniquelyReferenced` for details.
  public mutating func holdsUniqueOrPinnedReference() -> Bool

  internal var _nativeBuffer: Builtin.NativeObject
}

/// A class whose instances contain a property of type `Header` and raw
/// storage for an array of `Element`, whose size is determined at
/// instance creation.
public class ManagedBuffer<Header, Element>
  : ManagedProtoBuffer<Header, Element> {

  /// Create a new instance of the most-derived class, calling
  /// `initialHeader` on the partially-constructed object to
  /// generate an initial `Header`.
  public final class func create(
    minimumCapacity: Int,
    initialHeader: @noescape (ManagedProtoBuffer<Header, Element>) throws -> Header
  ) rethrows -> ManagedBuffer<Header, Element> {

    let p = try ManagedBufferPointer<Header, Element>(
      bufferClass: self,
      minimumCapacity: minimumCapacity,
      initialHeader: { buffer, _ in
        try initialHeader(
          unsafeDowncast(buffer, to: ManagedProtoBuffer<Header, Element>.self))
      })

    return unsafeDowncast(p.buffer, to: ManagedBuffer<Header, Element>.self)
  }
}
We propose to remove the NonObjectiveCBase class and change isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T> to:

/// Returns `true` iff `object` is a non-`@objc` class instance with a single
/// strong reference. `object` is assumed to be a non-`@objc` class instance.
/// In debug mode this function will check this assumption. Otherwise, it is
/// undefined what happens.
///
/// * Does *not* modify `object`; the use of `inout` is an
///   implementation artifact.
/// * Weak references do not affect the result of this function.
///
/// Useful for implementing the copy-on-write optimization for the
/// deep storage of value types:
///
///     mutating func modifyMe(_ arg: X) {
///       if isUniquelyReferencedUnsafe(&myStorage) {
///         myStorage.modifyInPlace(arg)
///       }
///       else {
///         myStorage = myStorage.createModified(arg)
///       }
///     }
///
/// This function is safe to use for `mutating` functions in
/// multithreaded code because a false positive would imply that there
/// is already a user-level data race on the value being mutated.
public func isUniquelyReferencedUnsafe<T : AnyObject>(
  _ object: inout T
) -> Bool {
  _debugPrecondition(
    _usesNativeSwiftReferenceCounting(object.dynamicType),
    "instance must be a non- at objc class instance")
  return _isUnique(&object)
}
Note, that today at -O we would actually not cause undefined behavior but rather just return false. We don't want to guarantee this in the future so the comment specifies undefined behavior.

 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md#impact-on-existing-code>Impact on existing code

Existing code that uses isUniquelyReferenced will need to remove the NonObjectiveCBase base class and replace calls to isUniquelyReferenced by isUniquelyReferencedUnsafe. The old API will be marked unavailable to help migration.

 <https://github.com/aschwaighofer/swift-evolution/blob/remove_nonobjectivecbase/proposals/0000-remove-nonobjectivecbase.md#alternatives-considered>Alternatives considered

Leave the status quo and pay for type safety with additional API surface.


> On Jul 16, 2016, at 1:21 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> 
> For presentation and clarity, could you show the full family of
> `isUniquely*` functions in the design section, including those
> functions that you are not proposing to change?  This will make it
> easier to see what choices users will get.  It would be also great to
> include the API of similar ManagedBuffer and ManagedBufferPointer
> APIs, if any exist.
> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160716/8a2e6982/attachment.html>