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> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
