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

[Kenneth Graunke]

On 09/24/2013 11:22 AM, Ian Romanick wrote:
> 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?

I don't have a strong preference here.  I like initializing fields from
parameters where both have basically the same name, i.e.

fs_visitor(brw_wm_prog_data *prog_data)
   : prog_data(prog_data), ...
{
}

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

It's a similar situation to function parameters - should you put them on
a few lines, or one per line?  There's usually a threshold where, for a
lot of parameters, one per-line makes sense.  I'd prefer not to
standardize this, and leave it up to individual developers' artistic taste.

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

I think my preference is:

constructor_name(...)
   : foo(...), bar(...),
     quux(...), baz(...),
{
}

(Colon goes on the line below the instruction, preceded by a standard
Mesa three space indent, followed by a single space.)

But I'm open to other ideas.

--Ken