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

[Ilia Mirkin]

On Fri, Nov 21, 2014 at 1:56 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Fri, Nov 21, 2014 at 10:52 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> 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.
>
> That's true, but they were added for usage in one place and that one
> place has been removed, and we've never needed it anywhere else.

Exactly. That's why I'm suggesting making a private version without
definition and leaving it in. That way accidental uses won't be added,
which tends to be easy to do due to implicit copy constructors.

  -ilia