[v2] glcpp: Sync line number for macro

Submitted by Zhaowei Yuan on July 22, 2018, 8:36 p.m.

Details

Message ID 20180723084543epcas5p2126f0597ceb785d2cc8f38c0a2ea7bba~D8w7aRATO1680716807epcas5p2I@epcas5p2.samsung.com
State New
Headers show
Series "glcpp: Sync line number for macro" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Zhaowei Yuan July 22, 2018, 8:36 p.m.
Line number of a predefined macro should be set
as where it is referenced rather than declared

Patch v2 does nothing more except ccing more maintainers

Signed-off-by: zhaowei yuan <zhaowei.yuan@samsung.com>
Bugzilla: https://patchwork.freedesktop.org/patch/225818/
---
 src/compiler/glsl/glcpp/glcpp-lex.l   |  1 +
 src/compiler/glsl/glcpp/glcpp-parse.y | 55 ++++++++++++++++++++++++++++++-----
 src/compiler/glsl/glcpp/glcpp.h       |  4 ++-
 src/compiler/glsl/glcpp/pp.c          |  3 +-
 4 files changed, 54 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l b/src/compiler/glsl/glcpp/glcpp-lex.l
index 9cfcc12..86b82c2 100644
--- a/src/compiler/glsl/glcpp/glcpp-lex.l
+++ b/src/compiler/glsl/glcpp/glcpp-lex.l
@@ -50,6 +50,7 @@  void glcpp_set_column (int  column_no , yyscan_t yyscanner);
 		yylloc->first_line = yylloc->last_line = yylineno;	\
 		yycolumn += yyleng;					\
 		yylloc->last_column = yycolumn + 1;			\
+		yylloc->position = (yytext - YY_CURRENT_BUFFER_LVALUE->yy_ch_buf); \
 		parser->has_new_line_number = 0;			\
 		parser->has_new_source_number = 0;			\
 	} while(0);
diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y
index ccb3aa1..68f8477 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -1021,7 +1021,7 @@  _token_list_append_list(token_list_t *list, token_list_t *tail)
 }
 
 static token_list_t *
