[Mesa-dev] [PATCH] glcpp: initializes version to -1

[Karol Herbst]

2016-11-05 2:50 GMT+01:00 Ian Romanick <idr at freedesktop.org>:
> (Sorry about the top post. Sent from my phone.)
>
> That expression will allow versions like 0130 as valid.  If you just want to
> allow 0, you need a more complex regular expression.  I feel like that's
> just a bandage... what about other bad values like "#version -130"?  Won't
> that have the same problem that 0 currently has?
>

no, it doesn't.

I tested the patch with glsl_compiler

"#version 0130": 0:1(10): error: GLSL 0.88 is not supported. Supported
versions are: 1.10, 1.20, 1.30, 1.00 ES, and 3.00 ES

"#version 0": 0:1(10): error: GLSL 0.00 is not supported. Supported
versions are: 1.10, 1.20, 1.30, 1.00 ES, and 3.00 ES

"#version -130":0:1(10): preprocessor error: syntax error, unexpected
'-', expecting INTEGER or INTEGER_STRING

but

"#version 0512": 0:1(10): error: GLSL 3.30 is not supported. Supported
versions are: 1.10, 1.20, 1.30, 1.00 ES, and 3.00 ES

so the issue with this would be, that "0512" is parsed as 3.30, which
isn't right either, but the current master version does the same. \o/
new bug found

>
> On November 4, 2016 6:09:58 AM "Juan A. Suarez Romero" <jasuarez at igalia.com>
> wrote:
>
>> Shader can define #version as an integer, including 0.
>>
>> Initializes version to -1 to know later if shader has defined a #version
>> or not.
>>
>> It fixes 4 piglit tests:
>>   spec/glsl-1.10/compiler/version-0.frag: crash pass
>>   spec/glsl-1.10/compiler/version-0.vert: crash pass
>>   spec/glsl-es-3.00/compiler/version-0.frag: crash pass
>>   spec/glsl-es-3.00/compiler/version-0.vert: crash pass
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97420
>> ---
>>  src/compiler/glsl/glcpp/glcpp-parse.y | 8 ++++----
>>  src/compiler/glsl/glcpp/glcpp.h       | 2 +-
>>  src/compiler/glsl/glsl_lexer.ll       | 2 +-
>>  3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y
>> b/src/compiler/glsl/glcpp/glcpp-parse.y
>> index b80ff04..6207a62 100644
>> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
>> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
>> @@ -420,13 +420,13 @@ control_line_success:
>>                 _glcpp_parser_skip_stack_pop (parser, & @1);
>>         } NEWLINE
>>  |      HASH_TOKEN VERSION_TOKEN integer_constant NEWLINE {
>> -               if (parser->version != 0) {
>> +               if (parser->version != -1) {
>>                         glcpp_error(& @1, parser, "#version must appear on
>> the first line");
>>                 }
>>                 _glcpp_parser_handle_version_declaration(parser, $3, NULL,
>> true);
>>         }
>>  |      HASH_TOKEN VERSION_TOKEN integer_constant IDENTIFIER NEWLINE {
>> -               if (parser->version != 0) {
>> +               if (parser->version != -1) {
>>                         glcpp_error(& @1, parser, "#version must appear on
>> the first line");
>>                 }
>>                 _glcpp_parser_handle_version_declaration(parser, $3, $4,
>> true);
>> @@ -1360,7 +1360,7 @@ glcpp_parser_create(glcpp_extension_iterator
>> extensions, void *state, gl_api api
>>     parser->extensions = extensions;
>>     parser->state = state;
>>     parser->api = api;
>> -   parser->version = 0;
>> +   parser->version = -1;
>>
>>     parser->has_new_line_number = 0;
>>     parser->new_line_number = 1;
>> @@ -2293,7 +2293,7 @@
>> _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t
>> versio
>>                                           const char *es_identifier,
>>                                           bool explicitly_set)
>>  {
>> -   if (parser->version != 0)
>> +   if (parser->version != -1)
>>        return;
>>
>>     parser->version = version;
>> diff --git a/src/compiler/glsl/glcpp/glcpp.h
>> b/src/compiler/glsl/glcpp/glcpp.h
>> index bb4ad67..2acac0c 100644
>> --- a/src/compiler/glsl/glcpp/glcpp.h
>> +++ b/src/compiler/glsl/glcpp/glcpp.h
>> @@ -207,7 +207,7 @@ struct glcpp_parser {
>>         glcpp_extension_iterator extensions;
>>         void *state;
>>         gl_api api;
>> -       unsigned version;
>> +       int version;
>>         bool has_new_line_number;
>>         int new_line_number;
>>         bool has_new_source_number;
>> diff --git a/src/compiler/glsl/glsl_lexer.ll
>> b/src/compiler/glsl/glsl_lexer.ll
>> index b473af7..7d1d616 100644
>> --- a/src/compiler/glsl/glsl_lexer.ll
>> +++ b/src/compiler/glsl/glsl_lexer.ll
>> @@ -249,7 +249,7 @@ HASH                ^{SPC}#{SPC}
>>                                    yylval->identifier =
>> linear_strdup(mem_ctx, yytext);
>>                                    return IDENTIFIER;
>>                                 }
>> -<PP>[1-9][0-9]*                        {
>> +<PP>[0-9][0-9]*                        {
>>                                     yylval->n = strtol(yytext, NULL, 10);
>>                                     return INTCONSTANT;
>>                                 }
>> --
>> 2.7.4
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev