[Mesa-dev] [PATCH 01/11] glsl: Initialize all member variables of _mesa_glsl_parse_state on construction.

[Ian Romanick]

On 09/20/2013 09:51 PM, Francisco Jerez wrote:
> The _mesa_glsl_parse_state object relies on the memory allocator
> zeroing out its contents before it's initialized, which is quite an
> unusual practice in the C++ world because it ties objects to some
> specific allocation scheme, and gives unpredictable results when an
> object is created with a different allocator -- Stack allocation,
> array allocation, or aggregation inside a different object are some of
> the useful possibilities that come to my mind.  Initialize all fields
> from the constructor and stop using the zeroing allocator.
> 
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>
> Reviewed-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>  src/glsl/glsl_parser_extras.cpp | 16 ++++++++++++++--
>  src/glsl/glsl_parser_extras.h   |  2 +-
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> 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()

Related to my comment on Vinson's patch (see "glsl: Initialize
assignment_generator member variables."), we need to come up with some
coding conventions.

 - What things should use the initialization list, and what things
should be ininitailized in the body of the constructor?

 - Should each initializer go on its own line, or should they be put on
fewer lines?

 - What should be the indentation before the ":"?

>  {
>     switch (target) {
>     case GL_VERTEX_SHADER:   this->target = vertex_shader; break;
> @@ -66,10 +66,14 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
>     this->scanner = NULL;
>     this->translation_unit.make_empty();
>     this->symbols = new(mem_ctx) glsl_symbol_table;
> +
> +   this->num_uniform_blocks = 0;
> +   this->uniform_block_array_size = 0;
> +   this->uniform_blocks = NULL;
> +
>     this->info_log = ralloc_strdup(mem_ctx, "");
>     this->error = false;
>     this->loop_nesting_ast = NULL;
> -   this->switch_state.switch_nesting_ast = NULL;
>  
>     this->struct_specifier_depth = 0;
>     this->num_builtins_to_link = 0;
> @@ -105,6 +109,13 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
>  
>     this->Const.MaxDrawBuffers = ctx->Const.MaxDrawBuffers;
>  
> +   this->current_function = NULL;
> +   this->toplevel_ir = NULL;
> +   this->found_return = false;
> +   this->all_invariant = false;
> +   this->user_structures = NULL;
> +   this->num_user_structures = 0;
> +
>     /* Populate the list of supported GLSL versions */
>     /* FINISHME: Once the OpenGL 3.0 'forward compatible' context or
>      * the OpenGL 3.2 Core context is supported, this logic will need
> @@ -163,6 +174,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
>  
>     this->gs_input_prim_type_specified = false;
>     this->gs_input_prim_type = GL_POINTS;
> +   this->gs_input_size = 0;
>     this->out_qualifier = new(this) ast_type_qualifier();
>  }
>  
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 364a983..853a9b0 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -73,7 +73,7 @@ struct _mesa_glsl_parse_state {
>     _mesa_glsl_parse_state(struct gl_context *_ctx, GLenum target,
>  			  void *mem_ctx);
>  
> -   DECLARE_RZALLOC_CXX_OPERATORS(_mesa_glsl_parse_state);
> +   DECLARE_RALLOC_CXX_OPERATORS(_mesa_glsl_parse_state);
>  
>     /**
>      * Generate a string representing the GLSL version currently being compiled
>