-_token_list_copy(glcpp_parser_t *parser, token_list_t *other)
+_token_list_copy(glcpp_parser_t *parser, token_list_t *other, token_node_t *macro_node)
 {
    token_list_t *copy;
    token_node_t *node;
@@ -1033,6 +1033,12 @@  _token_list_copy(glcpp_parser_t *parser, token_list_t *other)
    for (node = other->head; node; node = node->next) {
       token_t *new_token = linear_alloc_child(parser->linalloc, sizeof(token_t));
       *new_token = *node->token;
+
+      if(macro_node) {
+         new_token->location.first_line = macro_node->token->location.first_line;
+         new_token->location.last_line = macro_node->token->location.last_line;
+      }
+
       _token_list_append (parser, copy, new_token);
    }
 
@@ -1349,7 +1355,7 @@  add_builtin_define(glcpp_parser_t *parser, const char *name, int value)
 
 glcpp_parser_t *
 glcpp_parser_create(const struct gl_extensions *extension_list,
-                    glcpp_extension_iterator extensions, void *state, gl_api api)
+                    glcpp_extension_iterator extensions, void *state, gl_api api, const char *input)
 {
    glcpp_parser_t *parser;
 
@@ -1377,6 +1383,11 @@  glcpp_parser_create(const struct gl_extensions *extension_list,
    parser->lex_from_list = NULL;
    parser->lex_from_node = NULL;
 
+   parser->input = _mesa_string_buffer_create(parser, strlen(input) + 1);
+   strcpy(parser->input->buf, input);
+   parser->input->buf[strlen(input)] = '\0';
+   parser->input->length = strlen(input);
+
    parser->output = _mesa_string_buffer_create(parser,
                                                INITIAL_PP_OUTPUT_BUF_SIZE);
    parser->info_log = _mesa_string_buffer_create(parser,
@@ -1441,7 +1452,7 @@  typedef enum function_status
 static function_status_t
 _arguments_parse(glcpp_parser_t *parser,
                  argument_list_t *arguments, token_node_t *node,
-                 token_node_t **last)
+                 token_node_t **last, int *end_position)
 {
    token_list_t *argument;
    int paren_count;
@@ -1465,8 +1476,10 @@  _arguments_parse(glcpp_parser_t *parser,
          paren_count++;
       } else if (node->token->type == ')') {
          paren_count--;
-         if (paren_count == 0)
+         if (paren_count == 0) {
+            *end_position = node->token->location.position;
             break;
+         }
       }
 
       if (node->token->type == ',' && paren_count == 1) {
@@ -1702,6 +1715,28 @@  _glcpp_parser_apply_pastes(glcpp_parser_t *parser, token_list_t *list)
    list->non_space_tail = list->tail;
 }
 
+static int
+_glcpp_parser_get_line(glcpp_parser_t *parser, int offset)
+{
+   int line = 1;
+   int i;
+
+   for(i = 0; parser->input->buf[i] && i <= offset; i++) {
+      if(parser->input->buf[i] == '\n' || parser->input->buf[i] == '\r')
+         line++;
+   }
+
+   return line;
+}
+
+static void
+_glcpp_sync_location(glcpp_parser_t *parser, token_t *token, token_t *macro_token)
+{
+   token->location.source = macro_token->location.source;
+   token->location.first_line = _glcpp_parser_get_line(parser, token->location.position);
+   token->location.last_line = token->location.first_line;
+}
+
 /* This is a helper function that's essentially part of the
  * implementation of _glcpp_parser_expand_node. It shouldn't be called
  * except for by that function.
@@ -1732,7 +1767,9 @@  _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
    argument_list_t *arguments;
    function_status_t status;
    token_list_t *substituted;
+   token_node_t *macro_node = node;
    int parameter_index;
+   int end_position;
 
    identifier = node->token->value.str;
 
@@ -1742,7 +1779,7 @@  _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
    assert(macro->is_function);
 
    arguments = _argument_list_create(parser);
-   status = _arguments_parse(parser, arguments, node, last);
+   status = _arguments_parse(parser, arguments, node, last, &end_position);
 
    switch (status) {
    case FUNCTION_STATUS_SUCCESS:
@@ -1784,7 +1821,8 @@  _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
           * placeholder token for an empty argument. */
          if (argument->head) {
             token_list_t *expanded_argument;
-            expanded_argument = _token_list_copy(parser, argument);
+            expanded_argument = _token_list_copy(parser, argument, NULL);
+            _glcpp_sync_location(parser, expanded_argument->head->token, macro_node->token);
             _glcpp_parser_expand_token_list(parser, expanded_argument, mode);
             _token_list_append_list(substituted, expanded_argument);
          } else {
@@ -1795,6 +1833,8 @@  _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
             _token_list_append(parser, substituted, new_token);
          }
       } else {
+         node->token->location.position = end_position;
+         _glcpp_sync_location(parser, node->token, macro_node->token);
          _token_list_append(parser, substituted, node->token);
       }
    }
@@ -1835,6 +1875,7 @@  _glcpp_parser_expand_node(glcpp_parser_t *parser, token_node_t *node,
    const char *identifier;
    struct hash_entry *entry;
    macro_t *macro;
+   token_node_t *macro_node = node;
 
    /* We only expand identifiers */
    if (token->type != IDENTIFIER) {
@@ -1887,7 +1928,7 @@  _glcpp_parser_expand_node(glcpp_parser_t *parser, token_node_t *node,
       if (macro->replacements == NULL)
          return _token_list_create_with_one_space(parser);
 
-      replacement = _token_list_copy(parser, macro->replacements);
+      replacement = _token_list_copy(parser, macro->replacements, macro_node);
       _glcpp_parser_apply_pastes(parser, replacement);
       return replacement;
    }
diff --git a/src/compiler/glsl/glcpp/glcpp.h b/src/compiler/glsl/glcpp/glcpp.h
index c7e382e..eed6923 100644
--- a/src/compiler/glsl/glcpp/glcpp.h
+++ b/src/compiler/glsl/glcpp/glcpp.h
@@ -78,6 +78,7 @@  typedef struct YYLTYPE {
    int first_column;
    int last_line;
    int last_column;
+   int position;
    unsigned source;
 } YYLTYPE;
 # define YYLTYPE_IS_DECLARED 1
@@ -204,6 +205,7 @@  struct glcpp_parser {
 	token_list_t *lex_from_list;
 	token_node_t *lex_from_node;
 	struct _mesa_string_buffer *output;
+	struct _mesa_string_buffer *input;
 	struct _mesa_string_buffer *info_log;
 	int error;
 	glcpp_extension_iterator extensions;
@@ -229,7 +231,7 @@  struct glcpp_parser {
 
 glcpp_parser_t *
 glcpp_parser_create(const struct gl_extensions *extension_list,
-                    glcpp_extension_iterator extensions, void *state, gl_api api);
+                    glcpp_extension_iterator extensions, void *state, gl_api api, const char *input);
 
 int
 glcpp_parser_parse (glcpp_parser_t *parser);
diff --git a/src/compiler/glsl/glcpp/pp.c b/src/compiler/glsl/glcpp/pp.c
index 32dee11..1ac733f 100644
--- a/src/compiler/glsl/glcpp/pp.c
+++ b/src/compiler/glsl/glcpp/pp.c
@@ -228,7 +228,7 @@  glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
 {
 	int errors;
 	glcpp_parser_t *parser =
-		glcpp_parser_create(&gl_ctx->Extensions, extensions, state, gl_ctx->API);
+		glcpp_parser_create(&gl_ctx->Extensions, extensions, state, gl_ctx->API, *shader);
 
 	if (! gl_ctx->Const.DisableGLSLLineContinuations)
 		*shader = remove_line_continuations(parser, *shader);
@@ -247,6 +247,7 @@  glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
 	/* Crimp the buffer first, to conserve memory */
 	_mesa_string_buffer_crimp_to_fit(parser->output);
 
+	ralloc_steal(ralloc_ctx, parser->input->buf);
 	ralloc_steal(ralloc_ctx, parser->output->buf);
 	*shader = parser->output->buf;
 

Comments

On 07/22/2018 11:36 PM, zhaowei yuan wrote:
> Line number of a predefined macro should be set
> as where it is referenced rather than declared
> 
> Patch v2 does nothing more except ccing more maintainers
> 
> Signed-off-by: zhaowei yuan <zhaowei.yuan@samsung.com>
> Bugzilla: https://patchwork.freedesktop.org/patch/225818/

Probably a copy-paste mistake here, this should be the bug url instead 
of patchwork one, https://bugs.freedesktop.org/show_bug.cgi?id=99730.

IMO commit message should also mention the tests that it fixes, which are:

dEQP-GLES2.functional.shaders.preprocessor.predefined_macros.line_2_vertex
dEQP-GLES2.functional.shaders.preprocessor.predefined_macros.line_2_fragment

(same tests exist also in dEQP-GLES3)

I've run this through Intel CI and did not see any failures, I also run 
some dEQP preprocessor tests with valgrind and did not see anything 
reported;

Tested-by: Tapani Pälli <tapani.palli@intel.com>

some small coding style issues noted below:

> ---
>   src/compiler/glsl/glcpp/glcpp-lex.l   |  1 +
>   src/compiler/glsl/glcpp/glcpp-parse.y | 55 ++++++++++++++++++++++++++++++-----
>   src/compiler/glsl/glcpp/glcpp.h       |  4 ++-
>   src/compiler/glsl/glcpp/pp.c          |  3 +-
>   4 files changed, 54 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l b/src/compiler/glsl/glcpp/glcpp-lex.l
> index 9cfcc12..86b82c2 100644
> --- a/src/compiler/glsl/glcpp/glcpp-lex.l
> +++ b/src/compiler/glsl/glcpp/glcpp-lex.l
> @@ -50,6 +50,7 @@ void glcpp_set_column (int  column_no , yyscan_t yyscanner);
>   		yylloc->first_line = yylloc->last_line = yylineno;	\
>   		yycolumn += yyleng;					\
>   		yylloc->last_column = yycolumn + 1;			\
> +		yylloc->position = (yytext - YY_CURRENT_BUFFER_LVALUE->yy_ch_buf); \
>   		parser->has_new_line_number = 0;			\
>   		parser->has_new_source_number = 0;			\
>   	} while(0);
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y
> index ccb3aa1..68f8477 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -1021,7 +1021,7 @@ _token_list_append_list(token_list_t *list, token_list_t *tail)
>   }
>   
>   static token_list_t *
> -_token_list_copy(glcpp_parser_t *parser, token_list_t *other)
> +_token_list_copy(glcpp_parser_t *parser, token_list_t *other, token_node_t *macro_node)
>   {
>      token_list_t *copy;
>      token_node_t *node;
> @@ -1033,6 +1033,12 @@ _token_list_copy(glcpp_parser_t *parser, token_list_t *other)
>      for (node = other->head; node; node = node->next) {
>         token_t *new_token = linear_alloc_child(parser->linalloc, sizeof(token_t));
>         *new_token = *node->token;
> +
> +      if(macro_node) {

should have space after if

> +         new_token->location.first_line = macro_node->token->location.first_line;
> +         new_token->location.last_line = macro_node->token->location.last_line;
> +      }
> +
>         _token_list_append (parser, copy, new_token);
>      }
>   
> @@ -1349,7 +1355,7 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value)
>   
>   glcpp_parser_t *
>   glcpp_parser_create(const struct gl_extensions *extension_list,
> -                    glcpp_extension_iterator extensions, void *state, gl_api api)
> +                    glcpp_extension_iterator extensions, void *state, gl_api api, const char *input)
>   {
>      glcpp_parser_t *parser;
>   
> @@ -1377,6 +1383,11 @@ glcpp_parser_create(const struct gl_extensions *extension_list,
>      parser->lex_from_list = NULL;
>      parser->lex_from_node = NULL;
>   
> +   parser->input = _mesa_string_buffer_create(parser, strlen(input) + 1);
> +   strcpy(parser->input->buf, input);
> +   parser->input->buf[strlen(input)] = '\0';
> +   parser->input->length = strlen(input);
> +
>      parser->output = _mesa_string_buffer_create(parser,
>                                                  INITIAL_PP_OUTPUT_BUF_SIZE);
>      parser->info_log = _mesa_string_buffer_create(parser,
> @@ -1441,7 +1452,7 @@ typedef enum function_status
>   static function_status_t
>   _arguments_parse(glcpp_parser_t *parser,
>                    argument_list_t *arguments, token_node_t *node,
> -                 token_node_t **last)
> +                 token_node_t **last, int *end_position)
>   {
>      token_list_t *argument;
>      int paren_count;
> @@ -1465,8 +1476,10 @@ _arguments_parse(glcpp_parser_t *parser,
>            paren_count++;
>         } else if (node->token->type == ')') {
>            paren_count--;
> -         if (paren_count == 0)
> +         if (paren_count == 0) {
> +            *end_position = node->token->location.position;
>               break;
> +         }
>         }
>   
>         if (node->token->type == ',' && paren_count == 1) {
> @@ -1702,6 +1715,28 @@ _glcpp_parser_apply_pastes(glcpp_parser_t *parser, token_list_t *list)
>      list->non_space_tail = list->tail;
>   }
>   
> +static int
> +_glcpp_parser_get_line(glcpp_parser_t *parser, int offset)
> +{
> +   int line = 1;
> +   int i;
> +
> +   for(i = 0; parser->input->buf[i] && i <= offset; i++) {

should have space after for

> +      if(parser->input->buf[i] == '\n' || parser->input->buf[i] == '\r')

should have space after if

> +         line++;
> +   }
> +
> +   return line;
> +}
> +
> +static void
> +_glcpp_sync_location(glcpp_parser_t *parser, token_t *token, token_t *macro_token)
> +{
> +   token->location.source = macro_token->location.source;
> +   token->location.first_line = _glcpp_parser_get_line(parser, token->location.position);
> +   token->location.last_line = token->location.first_line;
> +}
> +
>   /* This is a helper function that's essentially part of the
>    * implementation of _glcpp_parser_expand_node. It shouldn't be called
>    * except for by that function.
> @@ -1732,7 +1767,9 @@ _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
>      argument_list_t *arguments;
>      function_status_t status;
>      token_list_t *substituted;
> +   token_node_t *macro_node = node;
>      int parameter_index;
> +   int end_position;
>   
>      identifier = node->token->value.str;
>   
> @@ -1742,7 +1779,7 @@ _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
>      assert(macro->is_function);
>   
>      arguments = _argument_list_create(parser);
> -   status = _arguments_parse(parser, arguments, node, last);
> +   status = _arguments_parse(parser, arguments, node, last, &end_position);
>   
>      switch (status) {
>      case FUNCTION_STATUS_SUCCESS:
> @@ -1784,7 +1821,8 @@ _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
>             * placeholder token for an empty argument. */
>            if (argument->head) {
>               token_list_t *expanded_argument;
> -            expanded_argument = _token_list_copy(parser, argument);
> +            expanded_argument = _token_list_copy(parser, argument, NULL);
> +            _glcpp_sync_location(parser, expanded_argument->head->token, macro_node->token);
>               _glcpp_parser_expand_token_list(parser, expanded_argument, mode);
>               _token_list_append_list(substituted, expanded_argument);
>            } else {
> @@ -1795,6 +1833,8 @@ _glcpp_parser_expand_function(glcpp_parser_t *parser, token_node_t *node,
>               _token_list_append(parser, substituted, new_token);
>            }
>         } else {
> +         node->token->location.position = end_position;
> +         _glcpp_sync_location(parser, node->token, macro_node->token);
>            _token_list_append(parser, substituted, node->token);
>         }
>      }
> @@ -1835,6 +1875,7 @@ _glcpp_parser_expand_node(glcpp_parser_t *parser, token_node_t *node,
>      const char *identifier;
>      struct hash_entry *entry;
>      macro_t *macro;
> +   token_node_t *macro_node = node;
>   
>      /* We only expand identifiers */
>      if (token->type != IDENTIFIER) {
> @@ -1887,7 +1928,7 @@ _glcpp_parser_expand_node(glcpp_parser_t *parser, token_node_t *node,
>         if (macro->replacements == NULL)
>            return _token_list_create_with_one_space(parser);
>   
> -      replacement = _token_list_copy(parser, macro->replacements);
> +      replacement = _token_list_copy(parser, macro->replacements, macro_node);
>         _glcpp_parser_apply_pastes(parser, replacement);
>         return replacement;
>      }
> diff --git a/src/compiler/glsl/glcpp/glcpp.h b/src/compiler/glsl/glcpp/glcpp.h
> index c7e382e..eed6923 100644
> --- a/src/compiler/glsl/glcpp/glcpp.h
> +++ b/src/compiler/glsl/glcpp/glcpp.h
> @@ -78,6 +78,7 @@ typedef struct YYLTYPE {
>      int first_column;
>      int last_line;
>      int last_column;
> +   int position;
>      unsigned source;
>   } YYLTYPE;
>   # define YYLTYPE_IS_DECLARED 1
> @@ -204,6 +205,7 @@ struct glcpp_parser {
>   	token_list_t *lex_from_list;
>   	token_node_t *lex_from_node;
>   	struct _mesa_string_buffer *output;
> +	struct _mesa_string_buffer *input;
>   	struct _mesa_string_buffer *info_log;
>   	int error;
>   	glcpp_extension_iterator extensions;
> @@ -229,7 +231,7 @@ struct glcpp_parser {
>   
>   glcpp_parser_t *
>   glcpp_parser_create(const struct gl_extensions *extension_list,
> -                    glcpp_extension_iterator extensions, void *state, gl_api api);
> +                    glcpp_extension_iterator extensions, void *state, gl_api api, const char *input);
>   
>   int
>   glcpp_parser_parse (glcpp_parser_t *parser);
> diff --git a/src/compiler/glsl/glcpp/pp.c b/src/compiler/glsl/glcpp/pp.c
> index 32dee11..1ac733f 100644
> --- a/src/compiler/glsl/glcpp/pp.c
> +++ b/src/compiler/glsl/glcpp/pp.c
> @@ -228,7 +228,7 @@ glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
>   {
>   	int errors;
>   	glcpp_parser_t *parser =
> -		glcpp_parser_create(&gl_ctx->Extensions, extensions, state, gl_ctx->API);
> +		glcpp_parser_create(&gl_ctx->Extensions, extensions, state, gl_ctx->API, *shader);
>   
>   	if (! gl_ctx->Const.DisableGLSLLineContinuations)
>   		*shader = remove_line_continuations(parser, *shader);
> @@ -247,6 +247,7 @@ glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
>   	/* Crimp the buffer first, to conserve memory */
>   	_mesa_string_buffer_crimp_to_fit(parser->output);
>   
> +	ralloc_steal(ralloc_ctx, parser->input->buf);
>   	ralloc_steal(ralloc_ctx, parser->output->buf);
>   	*shader = parser->output->buf;
>   
>