[Mesa-dev] [PATCH 08/14] glsl: Add copy-constructor for ast_struct_specifier.

[Chad Versace]

On 06/29/2013 07:43 PM, Matt Turner wrote:
> ---
>   src/glsl/ast.h | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 13c5e6b..0b70bb7 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -453,6 +453,13 @@ class ast_declarator_list;
>
>   class ast_struct_specifier : public ast_node {
>   public:
> +   ast_struct_specifier(const ast_struct_specifier *structure):
> +      is_declaration(structure->is_declaration), name(structure->name),
> +      declarations(structure->declarations)
> +   {
> +      /* empty */
> +   }
> +
>      ast_struct_specifier(const char *identifier,
>   			ast_declarator_list *declarator_list);
>      virtual void print(void) const;

We discussed in person, so this is for the list.

The copy constructors in patches 7 and 8 make a deep-copy the class's
value members up the inheritance hierarchy. This causes the unexpected
side effect that the copy of ast_struct_specifier
will inhabit the same position in the exec_list as the original ast_struct_specifier.

Also, please use 'that' as the param name. It makes the code extra easy read,
especially when writing code like this:
    this->foo = that->foo;

I'm done today for reviews because I need to move on to hw bring-up. But
I'll try to continue reviewing the series tomorrow morning.