I'm sympathetic to the desire to have a single fallible construction
function (this is generally how I structure things in C code), but I'm not
sure that this is really the right design for it. The general problem that
it doesn't alleviate is that failure to check the return value leaves you
with a reference/pointer to an object in an ill-defined half-constructed
state. At least for heap allocations, I'd much rather have the property
that failures leave you with a null pointer.
So, if we are going to do something along these lines, I would want it to
be a convention that if you use MakeUnique and the like (as you should)
then they automatically validate correct construction and if not return an
empty pointer.
-Ekr
On Thu, Apr 21, 2016 at 3:07 AM, Nicholas Nethercote <n.netherc...@gmail.com
> wrote:
> Hi,
>
> C++ constructors can't be made fallible without using exceptions. As a
> result,
> for many classes we have a constructor and a fallible Init() method which
> must
> be called immediately after construction.
>
> Except... there is one way to make constructors fallible: use an |nsresult&
> aRv| outparam to communicate possible failure. I propose that we start
> doing
> this.
>
> Here's an example showing stack allocation and heap allocation. Currently,
> we
> do this (boolean return type):
>
> T ts();
> if (!ts.Init()) {
> return NS_ERROR_FAILURE;
> }
> T* th = new T();
> if (!th.Init()) {
> delete th;
> return NS_ERROR_FAILURE;
> }
>
> or this (nsresult return type):
>
> T ts();
> nsresult rv = ts.Init();
> if (NS_FAILED(rv)) {
> return rv;
> }
> T* th = new T();
> rv = th.Init();
> if (NS_FAILED(rv)) {
> delete th;
> return rv;
> }
>
> (In all the examples you could use a smart pointer to avoid the explicit
> |delete|. This doesn't affect my argument in any way.)
>
> Instead, we would do this:
>
> nsresult rv;
> T ts(rv);
> if (NS_FAILED(rv)) {
> return rv;
> }
> T* th = new T(rv);
> if (NS_FAILED(rv)) {
> delete th;
> return rv;
> }
>
> For constructors with additional argument, I propose that the |nsresult&|
> argument go last.
>
> Using a bool outparam would be possible some of the time, but I suggest
> always
> using nsresult for consistency, esp. given that using bool here would be no
> more concise.
>
> SpiderMonkey is different because (a) its |operator new| is fallible and
> (b) it
> doesn't use nsresult. So for heap-allocated objects we *would* use bool,
> going
> from this:
>
> T* th = new T();
> if (!th) {
> return false;
> }
> if (!th.Init()) {
> delete th;
> return false;
> }
>
> to this:
>
> bool ok;
> T* th = new T(ok);
> if (!th || !ok) {
> delete th;
> return false;
> }
>
> These examples don't show inheritance, but this proposal works out
> straightforwardly in that case.
>
> The advantages of this proposal are as follows.
>
> - Construction is atomic. It's not artificially split into two, and
> there's no
> creation of half-initialized objects. This tends to make the code nicer
> overall.
>
> - Constructors are special because they have initializer lists -- there are
> things you can do in initializer lists that you cannot do in normal
> functions. In particular, using an Init() function prevents you from
> using
> references and |const| for some members. This is bad because references
> and
> |const| are good things that can make code more reliable.
>
> - There are fewer things to forget at call sites. With our current
> approach you
> can forget (a) to call Init(), and (b) to check the result of
> Init(). With this
> proposal you can only forget to check |rv|.
>
> The only disadvantage I can see is that it looks a bit strange at first.
> But if
> we started using it that objection would quickly go away.
>
> I have some example patches that show what this code pattern looks like in
> practice. See bug 1265626 parts 1 and 4, and bug 1265965 part 1.
>
> Thoughts?
>
> Nick
> _______________________________________________
> 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
