[Mesa-dev] [PATCH v2] glcpp: Sync line number for macro
Tapani Pälli
tapani.palli at intel.com
Tue Aug 7 09:58:38 UTC 2018
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 at 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 at 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;
>
>
More information about the mesa-dev
mailing list