From 41540997fbc74a6f67fe4e4e6990f2f3119c0201 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Thu, 3 Jul 2014 14:16:07 -0700 Subject: [PATCH] glsl/glcpp: Fix handling of commas that result from macro expansion Here is some additional stress testing of nested macros where the expansion of macros involves commas, (and whether those commas are interpreted as argument separators or not in subsequent function-like macro calls). Credit to the GCC documentation that directed my attention toward this issue: https://gcc.gnu.org/onlinedocs/gcc-3.2/cpp/Argument-Prescan.html Fixing the bug required only removing code from glcpp. When first testing the details of expansions involving commas, I had come to the mistaken conclusion that an expanded comma should never be treated as an argument separator, (so had introduced the rather ugly COMMA_FINAL token to represent this). In fact, an expanded comma should be treated as a separator, (as tested here), and this treatment can be avoided by judicious use of parentheses (as also tested here). With this simple removal of the COMMA_FINAL token, the behavior of glcpp matches that of gcc's preprocessor for all of these hairy cases. Reviewed-by: Ian Romanick --- src/glsl/glcpp/glcpp-parse.y | 13 +---------- .../tests/039-func-arg-obj-macro-with-comma.c | 21 +++++++++++++++++ ...9-func-arg-obj-macro-with-comma.c.expected | 23 +++++++++++++++++++ 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y index f24aa17c82a..a6169739406 100644 --- a/src/glsl/glcpp/glcpp-parse.y +++ b/src/glsl/glcpp/glcpp-parse.y @@ -178,7 +178,7 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value); /* We use HASH_TOKEN, DEFINE_TOKEN and VERSION_TOKEN (as opposed to * HASH, DEFINE, and VERSION) to avoid conflicts with other symbols, * (such as the and start conditions in the lexer). */ -%token COMMA_FINAL DEFINED ELIF_EXPANDED HASH_TOKEN DEFINE_TOKEN FUNC_IDENTIFIER OBJ_IDENTIFIER ELIF ELSE ENDIF ERROR_TOKEN IF IFDEF IFNDEF LINE PRAGMA UNDEF VERSION_TOKEN GARBAGE IDENTIFIER IF_EXPANDED INTEGER INTEGER_STRING LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE PLUS_PLUS MINUS_MINUS +%token DEFINED ELIF_EXPANDED HASH_TOKEN DEFINE_TOKEN FUNC_IDENTIFIER OBJ_IDENTIFIER ELIF ELSE ENDIF ERROR_TOKEN IF IFDEF IFNDEF LINE PRAGMA UNDEF VERSION_TOKEN GARBAGE IDENTIFIER IF_EXPANDED INTEGER INTEGER_STRING LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE PLUS_PLUS MINUS_MINUS %token PASTE %type INTEGER operator SPACE integer_constant %type expression @@ -1162,9 +1162,6 @@ _token_print (char **out, size_t *len, token_t *token) case MINUS_MINUS: ralloc_asprintf_rewrite_tail (out, len, "--"); break; - case COMMA_FINAL: - ralloc_asprintf_rewrite_tail (out, len, ","); - break; case DEFINED: ralloc_asprintf_rewrite_tail (out, len, "defined"); break; @@ -1847,14 +1844,6 @@ _glcpp_parser_expand_node (glcpp_parser_t *parser, /* We only expand identifiers */ if (token->type != IDENTIFIER) { - /* We change any COMMA into a COMMA_FINAL to prevent - * it being mistaken for an argument separator - * later. */ - if (token->type == ',') { - token->type = COMMA_FINAL; - token->value.ival = COMMA_FINAL; - } - return NULL; } diff --git a/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c b/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c index 0f7fe632b56..a7c053bb402 100644 --- a/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c +++ b/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c @@ -1,3 +1,24 @@ +/* This works. */ #define foo(a) (a) #define bar two,words foo(bar) + +/* So does this. */ +#define foo2(a,b) (a separate b) +#define foo2_wrap(a) foo2(a) +foo2_wrap(bar) + +/* But this generates an error. */ +#define foo_wrap(a) foo(a) +foo_wrap(bar) + +/* Adding parentheses to foo_wrap fixes it. */ +#define foo_wrap_parens(a) foo((a)) +foo_wrap_parens(bar) + +/* As does adding parentheses to bar */ +#define bar_parens (two,words) +foo_wrap(bar_parens) +foo_wrap_parens(bar_parens) + + diff --git a/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c.expected b/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c.expected index 8a15397a033..4cc795338b2 100644 --- a/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c.expected +++ b/src/glsl/glcpp/tests/039-func-arg-obj-macro-with-comma.c.expected @@ -1,3 +1,26 @@ +0:12(21): preprocessor error: Error: macro foo invoked with 2 arguments (expected 1) + + (two,words) + + + + +(two separate words) + + + +foo(two,words) + + + +((two,words)) + + + +((two,words)) +(((two,words))) + + -- 2.30.2