Pitch: Remove `NonObjectiveCBase` and replace `isUniquelyReferenced` by `isUniquelyReferencedUnsafe`


(Arnold Schwaighofer) #1

https://bugs.swift.org/browse/SR-1962

# Remove `NonObjectiveCBase` and replace `isUniquelyReferenced` by `isUniquelyReferencedUnsafe`

## Introduction

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

## 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.

## Proposed solution

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

## Detailed design

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-@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.

## 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.

## Alternatives considered

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


(Dmitri Gribenko) #2

Thank you for this proposal!

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

···

On Sat, Jul 16, 2016 at 12:47 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

## Proposed solution

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

--
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@gmail.com>*/


(Jaden Geller) #3

Some explanation of the benefit of keeping both `isUniquelyReferencedUnsafe` and `isUniqueReferencedNonObjC` would be useful. I’m not entirely sure why the latter is useful. If it is useful, I’m not sure why we can’t just have `isUniquelyReferencedUnsafe` and `isObjC`.

···

On Jul 16, 2016, at 1:21 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

On Sat, Jul 16, 2016 at 12:47 PM, Arnold Schwaighofer via > swift-evolution <swift-evolution@swift.org> wrote:

## Proposed solution

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

Thank you for this proposal!

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@gmail.com>*/
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Arnold Schwaighofer) #4

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-@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-@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@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@gmail.com>*/


(Arnold Schwaighofer) #5

`isUniqueReferencedNonObjC` is useful if our storage could be other a native Swift class or an objective-c class instance.
An example of this is Array's storage: It could either be a native Swift class instance or if the array was bridged from cocoa an NSArray instance. Before we write to the array we check that it is uniquely referenced and not an NSArray. If either checks fail we copy the contents to a new native Swift array.
You are correct that semantically we could compose the latter. But, in some situations, this code should be fast, for example the array code I mentioned: the overhead of two runtime calls vs one matters.

···

On Jul 16, 2016, at 3:49 PM, Jaden Geller <jgeller@caltech.edu> wrote:

Some explanation of the benefit of keeping both `isUniquelyReferencedUnsafe` and `isUniqueReferencedNonObjC` would be useful. I’m not entirely sure why the latter is useful. If it is useful, I’m not sure why we can’t just have `isUniquelyReferencedUnsafe` and `isObjC`.

On Jul 16, 2016, at 1:21 PM, Dmitri Gribenko via swift-evolution <swift-evolution@swift.org> wrote:

On Sat, Jul 16, 2016 at 12:47 PM, Arnold Schwaighofer via >> swift-evolution <swift-evolution@swift.org> wrote:

## Proposed solution

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

Thank you for this proposal!

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@gmail.com>*/
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Andrew Trick) #6

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Frankly, the type was a nicer way to do this, by I understand removing it from the API surface.

-Andy

···

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

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


(Arnold Schwaighofer) #7

Thank you for the feedback. Answers online.

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

···

Sent from my iPhone

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

Frankly, the type was a nicer way to do this, by I understand removing it from the API surface.

-Andy


(Andrew Trick) #8

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

-Andy

···

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com> wrote:

Thank you for the feedback. Answers online.

Sent from my iPhone

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]


(Arnold Schwaighofer) #9

Thank you for the feedback. Answers online.

Sent from my iPhone

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

The unsafe version would allow us to emit more efficient code (in the future) for a static unknown object type but we know it is a native type (but not which so we can't just cast it, this is public API so we can't cast to Builtin.NativeObject).

  var self: AnyObject // really: AnyNativeObject
  ...
  if (!unsafeIsUniquelyReferenced(&self))
    self = self.copy()
  }

I admit this is somewhat contrived and am happy to just nuke the API if we agree there is no value in the use case above.

···

Sent from my iPhone

On Jul 16, 2016, at 8:45 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:


(Andrew Trick) #10

There is no sense advertising this API under some new name if it hasn’t even been implemented. The API can be added when it makes sense.

+1 for eliminating it.

-Andy

···

On Jul 16, 2016, at 9:17 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 8:45 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com <mailto:aschwaighofer@apple.com>> wrote:

Thank you for the feedback. Answers online.

Sent from my iPhone

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com <mailto:atrick@apple.com>> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

The unsafe version would allow us to emit more efficient code (in the future) for a static unknown object type but we know it is a native type (but not which so we can't just cast it, this is public API so we can't cast to Builtin.NativeObject).

  var self: AnyObject // really: AnyNativeObject
  ...
  if (!unsafeIsUniquelyReferenced(&self))
    self = self.copy()
  }

