[Mesa-dev] [PATCH] glsl: Remove unused ast copy constructors.
Ilia Mirkin
imirkin at alum.mit.edu
Fri Nov 21 10:52:40 PST 2014
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
More information about the mesa-dev
mailing list