On 07/03/2013 12:03 PM, Matt Turner wrote:
---
src/glsl/ast.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 3bb33c5..f00ef3a 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -453,6 +453,18 @@ 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)
+ {
+ /* empty */
+ }
+
ast_struct_specifier(const char *identifier,
ast_declarator_list *declarator_list);
virtual void print(void) const;
See patch 7's review. This isn't a copy constructor. The copy constructor's
signature is T::T(const T&) where T=ast_struct_specifier.
The compiler still generates the default copy constructor, so we get this scary
behavior:
ast_struct_specifier orig;
... // do stuff to orig
// This creates the copy you want.
ast_struct_specifier copy0(&orig);
assert(copy0.link.next == NULL);
// If someone accidentally writes this code, it compiles without warning,
// but creates a buggy copy.
ast_struct_specifier copy1(orig);
assert(copy0.link.next == orig.link.next);
There are two ways to safely fix this patch. A C++-way that uses
references, and a way that lets people code in the C-way using pointers.
Fix 1
-----
Change the signature to `ast_struct_specifier(const ast_struct_specifier&
that)`.
Then you will have defined the copy ctor, preventing the generation of the
default
copy constructor. A possible unwanted side-effect, from the perspective of a C
programmer, is that we must pass the original ast_struct_specifier as a ref
rather
than a pointer.
Fix 2
-----
Leave the constructor as-is in the patch. Fix the commit subject as discussed in
patch 7's review, since the patch now adds a new constructor, not a copy
constructor. Then, to prevent accidental use of the default copy constructor,
prevent it's generation by declaring it but not providing a definition.
It doesn't matter if you declare it as public or private, since it will
cause a compile-time error either way if anyone tries to use it,
but Google's and LLVM's practice is to declare it private.
Like this:
class ast_struct_specifier: public ast_node {
...
private:
/* Prevent use of copy constructor. */
ast_struct_specifier(const ast_struct_specifier&);
};
----
I prefer 1 because, if we're defining a ctor that looks and acts like the
copy ctor, then let's just define the copy ctor and be done with it.
However, I don't know how you're using this in future patches, so it's
your call.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
