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.
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
