On Sun, Jan 26, 2014 at 8:15 PM, Jordan Justen <jljus...@gmail.com> wrote:
> On Sun, Jan 26, 2014 at 6:14 PM, Matt Turner <matts...@gmail.com> wrote:
>> The check was in the wrong place, such that if a shader incorrectly put
>> a preprocessor token before the #version declaration, the version would
>> be resolved twice, leading to a segmentation fault when attempting to
>> redefine the __VERSION__ macro.
>>
>> #define GL_ARB_sample_shading
>> #version 130
>> void main() {}
>>
>> Also, rename glcpp_parser_resolve_version to
>> glcpp_parser_resolve_implicit_version to avoid confusion.
>> ---
>> I couldn't come up with something better to clarify the naming. Feel free
>> to suggest something better.
>>
>> src/glsl/glcpp/glcpp-parse.y | 22 +++++++++++-----------
>> src/glsl/glcpp/glcpp.h | 2 +-
>> src/glsl/glcpp/pp.c | 2 +-
>> 3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>> index 184e5c2..efcf139 100644
>> --- a/src/glsl/glcpp/glcpp-parse.y
>> +++ b/src/glsl/glcpp/glcpp-parse.y
>> @@ -194,7 +194,7 @@ line:
>> ralloc_asprintf_rewrite_tail (&parser->output,
>> &parser->output_length, "\n");
>> }
>> | HASH_LINE {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> } pp_tokens NEWLINE {
>>
>> if (parser->skip_stack == NULL ||
>> @@ -254,10 +254,10 @@ define:
>>
>> control_line:
>> HASH_DEFINE {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> } define
>> | HASH_UNDEF {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> } IDENTIFIER NEWLINE {
>> macro_t *macro = hash_table_find (parser->defines, $3);
>> if (macro) {
>> @@ -267,7 +267,7 @@ control_line:
>> ralloc_free ($3);
>> }
>> | HASH_IF {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> } conditional_tokens NEWLINE {
>> /* Be careful to only evaluate the 'if' expression if
>> * we are not skipping. When we are skipping, we
>> @@ -299,14 +299,14 @@ control_line:
>> _glcpp_parser_skip_stack_push_if (parser, & @1, 0);
>> }
>> | HASH_IFDEF {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> } IDENTIFIER junk NEWLINE {
>> macro_t *macro = hash_table_find (parser->defines, $3);
>> ralloc_free ($3);
>> _glcpp_parser_skip_stack_push_if (parser, & @1, macro !=
>> NULL);
>> }
>> | HASH_IFNDEF {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> } IDENTIFIER junk NEWLINE {
>> macro_t *macro = hash_table_find (parser->defines, $3);
>> ralloc_free ($3);
>> @@ -380,7 +380,7 @@ control_line:
>> _glcpp_parser_handle_version_declaration(parser, $2, $3,
>> true);
>> }
>> | HASH NEWLINE {
>> - glcpp_parser_resolve_version(parser);
>> + glcpp_parser_resolve_implicit_version(parser);
>> }
>> ;
>>
>> @@ -2024,6 +2024,9 @@
>> _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t
>> versio
>> {
>> const struct gl_extensions *extensions = parser->extensions;
>>
>> + if (parser->version_resolved)
>> + return;
>> +
>> parser->version_resolved = true;
>
> Would this lead to the version being stuck at the implicit version
> since parser->version_resolved would be set in the implicit case too?
>
> It looks like something like:
> #define GL_ARB_sample_shading
> #version 150
>
> ...might not end up hitting this code:
> if (version >= 150)
> add_builtin_define(parser, "GL_core_profile", 1);
>
> -Jordan
That's intended. #version has to be the first non-comment token. If
it's not, the version is 1.10 (or 1.00 in ES).
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
