Re: [Mesa-dev] [PATCH v2] glcpp: Sync line number for macro

Thu, 09 Aug 2018 23:15:01 -0700 

Hi;

On 08/10/2018 09:08 AM, Zhaowei Yuan wrote:

Thank you for reminding me of the mistakes,  I'll re-commit a PATCH v3,
thanks
However, I've re-tested those failed cases with latest code on master branch
and got the same result. I don't why you didn't see anything wrong on your
board,
it doesn't make sense.



Sorry for confusion, what I meant is that I did not see any regressions 
that would occur from these changes. Only change was that they fix those 
preprocessor tests mentioned.



On 08/07/2018 5:58 PM, Tapani Pälli <tapani.pa...@intel.com> wrote:

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.y...@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.pa...@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;



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev






















					Reply via email to