Simon, thank you for bringing this up!
The original intent of
OwningNonNull was to implement a class with the same auto-conversion and annotation semantics as
NonNull<T> (i.e. never null once you have properly initialized it, auto-converts to
T&), but that holds a strong reference to the object involved. This was designed for use in DOM bindings and in particular for storing what WebIDL represents as
InterfaceName (as opposed to
InterfaceName?) in various containers (dictionaries, sequences). We could have just used
RefPtr for this use case, just like we could have used
T* instead of
NonNull<T>, but it seemed desirable to explicitly annotate the non-null nature of the things involved to eliminate pointless null-checks, which otherwise tend to proliferate.
This wasn't meant to be a generic container type for general use, really, though I see it got moved out of bindings code to XPCOM at some point...
When would I use OwningNonNull instead of RefPtr?
I described the bindings use cases above. Other consumers... it's really up to them. It adds a certain amount of noise to the code, obviously, but does make it clear when things are or are not null, if used properly.
Despite the name OwningNonNull, it can be nullptr even in Debug builds, either when using default-construction, or when calling forget.
Sticking to the original design space here, bindings never allow a default-constructed uninitialized OwningNonNull to escape. So if you are implementing a binding method, the default-constructed case is a non-issue. If you are doing initialization of things manually (i.e. acting as the caller, not implementor, of a binding method), you obviously need to make sure you properly initialize it, just like
NonNull and various other things that binding implementors expect to be set up properly.
forget, that is a really good catch. the version returning
already_AddRefed<T> is meant to be called from CC unlink only. The version with a
T** outparam.... should not exist. I have no idea why that was added and why one would use it. I filed bug 1605130 on making this saner.
You're right that in theory one could dereference a null
OwningNonNull after it's been unlinked. The same is true of any other container that unlinking puts into an invalid state; generally one should not be doing things after unlink...
Note that unlike a
OwningNonNull that is used in any way when null will assert in debug builds, at least...
Please explain under what circumstances this is still beneficial over a plain
Please let me know whether the above covers that. It's only beneficial if you're in a situation where you can 100% guarantee that the
OwningNonNull gets set up properly. Bindings can do that, because there's basically one single callsite that does the setup, and we just have to make sure that one callsite is implemented properly...
While it has a forget method, it can't be move-constructed or move-assigned
That's because the
forget method should not be used, per above, with the required exception of CC unlink. If C++ had Rust-like move semantics, where a moved-out-of object can't be accessed, we could add move stuff here, but given the actual C++ move semantics, we don't want to create a way to create null
It also can't be move-constructed or -assigned from a RefPtr<U>&&.
We hadn't needed a way to do that in bindings code, and since this was not meant as a generic container class but rather to address specific binding-related bits, we didn't add functionality that we didn't need. There's nothing wrong in principle with adding this.
I hope that helps, and I'm happy to write some documentation for this class, or review proposed documentation. Please just let me know.