[Mesa-dev] [PATCH] glsl: Remove unused ast copy constructors.

[Ilia Mirkin]

On Fri, Nov 21, 2014 at 1:47 PM, Matt Turner <mattst88 at gmail.com> wrote:
> These were added in commits a760c738 and 43757135 to be used in
> implementing C-style aggregate initializers (commit 1b0d6aef). Paul
> rewrote that code in commit 0da1a2cc to use GLSL types, rather than
> AST types, leaving these copy constructors unused.
>
> Tested by making them private and providing no definition.
> ---
>  src/glsl/ast.h | 29 -----------------------------
>  1 file changed, 29 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 15bf086..6995ae8 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -640,19 +640,6 @@ class ast_declarator_list;
>
>  class ast_struct_specifier : public ast_node {
>  public:
> -   /**
> -    * \brief Make a shallow copy of an ast_struct_specifier.
> -    *
> -    * Use only if the objects are allocated from the same context and will not
> -    * be modified. Zeros the inherited ast_node's fields.
> -    */
> -   ast_struct_specifier(const ast_struct_specifier& that):
> -      ast_node(), name(that.name), declarations(that.declarations),
> -      is_declaration(that.is_declaration)
> -   {
> -      /* empty */
> -   }
> -

Not sure what the style in mesa is, but IME, C++ copy constructors can
spring up on you at fairly unexpected times, esp implicit ones. It
will also happily generate a default one for you, which is almost
never what you want. The style I've usually seen is to create private
copy constructors without definition to catch such situations.

>     ast_struct_specifier(const char *identifier,
>                         ast_declarator_list *declarator_list);
>     virtual void print(void) const;
> @@ -670,22 +657,6 @@ 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,
> -                      ast_array_specifier *array_specifier)
> -      : ast_node(), type_name(that->type_name), structure(that->structure),
> -        array_specifier(array_specifier),
> -        default_precision(that->default_precision)
> -   {
> -      /* empty */
> -   }

This one, of course, can't be generated implicitly, so I don't see any
issue in just removing it.

> -
>     /** Construct a type specifier from a type name */
>     ast_type_specifier(const char *name)
>        : type_name(name), structure(NULL), array_specifier(NULL),
> --
> 2.0.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev