aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJonathan Thornburg <jthorn@aei.mpg.de>2005-09-21 14:50:00 +0000
committerJonathan Thornburg <jthorn@aei.mpg.de>2005-09-21 14:50:00 +0000
commit5b94bf377e8003537459425d2b232ddd3c369634 (patch)
tree1ac3643babbc3eca25a3e8c19bd3ea82965f649a
parent764823ab869c95d20204d9605497d63be579e570 (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.hh6
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