[Piglit] [PATCH] glsl-1.20: Verify GLSL rules about when a new name comes into scope

[Ian Romanick]

On 03/02/2012 10:54 AM, Kenneth Graunke wrote:
> On 03/02/2012 10:35 AM, Ian Romanick wrote:
>> From: Ian Romanick<ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick<ian.d.romanick at intel.com>
>> Cc: Kenneth Graunke<kenneth at whitecape.org>
>> ---
>> This issue came up in some ARB discussions. The GLSL rules are
>> different from C, and this surprised a couple people. It doesn't
>> appear that we have any tests for this, so I decided to add one.
>
> Hrm, you're right...I don't see any tests for this either.
>
>> ...n-nested-scope-hides-parameter-frag.shader_test | 35
>> +++++++++++++++++++
>> ...n-nested-scope-hides-parameter-vert.shader_test | 36
>> ++++++++++++++++++++
>> 2 files changed, 71 insertions(+), 0 deletions(-)
>> create mode 100644
>> tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-frag.shader_test
>>
>> create mode 100644
>> tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-vert.shader_test
>>
>>
>> diff --git
>> a/tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-frag.shader_test
>> b/tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-frag.shader_test
>>
>> new file mode 100644
>> index 0000000..c954339
>> --- /dev/null
>> +++
>> b/tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-frag.shader_test
>>
>> @@ -0,0 +1,35 @@
>> +[require]
>> +GLSL>= 1.10
>> +
>> +[fragment shader]
>
> Erp. :( Your test prefix is "glsl-1.20" and your spec quotes are from
> the GLSL 1.20 spec...but your shaders are 1.10, which has different
> scoping rules! That sends up red flags.
>
> That said, the 1.10 spec doesn't seem to say -anything- on this, so
> presumably this was the intended behavior all along...
>
> Otherwise, I agree with these tests. I would be happy with one of two
> options:
> - Move them to spec/glsl-1.10, change subject line appropriately, and
> add a comment as to why you think they apply to GLSL 1.10 as well.
> - Add #version 120 and change config block to GLSL >= 1.20

Foo.  I originally had them in spec/glsl-1.10.  When I couldn't find the 
relevant rules in the 1.10 spec, I moved the tests to spec/glsl-1.20. 
I'll opt to change the tests to actually require (and use) 1.20.

> With one of those changes, these are:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
>> +float f(float p)
>> +{
>> + /* Page 27 of the GLSL 1.20 spec says:
>> + *
>> + * "Within a declaration, the scope of a name starts immediately
>> + * after the initializer if present or immediately after the name
>> + * being declared if not."
>> + *
>> + * This means the use of 'p' in the first initializer below should
>> + * have the value of the function parameter 'p' and the use of 'p' in
>> + * the second initializer shoudl have the value of the function
>
> ^^^ should.

Oops.

>> + * parameter 'p' + 0.5.
>> + */
>> + {
>> + float p = p + 0.5, q = p;
>> + return q;
>> + }
>> +}
>> +
>> +void main()
>> +{
>> + gl_FragColor = vec4(f(0.25));
>> +}
>> +
>> +[test]
>> +clear color 0.1 0.1 0.1 1.0
>> +clear
>> +
>> +draw rect -1 -1 2 2
>> +probe all rgb 0.75 0.75 0.75
>> diff --git
>> a/tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-vert.shader_test
>> b/tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-vert.shader_test
>>
>> new file mode 100644
>> index 0000000..ec438d3
>> --- /dev/null
>> +++
>> b/tests/spec/glsl-1.20/execution/function-nested-scope-hides-parameter-vert.shader_test
>>
>> @@ -0,0 +1,36 @@
>> +[require]
>> +GLSL>= 1.10
>> +
>> +[vertex shader]
>> +
>> +float f(float p)
>> +{
>> + /* Page 27 of the GLSL 1.20 spec says:
>> + *
>> + * "Within a declaration, the scope of a name starts immediately
>> + * after the initializer if present or immediately after the name
>> + * being declared if not."
>> + *
>> + * This means the use of 'p' in the first initializer below should
>> + * have the value of the function parameter 'p' and the use of 'p' in
>> + * the second initializer shoudl have the value of the function
>
> ^^^ should.

Oops.

>> + * parameter 'p' + 0.5.
>> + */
>> + {
>> + float p = p + 0.5, q = p;
>> + return q;
>> + }
>> +}
>> +
>> +void main()
>> +{
>> + gl_Position = gl_Vertex;
>> + gl_FrontColor = vec4(f(0.25));
>> +}
>> +
>> +[test]
>> +clear color 0.1 0.1 0.1 1.0
>> +clear
>> +
>> +draw rect -1 -1 2 2
>> +probe all rgb 0.75 0.75 0.75