I admit this is somewhat contrived and am happy to just nuke the API if we agree there is no value in the use case above.


(Arnold Schwaighofer) #11

Thank you for the feedback. Answers online.

Sent from my iPhone

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

The unsafe version would allow us to emit more efficient code (in the future) for a static unknown object type but we know it is a native type (but not which so we can't just cast it, this is public API so we can't cast to Builtin.NativeObject).

  var self: AnyObject // really: AnyNativeObject
  ...
  if (!unsafeIsUniquelyReferenced(&self))
    self = self.copy()
  }

I admit this is somewhat contrived and am happy to just nuke the API if we agree there is no value in the use case above.

There is no sense advertising this API under some new name if it hasn’t even been implemented. The API can be added when it makes sense.

+1 for eliminating it.

Today you can implement something similar using NonObjectiveCBase as your base class:

var self: NonObjectiveCBase
...
if (isUniquelyReferenced(&self) {...}

And get the runtime performance of the native check.

If we implemented 'unsafeIsUniquelyReferenced' we would just unsafeBitCast the 'object' argument to Builtin.NativeObject and get the same performance.

I admit that this may be far fetched but I am trying to anticipated possible use cases that work today.

That use case will still work after nuking the API using the 'NonObjC' variant albeit slightly slower.

If we need to support it with best performance we can always bring the API back as you said.

+1 for nuking it from me

I will change the proposal.

···

Sent from my iPhone

On Jul 16, 2016, at 9:23 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 9:17 PM, Arnold <aschwaighofer@apple.com> wrote:
On Jul 16, 2016, at 8:45 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:


(Arnold Schwaighofer) #12

Sent from my iPhone

Thank you for the feedback. Answers online.

Sent from my iPhone

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

The unsafe version would allow us to emit more efficient code (in the future) for a static unknown object type but we know it is a native type (but not which so we can't just cast it, this is public API so we can't cast to Builtin.NativeObject).

  var self: AnyObject // really: AnyNativeObject
  ...
  if (!unsafeIsUniquelyReferenced(&self))
    self = self.copy()
  }

I admit this is somewhat contrived and am happy to just nuke the API if we agree there is no value in the use case above.

There is no sense advertising this API under some new name if it hasn’t even been implemented. The API can be added when it makes sense.

+1 for eliminating it.

Today you can implement something similar using NonObjectiveCBase as your base class:

var self: NonObjectiveCBase
...
if (isUniquelyReferenced(&self) {...}

And get the runtime performance of the native check.

Actually, this code could just implement their own 'class NonObjCBase' base class and just use isUniquelyReferencedNonObj for the same performance as before ...

···

Sent from my iPhone

On Jul 16, 2016, at 9:41 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 9:23 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 9:17 PM, Arnold <aschwaighofer@apple.com> wrote:
On Jul 16, 2016, at 8:45 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

If we implemented 'unsafeIsUniquelyReferenced' we would just unsafeBitCast the 'object' argument to Builtin.NativeObject and get the same performance.

I admit that this may be far fetched but I am trying to anticipated possible use cases that work today.

That use case will still work after nuking the API using the 'NonObjC' variant albeit slightly slower.

If we need to support it with best performance we can always bring the API back as you said.

+1 for nuking it from me

I will change the proposal.


(Arnold Schwaighofer) #13

Updated proposal:

Remove NonObjectiveCBase and isUniquelyReferenced

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/tree/remove_nonobjectivecbase#introduction>Introduction

Remove NonObjectiveCBase and isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T). isUniquelyReferenced can be replaced by isUniquelyReferencedNonObjC<T: AnyObject>(_ object: T). This replacement is as performant as the call to isUniquelyReferenced in cases where the compiler has static knowledge that the type of object is a native Swift class. This change will remove surface API.

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/tree/remove_nonobjectivecbase#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 isUniquelyReferencedNonObjC in the future.

class SwiftKlazz {}

expectTrue(isUniquelyReferencedNonObjC(SwiftKlazz()))
Removing isUniquelyReferenced<T : NonObjectiveCBase> will allow us to remove the NonObjectiveCBase class from the standard library thereby further shrinking API surface.

<https://github.com/aschwaighofer/swift-evolution/tree/remove_nonobjectivecbase#proposed-solution>Proposed solution

