Remove isUniquelyReferenced or isUniquelyReferencedNonObjC?


(Chris Eidhof) #1

There are two functions isUniquelyReferencedNonObjC and isUniquelyReferenced, which do exactly the same thing. One has a more constrained type, only accepting Swift objects. The other one accepts ObjC objects as well, but always returns false. Regardless of whether it should (could?) work for ObjC objects, I think this duplication is confusing (it has confused me for a long time, and I’m happy that I can now see they’re implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to do, as the function is useless for ObjC objects.

Chris


(Joe Groff) #2

+1, I think this is just legacy we haven't gotten around to cleaning up.

-Joe

···

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution <swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and isUniquelyReferenced, which do exactly the same thing. One has a more constrained type, only accepting Swift objects. The other one accepts ObjC objects as well, but always returns false. Regardless of whether it should (could?) work for ObjC objects, I think this duplication is confusing (it has confused me for a long time, and I’m happy that I can now see they’re implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to do, as the function is useless for ObjC objects.


(Joe Groff) #3

cc'ing Andy, who can confirm whether the distinction is still necessary.

-Joe

···

On Dec 10, 2015, at 8:56 AM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution <swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and isUniquelyReferenced, which do exactly the same thing. One has a more constrained type, only accepting Swift objects. The other one accepts ObjC objects as well, but always returns false. Regardless of whether it should (could?) work for ObjC objects, I think this duplication is confusing (it has confused me for a long time, and I’m happy that I can now see they’re implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to do, as the function is useless for ObjC objects.

+1, I think this is just legacy we haven't gotten around to cleaning up.


(Andrew Trick) #4

Yes! The most efficient runtime call is already inferred from the object type. So there is no efficiency advantage in using one entry point over the other. I think I left both in place because they were already public API. But I think the “NonObjC” entry point should be removed and documented wherever we document API changes. Maybe CHANGLELOG.md.

Side note: internally we have a Builtin.isUnique_native that force casts Any object to Builtin.NativeObject in order to bypass the ObjC check for unknown types. It’s only useful for ObjC bridged types and we do not expose this through a public API.

Andy

···

On Dec 10, 2015, at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 10, 2015, at 8:56 AM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution <swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and isUniquelyReferenced, which do exactly the same thing. One has a more constrained type, only accepting Swift objects. The other one accepts ObjC objects as well, but always returns false. Regardless of whether it should (could?) work for ObjC objects, I think this duplication is confusing (it has confused me for a long time, and I’m happy that I can now see they’re implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to do, as the function is useless for ObjC objects.

+1, I think this is just legacy we haven't gotten around to cleaning up.

cc'ing Andy, who can confirm whether the distinction is still necessary.


(Chris Eidhof) #5

I think the name isUniquelyReferenced is better, and it should use the constraint to make sure it only works on Swift objects. That way there’s no confusion.

Chris

···

On 10 Dec 2015, at 11:56, Joe Groff <jgroff@apple.com> wrote:

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution <swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and isUniquelyReferenced, which do exactly the same thing. One has a more constrained type, only accepting Swift objects. The other one accepts ObjC objects as well, but always returns false. Regardless of whether it should (could?) work for ObjC objects, I think this duplication is confusing (it has confused me for a long time, and I’m happy that I can now see they’re implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to do, as the function is useless for ObjC objects.

+1, I think this is just legacy we haven't gotten around to cleaning up.

-Joe


(Chris Eidhof) #6

Okay, great!

I can have a stab at this next week, when I’m back at my regular computer. Unless someone else feels like doing it before that, which is fine too =).

Chris

···

On 10 Dec 2015, at 12:38, Andrew Trick <atrick@apple.com> wrote:

On Dec 10, 2015, at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 10, 2015, at 8:56 AM, Joe Groff via swift-evolution <swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution <swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and isUniquelyReferenced, which do exactly the same thing. One has a more constrained type, only accepting Swift objects. The other one accepts ObjC objects as well, but always returns false. Regardless of whether it should (could?) work for ObjC objects, I think this duplication is confusing (it has confused me for a long time, and I’m happy that I can now see they’re implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to do, as the function is useless for ObjC objects.

+1, I think this is just legacy we haven't gotten around to cleaning up.

cc'ing Andy, who can confirm whether the distinction is still necessary.

Yes! The most efficient runtime call is already inferred from the object type. So there is no efficiency advantage in using one entry point over the other. I think I left both in place because they were already public API. But I think the “NonObjC” entry point should be removed and documented wherever we document API changes. Maybe CHANGLELOG.md.

Side note: internally we have a Builtin.isUnique_native that force casts Any object to Builtin.NativeObject in order to bypass the ObjC check for unknown types. It’s only useful for ObjC bridged types and we do not expose this through a public API.


(Joe Groff) #7

I don't think the constraint is desirable. For most ObjC objects we just conservatively return 'false', but it isn't unreasonable to think that some ObjC types might be able to reliably answer isUniquelyReferenced in the future. Furthermore, IIRC the constraint works by requiring a `NonObjC` base class, and enforcing a common base class is not an acceptable design constraint for pure Swift code.

-Joe

···

On Dec 10, 2015, at 9:00 AM, Chris Eidhof <chris@eidhof.nl> wrote:

I think the name isUniquelyReferenced is better, and it should use the constraint to make sure it only works on Swift objects. That way there’s no confusion.


(Chris Eidhof) #8

I didn’t realise that NonObjc was a class, I somehow thought it was a compiler-inserted protocol for Swift objects, haha.

So that’d mean you’d instead have the constraint in the documentation, saying that “this will currently not work on ObjC types”? Seems fair.

Chris

···

On 10 Dec 2015, at 12:03, Joe Groff <jgroff@apple.com> wrote:

On Dec 10, 2015, at 9:00 AM, Chris Eidhof <chris@eidhof.nl> wrote:

I think the name isUniquelyReferenced is better, and it should use the constraint to make sure it only works on Swift objects. That way there’s no confusion.

I don't think the constraint is desirable. For most ObjC objects we just conservatively return 'false', but it isn't unreasonable to think that some ObjC types might be able to reliably answer isUniquelyReferenced in the future. Furthermore, IIRC the constraint works by requiring a `NonObjC` base class, and enforcing a common base class is not an acceptable design constraint for pure Swift code.


(Dmitri Gribenko) #9

Just wanted to point out that like every API change, this needs to be
proposed on swift-evolution.

Dmitri

···

On Thu, Dec 10, 2015 at 11:25 AM, Chris Eidhof via swift-evolution < swift-evolution@swift.org> wrote:

On 10 Dec 2015, at 12:38, Andrew Trick <atrick@apple.com> wrote:

On Dec 10, 2015, at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 10, 2015, at 8:56 AM, Joe Groff via swift-evolution < > swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution < > swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and
isUniquelyReferenced, which do exactly the same thing. One has a more
constrained type, only accepting Swift objects. The other one accepts ObjC
objects as well, but always returns false. Regardless of whether it should
(could?) work for ObjC objects, I think this duplication is confusing (it
has confused me for a long time, and I’m happy that I can now see they’re
implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to
do, as the function is useless for ObjC objects.

+1, I think this is just legacy we haven't gotten around to cleaning up.

cc'ing Andy, who can confirm whether the distinction is still necessary.

Yes! The most efficient runtime call is already inferred from the object
type. So there is no efficiency advantage in using one entry point over the
other. I think I left both in place because they were already public API.
But I think the “NonObjC” entry point should be removed and documented
wherever we document API changes. Maybe CHANGLELOG.md.

Side note: internally we have a Builtin.isUnique_native that force casts
Any object to Builtin.NativeObject in order to bypass the ObjC check for
unknown types. It’s only useful for ObjC bridged types and we do not expose
this through a public API.

Okay, great!

I can have a stab at this next week, when I’m back at my regular computer.
Unless someone else feels like doing it before that, which is fine too =).

--
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>*/


(Johan K. Jensen) #10

I thought this *was* the proposal on swift-evolution.
Or do you mean github.com/apple/swift-evolution?
Does this need a full proposal and review-cycle,
as well as a number and markdown-file on the above repository?

···

On Thu, Dec 10, 2015 at 9:12 PM, Dmitri Gribenko via swift-evolution < swift-evolution@swift.org> wrote:

On Thu, Dec 10, 2015 at 11:25 AM, Chris Eidhof via swift-evolution < > swift-evolution@swift.org> wrote:

On 10 Dec 2015, at 12:38, Andrew Trick <atrick@apple.com> wrote:

On Dec 10, 2015, at 8:59 AM, Joe Groff <jgroff@apple.com> wrote:

On Dec 10, 2015, at 8:56 AM, Joe Groff via swift-evolution < >> swift-evolution@swift.org> wrote:

On Dec 10, 2015, at 8:50 AM, Chris Eidhof via swift-evolution < >> swift-evolution@swift.org> wrote:

There are two functions isUniquelyReferencedNonObjC and
isUniquelyReferenced, which do exactly the same thing. One has a more
constrained type, only accepting Swift objects. The other one accepts ObjC
objects as well, but always returns false. Regardless of whether it should
(could?) work for ObjC objects, I think this duplication is confusing (it
has confused me for a long time, and I’m happy that I can now see they’re
implemented in exactly the same way).

I think probably accepting just Swift objects would be the right thing to
do, as the function is useless for ObjC objects.

+1, I think this is just legacy we haven't gotten around to cleaning up.

cc'ing Andy, who can confirm whether the distinction is still necessary.

Yes! The most efficient runtime call is already inferred from the object
type. So there is no efficiency advantage in using one entry point over the
other. I think I left both in place because they were already public API.
But I think the “NonObjC” entry point should be removed and documented
wherever we document API changes. Maybe CHANGLELOG.md.

Side note: internally we have a Builtin.isUnique_native that force casts
Any object to Builtin.NativeObject in order to bypass the ObjC check for
unknown types. It’s only useful for ObjC bridged types and we do not expose
this through a public API.

Okay, great!

I can have a stab at this next week, when I’m back at my regular
computer. Unless someone else feels like doing it before that, which is
fine too =).

Just wanted to point out that like every API change, this needs to be
proposed on swift-evolution.

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


(Dmitri Gribenko) #11

Yes, the markdown file and review period.

Dmitri

···

On Thu, Dec 10, 2015 at 12:30 PM, Johan Jensen <jj@johanjensen.dk> wrote:

I thought this *was* the proposal on swift-evolution.
Or do you mean github.com/apple/swift-evolution?
Does this need a full proposal and review-cycle,
as well as a number and markdown-file on the above repository?

--
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>*/