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 <[email protected]>

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 <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 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)))
+
+