Re: [Mesa-dev] [PATCH 3/5] glcpp: Set extension defines after resolving the GLSL version.

2014-01-27 Thread Matt Turner
On Fri, Jan 17, 2014 at 9:08 PM, Matt Turner matts...@gmail.com wrote:
 Cc: mesa-sta...@lists.freedesktop.org

Unless there are objections, I think we should probably skip this
patch going to the stable branches. It wound up causing a couple of
regressions, and fixed a pretty minor (at least in my opinion) issue.
If we do want to pick it over, the following commits should be
squashed in:

73c3c7e3
3e0e9e3b
c59a605c
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/5] glcpp: Set extension defines after resolving the GLSL version.

2014-01-17 Thread Matt Turner
Instead of defining preprocessor macros in glcpp_parser_create based on
the GL API, wait until the shader version has been resolved. Doing this
allows us to correctly set (and not set) preprocessor macros for
extensions allowed by the API but not the shader, as in the case of
ARB_ES3_compatibility.

The shader version has been resolved when the preprocessor encounters
the first preprocessor token, since the GLSL spec says

   The #version directive must occur in a shader before anything else,
except for comments and white space.

Specifically, if a #version token is found the version is known
explicitly, and if any other preprocessor token is found then the GLSL
version is implicitly 1.10.

Cc: mesa-sta...@lists.freedesktop.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71630
---
The diff looks awful for a few reasons:
  - had to change $num in a number of places in the bison code
  - had to add a new define rule because of the new mid-rule action
  - code movement from glcpp_parser_create to 
_glcpp_parser_handle_version_declaration

but it's actually not a complicated patch.

 src/glsl/glcpp/glcpp-parse.y | 293 +++
 src/glsl/glcpp/glcpp.h   |   7 +-
 src/glsl/glcpp/pp.c  |   4 +-
 3 files changed, 167 insertions(+), 137 deletions(-)

diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index 55c4981..c774662 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -135,7 +135,7 @@ _glcpp_parser_skip_stack_pop (glcpp_parser_t *parser, 
YYLTYPE *loc);
 
 static void
 _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t 
version,
- const char *ident);
+ const char *ident, bool 
explicitly_set);
 
 static int
 glcpp_parser_lex (YYSTYPE *yylval, YYLTYPE *yylloc, glcpp_parser_t *parser);
@@ -194,12 +194,15 @@ line:
control_line {
ralloc_asprintf_rewrite_tail (parser-output, 
parser-output_length, \n);
}
-|  HASH_LINE pp_tokens NEWLINE {
+|  HASH_LINE {
+   glcpp_parser_resolve_version(parser);
+   } pp_tokens NEWLINE {
+
if (parser-skip_stack == NULL ||
parser-skip_stack-type == SKIP_NO_SKIP)
{
_glcpp_parser_expand_and_lex_from (parser,
-  LINE_EXPANDED, $2);
+  LINE_EXPANDED, $3);
}
}
 |  text_line {
@@ -238,25 +241,35 @@ expanded_line:
}
 ;
 
-control_line:
-   HASH_DEFINE OBJ_IDENTIFIER replacement_list NEWLINE {
-   _define_object_macro (parser,  @2, $2, $3);
+define:
+   OBJ_IDENTIFIER replacement_list NEWLINE {
+   _define_object_macro (parser,  @1, $1, $2);
}
-|  HASH_DEFINE FUNC_IDENTIFIER '(' ')' replacement_list NEWLINE {
-   _define_function_macro (parser,  @2, $2, NULL, $5);
+|  FUNC_IDENTIFIER '(' ')' replacement_list NEWLINE {
+   _define_function_macro (parser,  @1, $1, NULL, $4);
}
-|  HASH_DEFINE FUNC_IDENTIFIER '(' identifier_list ')' replacement_list 
NEWLINE {
-   _define_function_macro (parser,  @2, $2, $4, $6);
+|  FUNC_IDENTIFIER '(' identifier_list ')' replacement_list NEWLINE {
+   _define_function_macro (parser,  @1, $1, $3, $5);
}
-|  HASH_UNDEF IDENTIFIER NEWLINE {
-   macro_t *macro = hash_table_find (parser-defines, $2);
+;
+
+control_line:
+   HASH_DEFINE {
+   glcpp_parser_resolve_version(parser);
+   } define
+|  HASH_UNDEF {
+   glcpp_parser_resolve_version(parser);
+   } IDENTIFIER NEWLINE {
+   macro_t *macro = hash_table_find (parser-defines, $3);
if (macro) {
-   hash_table_remove (parser-defines, $2);
+   hash_table_remove (parser-defines, $3);
ralloc_free (macro);
}
-   ralloc_free ($2);
+   ralloc_free ($3);
}
-|  HASH_IF conditional_tokens NEWLINE {
+|  HASH_IF {
+   glcpp_parser_resolve_version(parser);
+   } conditional_tokens NEWLINE {
/* Be careful to only evaluate the 'if' expression if
 * we are not skipping. When we are skipping, we
 * simply push a new 0-valued 'if' onto the skip
@@ -268,7 +281,7 @@ control_line:
parser-skip_stack-type == SKIP_NO_SKIP)
{
_glcpp_parser_expand_and_lex_from (parser,
-  IF_EXPANDED, $2);
+  IF_EXPANDED, $3);
}   
else
{
@@ -286,15 +299,19 @@