[webkit-dev] Terminology for giving up ownership: take, release, move

[Maciej Stachowiak]

> On Sep 5, 2016, at 3:18 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> 
> On Mon, Sep 5, 2016 at 2:48 PM, Ryosuke Niwa <rniwa at webkit.org <mailto:rniwa at webkit.org>> wrote:
>> On Mon, Sep 5, 2016 at 2:47 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>>> 
>>> On Sep 5, 2016, at 2:35 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>>> 
>>> On Mon, Sep 5, 2016 at 10:13 AM, Darin Adler <darin at apple.com> wrote:
>>> 
>>> Hi folks.
>>> 
>>> WebKit has some critical functions that involve asking an object to give up
>>> ownership of something so the caller can take ownership.
>>> 
>>> In the C++ standard library itself, this is called move, as in std::move.
>>> 
>>> In WebKit smart pointers, we call this operation release, as in
>>> RefPtr::releaseNonNull and String::releaseImpl.
>>> 
>>> In WebKit collections, we call this operation take, as in HashMap::take and
>>> ExceptionOr::takeReturnValue.
>>> 
>>> The release vs. take terminology is distracting to my eyes. The verb “take"
>>> states what the caller wishes to do, and the verb “release” states what the
>>> caller wants the collection or smart pointer to do. My first thought was be
>>> to rename the take functions to use the word release instead, but I fear it
>>> might make them harder to understand instead of easier and clearly it would
>>> make them longer.
>>> 
>>> 
>>> I agree the verb "take" is not semantically sound here.  How about
>>> HashMap::receiveReleased / ExceptionOr::receiveReleased?  Or simply
>>> HashMap::released / ExceptionOr::takeReleased?  Even HashMap::receive
>>> / ExceptionOr::receiveReturnValue might work better because "receive"
>>> is more a passive form of accepting the ownership of something.
>>> 
>>> 
>>> I don't think that HashMap::receiveReleased() fits with
>>> Subject::verbPhrase().  In HashMap::take(), the HashMap is releasing
>>> ownership of a value.  So, it is releasing it.  It's definitely not
>>> receiving it.
>> 
>> Oh I see.  Sorry, I had assumed they were just taking Ref<>&& as an
>> argument.  In that case, release() definitely seems like the right
>> terminology to use.
> 
> Hm... now that I recall the semantics of HashMap::take, I've started
> to think that "release()" on its own may not be the right name for
> collections because these member functions remove an item from a
> collection while releasing the ownership.  Maybe "removeAndRelease()"
> or "removeToRelease()" would do.
> 
> Alternatively, we could make the regular "remove()" always return the
> removed value (or maybe this would result in more code bloat / runtime
> cost?)

Current HashMap::remove returns a boolean which indicates wether the item was found. It also avoids doing a move to a temporary. So both efficiency and compatibility may be relevant considerations. If you consider the option of renaming the basic remove as well as the one that returns a value, you could use erase() for the one that doesn't return the value and remove() for the one that does.





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20160905/9eb721e8/attachment.html>