[swift-evolution] RFC: Proposed rewrite of Unmanaged<T>
Félix Cloutier
felixcca at yahoo.ca
Sat Dec 19 23:56:34 CST 2015
There's still the "release" issue (std::unique_ptr::release versus -[NSObject release]), but "transfer" seems like a good word to me. What about "transferByRetaining" and "transferWithoutRetaining"?
Félix
> Le 20 déc. 2015 à 00:01:22, Nevin Brackett-Rozinsky via swift-evolution <swift-evolution at swift.org> a écrit :
>
> Floating an idea here—not sure if it’s even in the right ballpark, and I’m certainly not tied to the specific wording, but what about something along the lines of:
>
> .transferByReleasing()
> .transferWithoutReleasing() // or perhaps just .transfer()
>
> Or the slightly-more-verbose:
>
> .transferObjectByReleasingReference()
> .transferObjectWithoutReleasingReference() // or .transferObject()
>
> Nevin
>
>
> On Sat, Dec 19, 2015 at 11:37 PM, Jacob Bandes-Storch via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
> Hi Dave,
> Thanks for sharing the proposal. I finally had a chance to catch up with the discussion.
>
> Generally I like the proposal and it would serve my needs well.
>
> I share others' concern that completely manual retain/release calls should still be allowed in some way. This doesn't have to be via UnsafeReference, but I don't think it makes sense to replace Unmanaged with UnsafeReference as proposed without some solution to this.
>
> Regarding naming:
>
> > problems with Unmanaged that we wanted to fix:
> > It was poorly-named (the reference is managed by somebody, we just aren't representing that management in the type system).
>
> I don't really agree with this. The management isn't "not represented" — the Unmanaged type explicitly says, at least to me, "this particular reference to the object does not manage its lifetime". So I don't think UnsafeReference is particularly an improvement here, though I'm not against it.
>
> - The release() name makes sense to me; it behaves like removeFirst/removeAtIndex. I'd also suggest something like consumeReference(). And for manual retains, if that's in scope for this API, you could use addReference().
>
> Regarding COpaquePointer:
>
> - Just to clarify, it looks like the proposed API would produce usage patterns like:
>
> someCFunction(context: COpaquePointer(UnsafeReference(retaining: self)))
>
> func myCallback(context: COpaquePointer) {
> let object = UnsafeReference<Foo>(bitPattern: context).object
> }
>
> func myCleanup(context: COpaquePointer) {
> UnsafeReference<Foo>(bitPattern: context).release()
> }
>
> - I'm curious why you chose a COpaquePointer initializer, rather than toOpaque() as the current Unmanaged API provides? On my UnsafePointer+Unmanaged proposal, you commented <https://github.com/apple/swift-evolution/pull/44#issuecomment-165902471> "we can’t give just UnsafePointer<Void> an init taking an UnsafeReference". But (a) it sounds like this is an eventual goal (I started working on it but it's a bit over my head currently); (b) using toOpaque() instead solves this problem:
>
> func toOpaque() -> UnsafePointer<Void> {
> return unsafeBitCast(_storage, UnsafePointer<Void>.self)
> }
> ...
> someCFunction(context: UnsafeReference(retaining: self).toOpaque())
>
> My motivation here is mostly that C void* APIs are currently bridged as UnsafePointer<Void>, so APIs that produce COpaquePointer are an obstacle to using them. If the trend is away from UnsafePointer and toward better "Opaque" semantics, that's fine with me; I'd just like the API to be easy to use in the meantime.
>
> In the same comment you also said "pointers to Void and incomplete types are not in any sense “unsafe” (once you restrict the interface as appropriate for incomplete types), and so maybe we want OpaquePointer<T> for these". It seems to me, though, that anything which can operate on arbitrary memory addresses is indeed Unsafe.
>
> Regarding documentation comments:
>
> - It might be worth mentioning that init(retaining:) and init(withoutRetaining:) are most likely to be used before conversion to an opaque pointer, rather than immediately calling .object or .release() which wouldn't be a very useful pattern.
>
> - A weakness I see is that CF/other C APIs won't have comments saying which "state" the returned UnsafeReference is in. The UnsafeReference doc comments are very clear about which operations may be used when, but the user is forced to mentally translate "…responsible for releasing…" into "this reference is in the *retained* state" — a possible source of confusion, unless all the CF doc comments can be reworded when they're imported into Swift to be more explicit.
>
> - Very minor: your doc comment on "public var object" has a stray/double `. Similarly, the top-level comment has a stray/double ".
>
> Jacob Bandes-Storch
>
> On Sat, Dec 19, 2015 at 7:43 PM, Dave Abrahams via swift-evolution <swift-evolution at swift.org <mailto:swift-evolution at swift.org>> wrote:
>
> > On Dec 19, 2015, at 4:22 PM, Brent Royal-Gordon <brent at architechies.com <mailto:brent at architechies.com>> wrote:
> >
> >>> Mainly, because simply saying "release" or "released" is a bit ambiguous to me.Are you saying it *has been* released, or are you saying it *needs to be* released?
> >>
> >> But nobody proposed "released" as a method name. In what way is "release" ambiguous? It's an imperative verb.
> >
> > I guess you're right that "release" is unambiguous, but as you mentioned, it's also strange to release a value and then use it.
>
> Yes. I think there are no really great choices here (at least not so far) so the question is whether that strangeness is enough of a problem to outweigh the points release() has in its favor. What do you think?
>
> > I think what I'm trying to get at here is that I prefer to think of the operations on Unmanaged as "explain to ARC how it should handle this object", rather than "do some manual operations so that ARC will do the right thing". Maybe the current Unmanaged design has shown the limitations of that approach, though.
>
> Not at all; the Unmanaged design—at least in my best understanding of its intent—is firmly in the imperative/manual operations camp. I wanted to do something more declarative, but the "I want to manage the reference that I claim was passed to me at +1" operation is side-effectful. Are we really comfortable with hiding that fact?
>
> >> But you applied "take" to both of them? One of them is idempotent while the other is not.
> >
> > The preferred way to use Unmanaged is that you immediately convert it to a managed reference without ever storing it or using it in any other way. That means you should immediately call either the retain-and-return operation or the don't-retain-and-return operation. Both of these should only ever be called once. You may instead choose to keep the reference Unmanaged and manually retain, release, and access it, but best practices discourage that.
>
> As I said in my original post, I'm ambivalent about the importance of highlighting the distinctions of safety and idempotence between these methods, but even if they're named similarly I don't see any merit in starting with "take." One thing I really dislike about is that the receiver, the UnsafeReference, isn't "taking" anything. The *caller* might be said to be taking something from the UnsafeReference, but only in the "returned at +1" case.
>
> How I see it: along with the UnsafeReference the called CF function either notionally
> a) gives (possibly-shared) ownership of the object directly to the caller, or
> b) gives the caller a token that allows him to get (shared) ownership of the object
>
> In case a), the caller needs to ask the UnsafeReference to transfer (or "release") that ownership into a strong reference, and. In case b), the caller needs to explicitly get (shared) ownership.
>
> If this description doesn't sound right to you, please try to correct it; that may help me understand your perspective better.
>
> > Now, one of the preferred, do-only-once operations *happens* to be safe to apply more than once, but I view that as an implementation detail. Both of them *happen* to be implemented in the same way as manual operations (`manuallyRelease()` and `object`), but I view that as an implementation detail, too.
>
> Hm, well, I don't view `object` as a "manual operation" and there's value in having a smaller API surface area. I don't think I want a separate `manuallyRelease` method if there is another method that has the same semantics. One of the greatest weaknesses of the current Unmanaged is that its interface is too broad and hard to grasp.
>
> > Honestly, I might be happier splitting an UnsafeReference type out of Unmanaged and putting the manual retain/release stuff into that:
>
> As noted in my original post, I really don't want to keep the name "Unmanaged" for anything. If anything, it's "ManuallyManaged." And I am very wary of API surface area creep here, whether it's in one type or two.
>
> > // Unmanaged is a high-level type for moving object references in and out of ARC's control.
> > struct Unmanaged<T: class> {
> > func created() -> T
> > func gotten() -> T
> >
> > // Also would have stuff for passing, which I haven't even thought about yet
> > }
> >
> > // UnsafeReference is a low-level type for manually managing the retain count of an object.
> > struct UnsafeReference<T: class> {
> > init(_ object: T)
> > init(_ unmanaged: Unmanaged<T>)
> >
> > var object: T
> >
> > // Some or all of these might return T
> > func retain()
> > func release()
> > func autorelease()
> > }
> >
> > This puts the discouraged manual operations off in their own type where they'll be available to those who know about them, but not sitting right there on every unaudited call.
> >
> >>> (I kind of want to suggest that retrieving an object through these calls should destroy the reference so it can't be used again, but I don't think that fits with Swift's mutation model without turning `Unmanaged`/`UnsafeReference` into a reference type and adding lots of overhead.)
> >>
> >> Yes, there's no way to reconcile that with the safety offered by the recommended usage patterns, since you can't mutate an rvalue.
> >
> > I thought so. That's too bad. (I wonder if the compiler can emit warnings instead, though.)
>
> I don't know what you have in mind here.
>
> >
> >>> (One possibility would be to have a single call with an enum parameter, like `bridge(.Create)` and `bridge(.Get)`. This would let you use the regular form of the verb.)
> >>
> >> There's no "bridging" going on here, though. This is simply "turn this unsafe thing into a safe thing in one of two ways"
> >
> > The "bridge" here comes from the Objective-C bridging casts, but I think there it's meant to refer to toll-free bridging, which is not what's happening in Swift.
> >
> > If the type name remains `Unmanaged`, then perhaps `manage(_:)` would be better? (I don't like `managing` here because that again implies it's side-effect-free and safe to call more than once.)
>
> Well again, we're not asking the receiver, the UnsafeReference, to manage anything. And don't forget, we have two operations and need two names, especially if you want them to feel similar.
>
> >> So far, my personal assessment of this direction is that it's no better than what I proposed, and has several weaknesses I'd like to avoid. In fact, it seems very similar to and roughly as understandable as the current Unmanaged design. I recognize that this is a highly subjective judgement, so if others disagree with me, I'd really like to hear about it. This is a tough design space and ultimately, what resonates best with the community is likely to be the best choice.
> >
> > I understand. I'm obviously struggling with this too, as you can see from how much I'm changing my design based on your replies, rather than defending the design as suggested before.
> >
> > Ultimately, Unmanaged is an API for handling an abstraction failure. That's inherently going to be tricky and subjective.
>
> Yup.
>
> -Dave
>
>
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
>
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org <mailto:swift-evolution at swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution <https://lists.swift.org/mailman/listinfo/swift-evolution>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20151220/850ee2b1/attachment.html>
More information about the swift-evolution
mailing list