Kenneth Graunke <kenn...@whitecape.org> writes: > On 09/20/2013 09:51 PM, Francisco Jerez wrote: >> diff --git a/src/glsl/glsl_parser_extras.cpp >> b/src/glsl/glsl_parser_extras.cpp >> index cac5a18..772933f 100644 >> --- a/src/glsl/glsl_parser_extras.cpp >> +++ b/src/glsl/glsl_parser_extras.cpp >> @@ -55,7 +55,7 @@ static unsigned known_desktop_glsl_versions[] = >> >> _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx, >> GLenum target, void *mem_ctx) >> - : ctx(_ctx) >> + : ctx(_ctx), switch_state() > > Maybe it's just me, but I don't particularly like using explicit calls > to undefined constructors to zero out data. > Strictly speaking it's not "undefined" it's an implicitly defined constructor, just like the implicitly defined destructor, copy constructor and assignment operators you get according to the C++ standard. I think this is a very convenient language feature, I'd rather stick to the convention of assuming that all default constructors do something reasonable and leave the class in a well-defined state (as implicitly defined constructors do for POD types), than avoiding calls to implicitly defined constructors.
> It's a bit of an inconsistency in C++: for most classes, the default > constructor does nothing. But for POD, the default constructor > zero-initializes the data. Unless you're familiar with this rule, it > looks like a no-op. > For all classes the default constructor calls the default constructors of all member variables and superclasses recursively, it's never a no-op unless you explicitly define it and leave all POD fields undefined. > Perhaps more importantly, the moment someone adds a method to one of > these classes, the default constructor's behavior changes from > zero-initializing to no-op. It's easy to forget that, and suddenly have > a bunch of uninitialized data. > A class having methods defined doesn't change the behavior of the implicitly defined constructor in any way AFAIK, I really don't see the problem. > I would prefer to see explicit constructors for POD types which > zero-initialize all of the fields. Then, you don't need to call the > constructor - it just happens automatically. If you add methods, it > keeps working. > I agree with doing this for the reason of not having to use parenthesis to call the constructor on classes with POD members, but again, adding methods doesn't have any influence on it, and it seems like something that would be better done as a separate follow-up patch series because it would involve a considerable amount of changes to do it consistently. > For tiny structures, like two or three elements, I'd also be okay with > the containing class's constructor initializing them. For example, > _mesa_glsl_parse_state() could just initialize switch_state->test_var > and so on. (In this case I think glsl_switch_state is too large for > this approach.) > IMHO having a reasonable default constructor for all classes (possibly the implicitly generated one) for all classes is usually a more concise code style and also more robust against changes... > Sorry for the nitpicking. Otherwise, I'm definitely in favor of this > change. > Thank you for your comments, I pretty much agree with everything else you said in this thread, I'll send a revised patch series taking into account your suggestions soon. >[...]
pgp7Ly4Vaqyd5.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
