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

[Dave Abrahams]

on Tue Jul 19 2016, 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`?  

Only if we were unable to determine that CXX imports were
uniquely-referenced.

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

That's not really the point.  The point is to find out whether it's
*known* to be a unique reference.  False negatives are part of the
game.

       isKnownUniquelyReferenced

would work.

> But I would probably suggest that we just go with just
> `isUniquelyReferenced` and mention the Swift-only requirement in the
> documentation.

It's not a requirement, as in “precondition.”  The point of having
`NonObjC` in this API is actually that it *does* work on ObjC
references, by returning false.

> - 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?

       isUniqueReference

would work for ManagedBufferPointer.

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

Maybe, but it's not crucial.

-- 
Dave