Remove isUniquelyReferenced<T : NonObjectiveCBase> and remove the NonObjectiveCBase class from the standard library. Clients of the the isUniquelyReferenced API can be migrated to use isUniquelyReferencedNonObjC. In most cases -- where the type of the object parameter is statically known to be a native non-@objc class -- the resulting code will have identical performance characteristics. In cases where the type is statically not known it will have the slight overhead of first checking that the dynamic type is not an @objc class.

<https://github.com/aschwaighofer/swift-evolution/tree/remove_nonobjectivecbase#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 isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T>.

<https://github.com/aschwaighofer/swift-evolution/tree/remove_nonobjectivecbase#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 isUniquelyReferencedNonObjC. The old API will be marked unavailable to help migration.

<https://github.com/aschwaighofer/swift-evolution/tree/remove_nonobjectivecbase#alternatives-considered>Alternatives considered

Leave the status quo and pay for type safety with additional API surface. Another alternative we considered -- the first version of this proposal -- was to replace the isUniquelyReferenced API by an isUniquelyReferencedUnsafe<T: AnyObject>(_ object: T) API that would assume the object to be a non-@objc class and only check this precondition under -Onone. There is however no good reason to keep this API given that the isUniquelyReferencedNonObjC is as performant when the type is statically known to be non-@objc class.

···

On Jul 16, 2016, at 10:12 PM, Arnold via swift-evolution <swift-evolution@swift.org> wrote:

Sent from my iPhone

On Jul 16, 2016, at 9:41 PM, Arnold <aschwaighofer@apple.com> wrote:

Sent from my iPhone

On Jul 16, 2016, at 9:23 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 9:17 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 8:45 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com> wrote:

Thank you for the feedback. Answers online.

Sent from my iPhone

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

The unsafe version would allow us to emit more efficient code (in the future) for a static unknown object type but we know it is a native type (but not which so we can't just cast it, this is public API so we can't cast to Builtin.NativeObject).

  var self: AnyObject // really: AnyNativeObject
  ...
  if (!unsafeIsUniquelyReferenced(&self))
    self = self.copy()
  }

I admit this is somewhat contrived and am happy to just nuke the API if we agree there is no value in the use case above.

There is no sense advertising this API under some new name if it hasn’t even been implemented. The API can be added when it makes sense.

+1 for eliminating it.

Today you can implement something similar using NonObjectiveCBase as your base class:

var self: NonObjectiveCBase
...
if (isUniquelyReferenced(&self) {...}

And get the runtime performance of the native check.

Actually, this code could just implement their own 'class NonObjCBase' base class and just use isUniquelyReferencedNonObj for the same performance as before ...

If we implemented 'unsafeIsUniquelyReferenced' we would just unsafeBitCast the 'object' argument to Builtin.NativeObject and get the same performance.

I admit that this may be far fetched but I am trying to anticipated possible use cases that work today.

That use case will still work after nuking the API using the 'NonObjC' variant albeit slightly slower.

If we need to support it with best performance we can always bring the API back as you said.

+1 for nuking it from me

I will change the proposal.

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Arnold Schwaighofer) #14

TL;DR: Remove the isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T) API. All its current use cases can be satisfied equally performant by the isUniquelyReferencedNonObjC<T : AnyObject>(_ object: T) API.

This will reduce API surface by both the NonObjectiveCBase class and the isUniquelyReferenced API.

I think this is pretty uncontroversial, too?

[Today’s implementation of both APIs is the same builtin call. The compiler can optimize out the check for objective-c class ancestry when it sees that the static type of ‘object’ is not an @objc-class.]

···

On Jul 16, 2016, at 10:50 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

Updated proposal:

Remove NonObjectiveCBase and isUniquelyReferenced

  • Proposal: SE-0000
  • Author: Arnold Schwaighofer
  • Status: Pitch
  • Review manager: TBD
Introduction

Remove NonObjectiveCBase and isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T). isUniquelyReferenced can be replaced by isUniquelyReferencedNonObjC<T: AnyObject>(_ object: T). This replacement is as performant as the call to isUniquelyReferenced in cases where the compiler has static knowledge that the type of object is a native Swift class. This change will remove surface API.

  • Swift-evolution thread: Pitch
  • Swift bug: SR-1962
  • Branch with change to stdlib: remove_nonobjectivecbase
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 isUniquelyReferencedNonObjC in the future.

class
SwiftKlazz {}

expectTrue(
isUniquelyReferencedNonObjC(SwiftKlazz()))
Removing isUniquelyReferenced<T : NonObjectiveCBase> will allow us to remove the NonObjectiveCBase class from the standard library thereby further shrinking API surface.

Proposed solution

