[Mesa-dev] [PATCH] glsl/glcpp: Fix handling of commas that result from macro expansion

[Carl Worth]

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.
---
 src/glsl/glcpp/glcpp-parse.y                       | 13 +-----------
 .../tests/039-func-arg-obj-macro-with-comma.c      | 21 ++++++++++++++++++++
 .../039-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 82fba20..8cf9258 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 <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 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 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
@@ -1161,9 +1161,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;
@@ -1846,14 +1843,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 0f7fe63..a7c053b 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 8a15397..4cc7953 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.0.0