[Mesa-dev] [PATCH 1/2] i965: Allocate vec4_visitor's uniform_size and uniform_vector_size arrays dynamically.

[Kenneth Graunke]

On 11/22/2013 12:23 PM, Francisco Jerez wrote:
> Kenneth Graunke <kenneth at whitecape.org> writes:
> 
>> On 11/22/2013 11:27 AM, Ian Romanick wrote:
>>> 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&);
>> [...]
>> Or you could just be sensible and just not make copies of things where
>> it makes no sense to do so...
>>
>> I would drop this hunk...it's unrelated and unnecessary.
>>
> 
> It makes it less likely to make mistakes, doesn't it?  E.g. in cases
> where you pass an argument by value where you really meant to pass it by
> reference.  I think our code would benefit from doing this consistently,
> not only because it tells the compiler which classes it makes sense to
> copy and which ones don't, but also because it makes the semantics of
> the class clearer to developers.

Ah, I see...the point is that you might accidentally do:

void foo(vec4_visitor v, int bar);

and end up copying the world, when you meant to pass by reference.  I
guess that's useful.  I probably wouldn't NAK that.

I was thinking you were trying to prevent things like:

vec4_visitor v2(v);
vec4_visitor v3 = v;

which suggests a fundamental misunderstanding of the code which a
compiler error is unlikely to help you overcome.

At any rate, it's really a separate change, so it should be a separate
patch (as Ian mentioned).  Each patch should ideally only make one
specific change.  Smaller patches are almost always better.

--Ken