Ian Romanick <i...@freedesktop.org> writes: > On 11/22/2013 12:09 AM, Petri Latvala wrote: >[...] >> @@ -546,6 +548,12 @@ private: >> * If true, then register allocation should fail instead of spilling. >> */ >> const bool no_spills; >> + >> + /* >> + * Make noncopyable. >> + */ >> + vec4_visitor(const vec4_visitor&); >> + vec4_visitor& operator=(const vec4_visitor&); > > I think this should be a separate patch with it's own justification. > I'm also not sure it's strictly necessary. I'm not a C++ expert (and > most people on the project aren't either), so bear with me a bit. The > usual reason to make a class non-copyable is because the default > copy-constructor will only make a shallow copy of pointer fields. This > can lead to either double-frees or dereferencing freed memory. However, > since we're using ralloc, and the first construction of a vec4_visitor > object will out-live any possible copies, neither of these situations > can occur. Is that a fair assessment? > > Now... we probably should do this anyway... and there are a lot of > classes in the i965 back-end where this should occur. I don't know if > we want to make a macro to generate this boiler-plate code or if we just > want to manually add it to every class. Perhaps Ken, Paul, or Curro > will have an opinion... >
This hunk looks perfectly fine to me (well, aside from the &-placement).
All singleton classes should have its copy-constructor declared private
to avoid the situations you have described, and adding a member variable
that needs special handling on copy construction seems like the right
time to do so. I'm OK with using the private copy-constructor and
assignment operator idiom explicitly as in Petri's code or with having a
macro for the same purpose to save typing -- A third possibility that I
find slightly more elegant would be to define a noncopyable class all
singleton objects could inherit from, like:
| class noncopyable {
| private:
| noncopyable(noncopyable &);
| noncopyable &operator=(const noncopyable &);
| };
Because macros that declare class members and need to use access
specifiers can have unexpected side-effects.
Thanks.
pgpU793roSig1.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
