diff options
author | Jonathan Thornburg <jthorn@aei.mpg.de> | 2005-09-21 14:50:00 +0000 |
---|---|---|
committer | Jonathan Thornburg <jthorn@aei.mpg.de> | 2005-09-21 14:50:00 +0000 |
commit | 5b94bf377e8003537459425d2b232ddd3c369634 (patch) | |
tree | 1ac3643babbc3eca25a3e8c19bd3ea82965f649a | |
parent | 764823ab869c95d20204d9605497d63be579e570 (diff) |
CarpetLib: permute order of member initializers in comm_state::procbufdesc() constructor to match declaration order
Long comment for this patch
===========================
C++ guarantees that an object's members will be destroyed in the reverse
order from the order in which they were constructed. (This is A Good
Thing; if nothing else, it's essential for exception-safety, since it's
possible for an exception to be thrown part-way through the construction
of an object.)
Since there might be multiple constructors, and it would be expensive
(slow!) to keep a run-time record of the actual order in which various
constructors constructed things, C++ fixes a standard order, namely
the order in which the members are declared, and always uses that.
(Hence C++ destructors, including ones automagically generated by
the compiler, always destroy members in reverse-declaration order.)
That is, if you declare a class (or struct)
class foo
{
int A, B, C;
foo();
};
then the 3 members will ALWAYS be initialized in the order A,B,C,
and destroyed in the order C,B,A, whether you write the constructor
like this
foo::foo: A(69), B(105), C(42) { } // version 1
or like this
foo::foo: B(105), C(42), A(69) { } // version 2
So far all is well. But look what happens if you use one member in
the calculation of another member's initializer, eg
foo::foo: A(69), B(A+36), C(42) { } // version 3
This is still ok... but if we now use a different order for the
member initializers,
foo::foo: B(105), C(42), A(B-36) { } // version 4
then disaster strikes: As explained above, even though they were
written in the order B,C,A, these initializers will actuallly be
executed in the order the members were delcared, i.e. A,B,C. This
means that the A initializer will use the not-yet-initialized
(and hence garbage) value of B , and hence will initialize A
to a garbage value.
Alternatively, starting with the version 3 constructor, if someone
now permutes the declarations of A, B, and C such that B is declared
before A, then the same disaster will strike again (B will be
"initialized" to garbage).
There are two standard rules of C++ style which exist precisely
to avoid this problem:
1. ALWAYS write member initializers in the same order as the members
are declared. Many C++ compilers will in fact give a warning if you
violate this rule.
2. Prefer to not use the value of any member in the calculation of
the initializer for any other member. If you need to violate
this rule, there should be a comment on both the declaration
of the members, and on the initializers, that the code relies
on a particular ordering.
For more details see, for example, item 13 in Meyeres "Effective C++".
Ok, given all this explanation... the purpose of *this* patch is to
fix the comm_state::procbufdesc() constructor to comply with rule #1
(in this case the code is still correct even when violating that rule,
but it's cleaner style to always comply.)
Previously g++ had warned of the violation:
In file included from /home/jonathan/cactus/Cactus/arrangements/Carpet/CarpetLib/src/gdata.hh:13,
from /home/jonathan/cactus/Cactus/arrangements/Carpet/CarpetLib/src/data.hh:14,
from /home/jonathan/cactus/Cactus/configs/gzmultipatch-tofu-mp/bindings/include/data.hh:4,
from /home/jonathan/cactus/Cactus/arrangements/Carpet/Carpet/src/variables.hh:21,
from /home/jonathan/cactus/Cactus/arrangements/Carpet/Carpet/src/carpet_public.hh:11,
from /home/jonathan/cactus/Cactus/configs/gzmultipatch-tofu-mp/bindings/include/carpet.hh:4,
from /home/jonathan/cactus/Cactus/configs/gzmultipatch-tofu-mp/build/GZPatchSystem/patch/angular_interpatch_ghost_zone.cc:45:
/home/jonathan/cactus/Cactus/arrangements/Carpet/CarpetLib/src/commstate.hh: In
constructor `comm_state::procbufdesc::procbufdesc()':
/home/jonathan/cactus/Cactus/arrangements/Carpet/CarpetLib/src/commstate.hh:93: warning: `
comm_state::procbufdesc::recvbuf' will be initialized after
/home/jonathan/cactus/Cactus/arrangements/Carpet/CarpetLib/src/commstate.hh:83: warning:
`char*comm_state::procbufdesc::sendbufbase'
darcs-hash:20050921145044-b0a3f-b59f991ea43729fd06fca6a5ea5d7397cefebbc3.gz
-rw-r--r-- | Carpet/CarpetLib/src/commstate.hh | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/Carpet/CarpetLib/src/commstate.hh b/Carpet/CarpetLib/src/commstate.hh index e64fa8781..231e9ccba 100644 --- a/Carpet/CarpetLib/src/commstate.hh +++ b/Carpet/CarpetLib/src/commstate.hh @@ -93,9 +93,9 @@ public: char* recvbuf; // constructor for an instance of this structure - procbufdesc() : sendbufsize(0), recvbufsize(0), - sendbuf(NULL), recvbuf(NULL), - sendbufbase(NULL), recvbufbase(NULL) {} + procbufdesc() : sendbufbase(NULL), recvbufbase(NULL), + sendbufsize(0), recvbufsize(0), + sendbuf(NULL), recvbuf(NULL) { } }; // structure describing a collective communications buffer for a C datatype |