I agree that MakeNotNull doesn't sound like it allocates. Reads to me as Make[Into]NotNull. Context will *usually* make this clear, but why not NewNotNull? (Honestly I don't like MakeUnique to begin with)
NotNull<T*>::New(args...) is another option. "My first goal was to avoid the unnecessary nullptr-check in the NotNull constructor, since our new is infallible by default." Even if we can't communicate to the optimizer that new is infallible and incur it to omit the branch in this inlined code, it would be extremely well predicted. MOZ_UNLIKELY() it and call it good, IMO. On Thu, Nov 23, 2017 at 12:58 AM, <bo...@mozilla.com> wrote: > I'm not sure what the benefits are (the MakeUnique comments only really seem > to give aesthetic ones), but they do make it a bit harder when searching > through code to see where things are constructed. > > I suppose if we always stick to using Make* then (with regex) it wouldn't add > too much burden for searching. > > On Thursday, 23 November 2017 07:50:27 UTC, gsqu...@mozilla.com wrote: >> Should we allow hiding 'new' statements, or keep them as visible as possible? >> >> >> Some context: >> Recently in bug 1410252 I added a MakeNotNull<T*>(args...) function that >> does `NotNull<T*>(new T(args...))`, in the style of MakeUnique and others. >> It also works with RefPtr<T>. >> >> My first goal was to avoid the unnecessary nullptr-check in the NotNull >> constructor, since our new is infallible by default. >> >> And I thought that hiding the naked new statement was a nice side benefit, >> as I was under the impression that it was A Good Thing in modern C++. >> (Though I think the main reason for that, was to prevent leaks when handling >> exceptions in multi-allocation expressions, so it doesn't apply to us here.) >> >> Now, a colleague remarked that this was making the memory allocation less >> obvious. >> "What about MakeUnique?" I asked; "I'm used to them now" he retorted. :-P >> >> >> So, what do you all think? >> - Should I remove MakeNotNull? >> - Or should we allow/encourage more MakeX functions instead of X(new...)? >> I'm thinking MakeRefPtr might be nice. > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
