> On Sep 5, 2016, at 3:18 PM, Ryosuke Niwa <rn...@webkit.org> wrote: > > On Mon, Sep 5, 2016 at 2:48 PM, Ryosuke Niwa <rn...@webkit.org > <mailto:rn...@webkit.org>> wrote: >> On Mon, Sep 5, 2016 at 2:47 PM, Filip Pizlo <fpi...@apple.com> wrote: >>> >>> On Sep 5, 2016, at 2:35 PM, Ryosuke Niwa <rn...@webkit.org> wrote: >>> >>> On Mon, Sep 5, 2016 at 10:13 AM, Darin Adler <da...@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.
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
