[Mesa-dev] [PATCH 3/5] glsl: Create AST structs corresponding to new productions in grammar

[Dan McCabe]

On 06/28/2011 10:47 PM, Kenneth Graunke wrote:
> On 06/28/2011 02:48 PM, Dan McCabe wrote:
>> Previously we added productions for:
>> 	switch_body
>> 	case_label_list
>> 	case_statement
>> 	case_statement_list
>> Now add AST structs corresponding to those productions.
> Both 1/3 and 3/3 look good.  You might actually want to squash
> them...they're so similar.  I'd order them like this:
>
> [PATCH 1/4] glsl: Add productions to GLSL grammar for switch statements
> [PATCH 2/4] glsl: Create AST structures for switch statements
> [PATCH 3/4] glsl: Reference data structure ctors in grammar
> [PATCH 4/4] glsl: Generate IR for switch statements
My reasoning for ordering them the way I did is that 1/5 was the least 
amount of code the **COULD** be added. 2/5 included the discovery that 
the naive approach to the grammar doesn't work and that you needed 
something more complex. 3/5 then adds the support for the realistic 
variant of the grammar.

I tried to grow the complexity organically and order them with the 
intent that if you nothing about the code (as was the case for me when I 
started this work), you would gradually learn all that was needed to 
make it work in a logical progression.

But your suggested re-ordering makes sense if you already know the code 
base.

When 4/5 and 5/5 get reviewed, I will revisit the issue. It's not a big 
deal to me either way.

cheers, danm