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 <ian.d.romanick@intel.com>

diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index f24aa17c82a8c5be9a930a84f5c3c0649181b02b..a6169739406a97d528ccd8f3e174b1e528f02605 100644 (file)
--- 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 <HASH> and <DEFINE> 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 <ival> INTEGER operator SPACE integer_constant
 %type <expression_value> 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 0f7fe632b56720ffdedd2bf36eadcfb4e387e815..a7c053bb4026a18e92e8357eb555bddeedd19c81 100644 (file)
--- 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 8a15397a033d9b94c8dc58ff637e7af75ef24c4d..4cc795338b24cb92850c9fd2519ed6dd025bb847 100644 (file)
--- 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)))
+
+