[Mesa-dev] [PATCH 07/14] glsl: Add copy-constructor for ast_type_specifier.

[Chad Versace]

On 07/03/2013 12:01 PM, Matt Turner wrote:
> ---
>   src/glsl/ast.h | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index b86f97b..3bb33c5 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -469,6 +469,22 @@ public:
>
>   class ast_type_specifier : public ast_node {
>   public:
> +   /**
> +    * \brief Make a shallow copy of an ast_type_specifier, specifying array
> +    *        fields.
> +    *
> +    * Use only if the objects are allocated from the same context and will not
> +    * be modified. Zeros the inherited ast_node's fields.
> +    */
> +   ast_type_specifier(const ast_type_specifier *that, bool is_array,
> +                      ast_expression *array_size)
> +      : ast_node(), type_name(that->type_name), structure(that->structure),
> +        is_array(is_array), array_size(array_size), precision(that->precision),
> +        is_precision_statement(that->is_precision_statement)
> +   {
> +      /* empty */
> +   }
> +
>      /** Construct a type specifier from a type name */
>      ast_type_specifier(const char *name)
>         : type_name(name), structure(NULL),
>


The constructor looks good to me.

However, the term 'copy constructor' has a specific meaning in C++, and this
constructor doesn't meet that. It's just a plain vanilla constructor. Please
adjust the commit subject to reflect that. I think it's fine to just say
"Add a new constructor for ast_type_specifier".

Each class has exactly one copy constructor, and its signature is `T::T(const T&)`.

I'm still uncomfortable that we're not deep-copying ast_type_specifier::structure,
but I'll trust you.

With the commit subject fixed,
Reviewed-by: Chad Versace <chad.versace at linux.intel.com>