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
