On Wed, Jul 3, 2013 at 2:52 PM, Chad Versace
<chad.vers...@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.vers...@linux.intel.com>
Yes, sorry, meant to change the commit subject.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
