[Mesa-dev] [PATCH v2] glcpp: Sync line number for macro
Tapani Pälli
tapani.palli at intel.com
Fri Aug 10 06:14:32 UTC 2018
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.palli at 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.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;
>>>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list