[swift-evolution] [swift-evolution-announce] [Review] SE-0125: Remove NonObjectiveCBase and isUniquelyReferenced

[Arnold Schwaighofer]

> On Jul 20, 2016, at 5:55 AM, Arnold Schwaighofer via swift-evolution <swift-evolution at swift.org> wrote:
> 
>> 
>> On Jul 19, 2016, at 11:06 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>> 
>> On Tue, Jul 19, 2016 at 10:51 PM, Chris Lattner <clattner at apple.com> wrote:
>>> The review of "SE-0125: Remove NonObjectiveCBase and isUniquelyReferenced" begins now and runs through July 22. The proposal is available here:
>>> 
>>>       https://github.com/apple/swift-evolution/blob/master/proposals/0125-remove-nonobjectivecbase.md
>> 
>> I like the API simplification.  A few thoughts that I have after
>> reading the proposal that could take the simplification it even
>> further:
>> 
>> - I find it a little strange to see a mention of Objective-C, and a
>> negation in `isUniquelyReferencedNonObjC`.  For example, if we get C++
>> import, would we need to rename it to
>> `isUniquelyReferencedNonObjCAndNonCXX`?  I think that is the issue
>> with negation.  If we want to emphasize that the API will work only
>> with Swift types, we can do something
>> `isUniquelyReferencedSwiftReference`.  But I would probably suggest
>> that we just go with just `isUniquelyReferenced` and mention the
>> Swift-only requirement in the documentation.
> 
> isUniquelyReferencedNonObjC checks that the object is a uniquely referenced swift only class. It works on non- at objc classes but will return false: The API will work for @objc-classes but return false.
> 
> The reason for this combination is data structures like Array which might hold an objc class which is not mutable. On a mutating operation you check is it uniquely reference and a swift class - if the answer is yes and you have enough storage you store to the current instance.
> 
>  expectTrue(isUniquelyReferencedNonObjc(SwiftKlazz()))
>  expectFalse(isUniquelyReferencedNonObjc(ObjcKlazz()))
>  var y = SwiftKlazz()
>  var z = [Any]
>  z.append(y)
>  z.append(y)
>  expectFalse(isUniquelyReferencedNonObjc(y)
> 
> The simplification of this proposal just to remove the variant that had the NonObjectiveCBase prerequisite.


Your critique of the negation still holds though.

So maybe we still rename it from isUniquelyReferencedNonObjC to isUniquelyReferencedSwiftReference?


> 
> 
> 
>> 
>> - I find the use of different names in `isUniquelyReferenced()` and
>> `ManagedBufferPointer.holdsUniqueReference()` to be strange.  Why not
>> call the latter `ManagedBufferPointer. isUniquelyReferenced()`?  It is
>> true that we are not checking that pointer is not uniquely referenced,
>> if we want to emphasize that, maybe something like
>> `ManagedBufferPointer.isBufferUniquelyReferenced()` or
>> `isPointeeUniquelyReferenced()` will work?
> 
> Okay I think isBufferUniquelyReferenced is good. I’ll add that change to the proposal.
> 
> 
>> 
>> The reason why I'm suggesting this is that `isUniquelyReferenced` and
>> `holdsUniqueReference` don't immediately strike me as performing the
>> same operation, the immediate impression I get is that these
>> operations are related, but subtly different, and it is not obvious
>> what the difference is (but after reading the docs I figure out that
>> there is no difference... until I need to use these APIs again).
>> 
>> - We have `ManagedBufferPointer.holdsUniqueOrPinnedReference()`, but
>> don't have an equivalent free function `isUniquelyReferencedOrPinned`
>> (we actually have one, but it is internal).  Do we need a public one?
>> If we do, we could as well add it as a part of this proposal, while we
>> are cleaning up this library subsystem.
> 
> No we don’t one. . We have not exposed pinning outside of the standard library (because we don’t have a design for it).
> 
> I will remove it.
> 
>> 
>> 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>*/
> 
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution