[Mesa-dev] [PATCH] glsl: type check between switch init-expression and case

[Matt Turner]

On Mon, Jun 9, 2014 at 11:51 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
> On 06/10/2014 08:28 AM, Tapani Pälli wrote:
>> On 06/09/2014 09:40 PM, Matt Turner wrote:
>>> On Mon, Jun 9, 2014 at 4:29 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79724
>>>> ---
>>>>  src/glsl/ast_to_hir.cpp | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>>> index d1c77f1..131b25f 100644
>>>> --- a/src/glsl/ast_to_hir.cpp
>>>> +++ b/src/glsl/ast_to_hir.cpp
>>>> @@ -4629,6 +4629,15 @@ ast_case_label::hir(exec_list *instructions,
>>>>           }
>>>>        }
>>>>
>>>> +      /* Check that types match (no implicit conversions allowed). */
>>> Good news. GLSL 4.30 updated the text to say (I think as a
>>> clarification, so we should have this behavior always):
>> OK, should have realized to check the latest spec too. This is a good
>> addition.
>>
>>> The type of the init-expression value in a switch statement must be a
>>> scalar int or uint. The type of the constant-expression value in a case
>>> label also must be a scalar int or uint. When any pair of these values
>>> is tested for "equal value" and the types do not match, an implicit
>>> conversion will be done to convert the int to a uint (see section 4.1.10
>>> “Implicit Conversions”) before the compare is done.
>>>
>>> Please replace this comment with that spec citation in our usual
>>> format. I guess we'll need a little bit of code to handle implicit
>>> conversions here, but apparently only for int -> uint which is a
>>> relief.
>> Thanks, I will make the appropriate changes.
>
> Older versions of GLSL spec (like 3.30) does have "There are no implicit
> conversions between signed and unsigned integers." though, does this
> check need to be version specific or are we safe just supporting the
> implicit conversion?

Oh, good question. My mistake -- implicit conversions from int -> uint
were added by ARB_gpu_shader5 or GL 4.0. We should only perform them
if we're using either of those.

Thanks for catching that.