[Mesa-dev] [PATCH 02/15] glsl: Do not allow undefining the built-in macros

[Anuj Phogat]

On Wed, Jun 11, 2014 at 5:32 PM, Carl Worth <cworth at cworth.org> wrote:
> Anuj Phogat <anuj.phogat at gmail.com> writes:
>> Fixes piglit tests in spec/glsl-es-3.00/compile:
>> undef-__FILE__.vert
>> undef-GL_ES.vert
>> undef-__LINE__.vert
>> undef-__VERSION__.vert
>
> I see the code that will fix __FILE__, __LINE__, and __VERSION__, but
> not GL_ES. Should that line be dropped from the commit message?
>
I missed adding code for GL_ES :(.
>> +             if (strcmp("__LINE__", $3) == 0
>> +                 || strcmp("__FILE__", $3) == 0
>> +                 || strcmp("__VERSION__", $3) == 0)
>> +                     glcpp_error(& @1, parser, "Built-in (pre-defined)"
>> +                                 " macro names can not be undefined.");
>> +
>
Following changes in above code will also fix the GL_ES macro case:
-                   || strcmp("__VERSION__", $3) == 0)
+                   || strcmp("__VERSION__", $3) == 0
+                   || (parser->is_gles && strcmp("GL_ES", $3) == 0))

> In the future, we might think about looping a list of defined builtins,
> (which might make it easier to support conditional things like I imagine
> GL_ES is). But the above looks fine for now.
>
> If the commit message is updated as appropriate based on my comment
> above:
>
> Reviewed-by: Carl Worth <cworth at cworth.org>
>
Do you want me to send out an updated patch with changes suggested
by me in above comment or I can just push it with your r-b?

> But I don't want to see any changes to the pre-processor without
> corresponding new tests added to glcpp/tests. I'm writing such tests for
> each patch in your series, and I'll follow up here with them.
Thanks Carl. I'll take care of it in future.

>
> Thanks,
>
> -Carl
>
> --
> carl.d.worth at intel.com