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

Matt Turner mattst88 at gmail.com
Wed Jul 3 15:33:38 PDT 2013


On Wed, Jul 3, 2013 at 2:52 PM, Chad Versace
<chad.versace at linux.intel.com> wrote:
> 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>

Yes, sorry, meant to change the commit subject.


More information about the mesa-dev mailing list