On 09/18/2013 07:43 AM, Ian Romanick wrote: > On 09/15/2013 02:10 AM, Francisco Jerez wrote: >> The _mesa_glsl_parse_state object relies on the memory allocator >> zeroing out its contents before it's initialized, which seems rather >> risky. One of the following commits will homogenize implementations >> of the new operator in a way that would break this assumption leaving >> some of the member variables of this object uninitialized. > > This was an intentional design decision that we made, after a fair > amount of consideration, in the early days of the compiler. A number of > factors came together: > > * Use of ralloc et al. meant that we could never have stack-allocated > objects. > > * Use of rzalloc meant that objects would always be pre-allocated to zero. > > * Uninitialized class member bugs are hard to track down. > > * It's easy to forget to add 'this->foo = 0;' when you add a field foo > to a class. > > * A dozen lines of 'this->foo = 0;' is hard to properly review when a > new class is added. Are there actually 13 fields that need to be > initialized? > > * Writing a dozen lines of 'this->foo = 0;' is annoying. :) > > In the end, we decided to just rely on the allocator always providing > cleared memory. > > I'm not very excited about adding a bunch of redundant code to the these > constructors... especially since that code probably won't get maintained > (e.g., when the next member gets added to those classes). > > I'd like to hear Ken and Eric weigh in.
I do like simply using rzalloc or memset to avoid having to fill in dozens of fields with 0. It's very concise and convenient. However, an object's constructor is supposed to initialize its fields. This is idiomatic for *any* object oriented language. If we're going to use the OOP paradigm, we probably ought take this approach. In my mind, using memset() inside the constructor counts as the constructor initializing the fields. It also works with stack allocation, normal heap allocation, or ralloc allocation. Uninitialized class member bugs will be caught with Valgrind, and Piglit makes it very easy to check that: $ piglit-run.py --valgrind tests/quick.tests results/vg-1 With these tools, I don't think tracking down uninitialized fields is that difficult. I also don't think "we'll forget to initialize things" is a good argument for relying on zero-allocation. Sometimes fields need to be initialized to a value other than zero, so it's a good idea to get in the habit of initializing them. I would have to say I agree with Paul and Curro, but I can also respect the current rely-on-rzalloc style, and it doesn't honestly bother me much. If people want to explicitly initialize member variables, I applaud them; if not, I can live with that too. --Ken _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
