Bug 153617

This would be awesome.

Created attachment 270876 [details] Proof of concept I took a stab at this over the weekend. Here's a proof of concept with a few examples of how this would be adopted. I ended up adding a static create function in WTF::Ref. It's not possible to do this correctly in RefCounted, since the RefCounted template is not always instantiated with the class that needs to be constructed. I like the way this ended up looking, since it plays really nicely with auto, e.g.: auto stopwatch = Ref<Stopwatch>::create(); There are a few things I don't love about this approach, though: 1. Classes that wish to use this function need to become friends with WTF::Ref so that it can call their private/protected constructors. Making constructors public would defeat the point of this exercise, so this is probably an acceptable trade-off. 2. Some classes have non-trivial create functions, where extra work needs to be done before or after calling constructors, and some classes even return a null RefPtr in some cases. Those classes should be ineligible for using Ref::create() (even though they might choose to use that function internally). I'd like to add some enforcement at compile time that prevents callers from using Ref::create if a per-class create function exists. It might make sense to add a trait to classes that wish to retain a custom create function that Ref::create can static_assert() on. I'd welcome any feedback on how to make this better!

Comment on attachment 270876 [details] Proof of concept View in context: https://bugs.webkit.org/attachment.cgi?id=270876&action=review > Source/JavaScriptCore/API/JSProfilerPrivate.cpp:39 > + auto stopwatch = Ref<Stopwatch>::create(); Does this mean that the caller could accidentally create a RefPtr<T> even if the custom T::create() would always return a Ref<T>?

Comment on attachment 270876 [details] Proof of concept View in context: https://bugs.webkit.org/attachment.cgi?id=270876&action=review > Source/JavaScriptCore/API/JSWeakObjectMapRefInternal.h:52 > + friend class Ref<OpaqueJSWeakObjectMap>; Depending on the current tastes among reviewers, this could be a macro a la MAKE_FAST_ALLOCATED or whatever it is. It would definitely make the pervasive friend'ing more obvious if, say, it said ALLOW_CREATE_FUNCTION_FOR(OpaqueJSWeakObjectMap). Even better if the class name could be derived automatically, so the macro would be ALLOW_CREATE_FUNCTION or something similar.

(In reply to comment #3) > Comment on attachment 270876 [details] > Proof of concept > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270876&action=review > > > Source/JavaScriptCore/API/JSProfilerPrivate.cpp:39 > > + auto stopwatch = Ref<Stopwatch>::create(); > > Does this mean that the caller could accidentally create a RefPtr<T> even if > the custom T::create() would always return a Ref<T>? Yeah, I alluded to this issue in comment #2. I agree that a custom T::create() and Ref::create() should be mutually exclusive. We should have a mechanism that prevents you at compile time opting into Ref::create() if you have a custom create() functions. This could be part of the macro you describe in your following comment.

... and the same macro would also prevent you from adding a custom create() function once you've opted in to Ref::create().