[swift-evolution] RFC: Proposed rewrite of Unmanaged<T>

Jacob Bandes-Storch jtbandes at gmail.com
Sat Dec 19 22:37:13 CST 2015


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> wrote:

>
> > On Dec 19, 2015, at 4:22 PM, Brent Royal-Gordon <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
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20151219/a2b7b376/attachment.html>


More information about the swift-evolution mailing list