Remove isUniquelyReferenced<T : NonObjectiveCBase> and remove the NonObjectiveCBase class from the standard library. Clients of the the isUniquelyReferenced API can be migrated to use isUniquelyReferencedNonObjC. In most cases -- where the type of the object parameter is statically known to be a native non-@objc class -- the resulting code will have identical performance characteristics. In cases where the type is statically not known it will have the slight overhead of first checking that the dynamic type is not an @objc class.

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 isUniquelyReferenced<T: NonObjectiveCBase>(_ object: T>.

Impact on existing code

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

Alternatives considered

Leave the status quo and pay for type safety with additional API surface. Another alternative we considered -- the first version of this proposal -- was to replace the isUniquelyReferenced API by an isUniquelyReferencedUnsafe<T: AnyObject>(_ object: T) API that would assume the object to be a non-@objc class and only check this precondition under -Onone. There is however no good reason to keep this API given that the isUniquelyReferencedNonObjC is as performant when the type is statically known to be non-@objc class.

On Jul 16, 2016, at 10:12 PM, Arnold via swift-evolution <swift-evolution@swift.org> wrote:

Sent from my iPhone

On Jul 16, 2016, at 9:41 PM, Arnold <aschwaighofer@apple.com> wrote:

Sent from my iPhone

On Jul 16, 2016, at 9:23 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 9:17 PM, Arnold <aschwaighofer@apple.com> wrote:

On Jul 16, 2016, at 8:45 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 8:36 PM, Arnold <aschwaighofer@apple.com> wrote:

Thank you for the feedback. Answers online.

Sent from my iPhone

On Jul 16, 2016, at 7:38 PM, Andrew Trick <atrick@apple.com> wrote:

On Jul 16, 2016, at 6:46 PM, Arnold Schwaighofer via swift-evolution <swift-evolution@swift.org> wrote:

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

So we’ll have:

- isUniquelyReferencedNonObjC(object): true iff object is uniquely referenced and NonObjC

- isUniquelyReferencedUnsafe(object): true iff object is uniquely reference, assert NonObjC

I’m going to be picky. The “Unsafe” suffix doesn’t make sense to me. If you think this is an unsafe API then it should be:
“unsafeIsUniquelyReferenced”.

But I don’t really see how it is unsafe in the usual sense. If you want to convey that the programmer needs to satisfy some precondition, which is not generally associated with unsafety, then it should be:
“isUniquelyReferencedAssumingNonObjC”

Makes sense to me. I think it is unsafe in the sense if you don't satisfy the precondition the resulting behavior is undefined in modes other than -Onone and not checked by a precondition predicate that traps.

unsafeIsUniquelyReferenced

I find it kind of nice to recognize a predicate by the 'is' prefix. All unsafe APIs start with the word unsafe though. I could not find an unsafe freestanding predicate.

[As the implementor of the underlying builtin you may remember that it is not actually undefined and will return a implementation defined value (false) for objc classes. But we might not want to guarantee this going forward.]

Oh yeah. I think I only kept the two versions for fear of breaking the API. Since you’re renaming the second one anyway, why not just delete it with a fixit that it's renamed to isUniquelyReferencedNonObjC?

The “assuming” version of the API is extremely confusing in addition to being useless.

The unsafe version would allow us to emit more efficient code (in the future) for a static unknown object type but we know it is a native type (but not which so we can't just cast it, this is public API so we can't cast to Builtin.NativeObject).

  var self: AnyObject // really: AnyNativeObject
  ...
  if (!unsafeIsUniquelyReferenced(&self))
    self = self.copy()
  }

I admit this is somewhat contrived and am happy to just nuke the API if we agree there is no value in the use case above.

There is no sense advertising this API under some new name if it hasn’t even been implemented. The API can be added when it makes sense.

+1 for eliminating it.

Today you can implement something similar using NonObjectiveCBase as your base class:

var self: NonObjectiveCBase
...
if (isUniquelyReferenced(&self) {...}

And get the runtime performance of the native check.

Actually, this code could just implement their own 'class NonObjCBase' base class and just use isUniquelyReferencedNonObj for the same performance as before ...

If we implemented 'unsafeIsUniquelyReferenced' we would just unsafeBitCast the 'object' argument to Builtin.NativeObject and get the same performance.

I admit that this may be far fetched but I am trying to anticipated possible use cases that work today.

That use case will still work after nuking the API using the 'NonObjC' variant albeit slightly slower.

If we need to support it with best performance we can always bring the API back as you said.

+1 for nuking it from me

I will change the proposal.

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution