RFA: implement all missing macro expansion features

RFA: implement all missing macro expansion features

[Tom Tromey]

This patch obsoletes my earlier stringification patch.

This implements all the missing macro expansion features:
stringification, token pasting, and varargs (both C99 and GNU style,
including the special GNU token-pasting behavior).

Built and regtested on x86-64 (compile farm).
A bunch of new tests are included.
Ok?

Tom

:ADDPATCH macros:

2008-08-22  Tom Tromey  <tromey@redhat.com>

	* macrocmd.c (extract_identifier): Add is_parameter argument.
	(macro_define_command): Update.
	(macro_undef_command): Likewise.
	* macroexp.c (stringify): New function.
	(find_parameter): Likewise.
	(gather_arguments): Add nargs argument.  Handle varargs.
	(substitute_args): Add is_varargs and va_arg_name arguments.
	Handle varargs, splicing, stringification.  Use find_parameter.
	(expand): Handle varargs.

2008-08-21  Tom Tromey  <tromey@redhat.com>

	* gdb.base/macscp.exp: Add tests for stringification, splicing,
	and varargs.

diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index 8213c0d..9a4fdee 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -197,18 +197,36 @@ skip_ws (char **expp)
     ++*expp;
 }
 
+/* Try to find the bounds of an identifier.  If an identifier is
+   found, returns a newly allocated string; otherwise returns NULL.
+   EXPP is a pointer to an input string; it is updated to point to the
+   text following the identifier.  If IS_PARAMETER is true, this
+   function will also allow "..." forms as used in varargs macro
+   parameters.  */
 static char *
-extract_identifier (char **expp)
+extract_identifier (char **expp, int is_parameter)
 {
   char *result;
   char *p = *expp;
   unsigned int len;
-  if (! *p || ! macro_is_identifier_nondigit (*p))
-    return NULL;
-  for (++p;
-       *p && (macro_is_identifier_nondigit (*p) || macro_is_digit (*p));
-       ++p)
-    ;
+
+  if (is_parameter && !strncmp (p, "...", 3))
+    {
+      /* Ok.  */
+    }
+  else
+    {
+      if (! *p || ! macro_is_identifier_nondigit (*p))
+	return NULL;
+      for (++p;
+	   *p && (macro_is_identifier_nondigit (*p) || macro_is_digit (*p));
+	   ++p)
+	;
+    }
+
+  if (is_parameter && !strncmp (p, "...", 3))      
+    p += 3;
+
   len = p - *expp;
   result = (char *) xmalloc (len + 1);
   memcpy (result, *expp, len);
@@ -246,7 +264,7 @@ macro_define_command (char *exp, int from_tty)
   memset (&new_macro, 0, sizeof (struct macro_definition));
 
   skip_ws (&exp);
-  name = extract_identifier (&exp);
+  name = extract_identifier (&exp, 0);
   if (! name)
     error (_("Invalid macro name."));
   if (*exp == '(')
@@ -274,7 +292,7 @@ macro_define_command (char *exp, int from_tty)
 	      /* Must update new_macro as well... */
 	      new_macro.argv = (const char * const *) argv;
 	    }
-	  argv[new_macro.argc] = extract_identifier (&exp);
+	  argv[new_macro.argc] = extract_identifier (&exp, 1);
 	  if (! argv[new_macro.argc])
 	    error (_("Macro is missing an argument."));
 	  ++new_macro.argc;
@@ -317,7 +335,7 @@ macro_undef_command (char *exp, int from_tty)
     error (_("usage: macro undef NAME"));
 
   skip_ws (&exp);
-  name = extract_identifier (&exp);
+  name = extract_identifier (&exp, 0);
   if (! name)
     error (_("Invalid macro name."));
   macro_undef (macro_main (macro_user_macros), -1, name);
diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index 8102bc0..c3c8072 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -625,6 +625,51 @@ append_tokens_without_splicing (struct macro_buffer *dest,
                   _("unable to avoid splicing tokens during macro expansion"));
 }
 
+/* Stringify an argument, and insert it into DEST.  ARG is the text to
+   stringify; it is LEN bytes long.  */
+static void
+stringify (struct macro_buffer *dest, char *arg, int len)
+{
+  /* Trim initial whitespace from ARG.  */
+  while (len > 0 && macro_is_whitespace (*arg))
+    {
+      ++arg;
+      --len;
+    }
+
+  /* Trim trailing whitespace from ARG.  */
+  while (len > 0 && macro_is_whitespace (arg[len - 1]))
+    --len;
+
+  /* Insert the string.  */
+  appendc (dest, '"');
+  while (len > 0)
+    {
+      /* We could try to handle strange cases here, like control
+	 characters, but there doesn't seem to be much point.  */
+      if (macro_is_whitespace (*arg))
+	{
+	  /* Replace a sequence of whitespace with a single space.  */
+	  appendc (dest, ' ');
+	  while (len > 1 && macro_is_whitespace (arg[1]))
+	    {
+	      ++arg;
+	      --len;
+	    }
+	}
+      else if (*arg == '\\' || *arg == '"')
+	{
+	  appendc (dest, '\\');
+	  appendc (dest, *arg);
+	}
+      else
+	appendc (dest, *arg);
+      ++arg;
+      --len;
+    }
+  appendc (dest, '"');
+  dest->last_token = dest->len;
+}
 
 \f
 /* Expanding macros!  */
@@ -674,6 +719,11 @@ currently_rescanning (struct macro_name_list *list, const char *name)
 
    If SRC doesn't contain a properly terminated argument list, then
    raise an error.
+   
+   For a variadic macro, NARGS holds the number of formal arguments to
+   the macro.  For a GNU-style variadic macro, this should be the
+   number of named arguments.  For a non-variadic macro, NARGS should
+   be -1.
 
    Otherwise, return a pointer to the first element of an array of
    macro buffers referring to the argument texts, and set *ARGC_P to
@@ -694,7 +744,8 @@ currently_rescanning (struct macro_name_list *list, const char *name)
    following the invocation.  */
 
 static struct macro_buffer *
-gather_arguments (const char *name, struct macro_buffer *src, int *argc_p)
+gather_arguments (const char *name, struct macro_buffer *src,
+		  int nargs, int *argc_p)
 {
   struct macro_buffer tok;
   int args_len, args_size;
@@ -760,6 +811,20 @@ gather_arguments (const char *name, struct macro_buffer *src, int *argc_p)
                  the end of the argument list.  */
               if (depth == 0)
                 {
+		  /* In the varargs case, the last argument may be
+		     missing.  Add an empty argument in this case.  */
+		  if (nargs != -1 && args_len == nargs - 1)
+		    {
+		      /* Make sure we have room for the argument.  */
+		      if (args_len >= args_size)
+			{
+			  args_size++;
+			  args = xrealloc (args, sizeof (*args) * args_size);
+			}
+		      arg = &args[args_len++];
+		      set_token (arg, src->text, src->text);
+		    }
+
                   discard_cleanups (back_to);
                   *argc_p = args_len;
                   return args;
@@ -769,8 +834,11 @@ gather_arguments (const char *name, struct macro_buffer *src, int *argc_p)
             }
 
           /* If tok is a comma at top level, then that's the end of
-             the current argument.  */
-          else if (tok.len == 1 && tok.text[0] == ',' && depth == 0)
+             the current argument.  However, if we are handling a
+             variadic macro and we are computing the last argument, we
+             want to include the comma and remaining tokens.  */
+          else if (tok.len == 1 && tok.text[0] == ',' && depth == 0
+		   && (nargs == -1 || args_len < nargs))
             break;
 
           /* Extend the current argument to enclose this token.  If
@@ -801,6 +869,31 @@ static void scan (struct macro_buffer *dest,
                   void *lookup_baton);
 
 
+/* A helper function for substitute_args.  If the token TOK is the
+   name of a parameter, return the parameter's index.  ARGV is a
+   vector of all the arguments; ARGC is the number of arguments.  If
+   TOK is not an argument, return -1.  */
+static int
+find_parameter (const struct macro_buffer *tok,
+		int is_varargs, const struct macro_buffer *va_arg_name,
+		int argc, const char * const *argv)
+{
+  int i;
+
+  if (! tok->is_identifier)
+    return -1;
+
+  for (i = 0; i < argc; ++i)
+    if (tok->len == strlen (argv[i]) && ! memcmp (tok->text, argv[i], tok->len))
+      return i;
+
+  if (is_varargs && tok->len == va_arg_name->len
+      && ! memcmp (tok->text, va_arg_name->text, tok->len))
+    return argc - 1;
+
+  return -1;
+}
+ 
 /* Given the macro definition DEF, being invoked with the actual
    arguments given by ARGC and ARGV, substitute the arguments into the
    replacement list, and store the result in DEST.
@@ -812,6 +905,7 @@ static void scan (struct macro_buffer *dest,
 static void
 substitute_args (struct macro_buffer *dest, 
                  struct macro_definition *def,
+		 int is_varargs, const struct macro_buffer *va_arg_name,
                  int argc, struct macro_buffer *argv,
                  struct macro_name_list *no_loop,
                  macro_lookup_ftype *lookup_func,
@@ -819,6 +913,17 @@ substitute_args (struct macro_buffer *dest,
 {
   /* A macro buffer for the macro's replacement list.  */
   struct macro_buffer replacement_list;
+  /* The token we are currently considering.  */
+  struct macro_buffer tok;
+  /* The replacement list's pointer from just before TOK was lexed.  */
+  char *original_rl_start;
+  /* We have a single lookahead token to handle token splicing.  */
+  struct macro_buffer lookahead;
+  /* The lookahead token might not be valid.  */
+  int lookahead_valid;
+  /* The replacement list's pointer from just before LOOKAHEAD was
+     lexed.  */
+  char *lookahead_rl_start;
 
   init_shared_buffer (&replacement_list, (char *) def->replacement,
                       strlen (def->replacement));
@@ -826,16 +931,14 @@ substitute_args (struct macro_buffer *dest,
   gdb_assert (dest->len == 0);
   dest->last_token = 0;
 
+  original_rl_start = replacement_list.text;
+  if (! get_token (&tok, &replacement_list))
+    return;
+  lookahead_rl_start = replacement_list.text;
+  lookahead_valid = get_token (&lookahead, &replacement_list);
+
   for (;;)
     {
-      struct macro_buffer tok;
-      char *original_rl_start = replacement_list.text;
-      int substituted = 0;
-      
-      /* Find the next token in the replacement list.  */
-      if (! get_token (&tok, &replacement_list))
-        break;
-
       /* Just for aesthetics.  If we skipped some whitespace, copy
          that to DEST.  */
       if (tok.text > original_rl_start)
@@ -847,46 +950,169 @@ substitute_args (struct macro_buffer *dest,
       /* Is this token the stringification operator?  */
       if (tok.len == 1
           && tok.text[0] == '#')
-        error (_("Stringification is not implemented yet."));
+	{
+	  int arg;
 
-      /* Is this token the splicing operator?  */
-      if (tok.len == 2
-          && tok.text[0] == '#'
-          && tok.text[1] == '#')
-        error (_("Token splicing is not implemented yet."));
+	  if (!lookahead_valid)
+	    error (_("Stringification operator requires an argument."));
 
-      /* Is this token an identifier?  */
-      if (tok.is_identifier)
-        {
-          int i;
-
-          /* Is it the magic varargs parameter?  */
-          if (tok.len == 11
-              && ! memcmp (tok.text, "__VA_ARGS__", 11))
-            error (_("Variable-arity macros not implemented yet."));
-
-          /* Is it one of the parameters?  */
-          for (i = 0; i < def->argc; i++)
-            if (tok.len == strlen (def->argv[i])
-                && ! memcmp (tok.text, def->argv[i], tok.len))
-              {
-                struct macro_buffer arg_src;
-
-                /* Expand any macro invocations in the argument text,
-                   and append the result to dest.  Remember that scan
-                   mutates its source, so we need to scan a new buffer
-                   referring to the argument's text, not the argument
-                   itself.  */
-                init_shared_buffer (&arg_src, argv[i].text, argv[i].len);
-                scan (dest, &arg_src, no_loop, lookup_func, lookup_baton);
-                substituted = 1;
-                break;
-              }
-        }
+	  arg = find_parameter (&lookahead, is_varargs, va_arg_name,
+				def->argc, def->argv);
+	  if (arg == -1)
+	    error (_("Argument to stringification operator must name "
+		     "a macro parameter."));
 
-      /* If it wasn't a parameter, then just copy it across.  */
-      if (! substituted)
-        append_tokens_without_splicing (dest, &tok);
+	  stringify (dest, argv[arg].text, argv[arg].len);
+
+	  /* Read one token and let the loop iteration code handle the
+	     rest.  */
+	  lookahead_rl_start = replacement_list.text;
+	  lookahead_valid = get_token (&lookahead, &replacement_list);
+	}
+      /* Is this token the splicing operator?  */
+      else if (tok.len == 2
+	       && tok.text[0] == '#'
+	       && tok.text[1] == '#')
+	{
+	  /* Just ignore a stray token splicing operator.  Really this
+	     is an error, but there's no reason to penalize the
+	     user.  */
+	}
+      /* Is the next token the splicing operator?  */
+      else if (lookahead_valid
+	       && lookahead.len == 2
+	       && lookahead.text[0] == '#'
+	       && lookahead.text[1] == '#')
+	{
+	  int arg, finished = 0;
+	  int prev_was_comma = 0;
+
+	  /* Note that GCC warns if the result of splicing is not a
+	     token.  In the debugger there doesn't seem to be much
+	     benefit from doing this.  */
+
+	  /* Insert the first token.  */
+	  if (tok.len == 1 && tok.text[0] == ',')
+	    prev_was_comma = 1;
+	  else
+	    {
+	      int arg = find_parameter (&tok, is_varargs, va_arg_name,
+					def->argc, def->argv);
+	      if (arg != -1)
+		appendmem (dest, argv[arg].text, argv[arg].len);
+	      else
+		appendmem (dest, tok.text, tok.len);
+	    }
+
+	  /* Apply a possible sequence of ## operators.  */
+	  for (;;)
+	    {
+	      if (! get_token (&tok, &replacement_list))
+		{
+		  /* Just ignore a ## at the end.  */
+		  finished = 1;
+		  break;
+		}
+
+	      /* Handle a comma before a ##.  If we are handling
+		 varargs, and the token on the right hand side is the
+		 varargs marker, and the final argument is empty or
+		 missing, then drop the comma.  This is a GNU
+		 extension.  There is one ambiguous case here,
+		 involving pedantic behavior with an empty argument,
+		 but we settle that in favor of GNU-style (GCC uses an
+		 option).  If we aren't dealing with varargs, we
+		 simply insert the comma.  */
+	      if (prev_was_comma)
+		{
+		  if (! (is_varargs
+			 && tok.len == va_arg_name->len
+			 && !memcmp (tok.text, va_arg_name->text, tok.len)
+			 && argv[argc - 1].len == 0))
+		    appendmem (dest, ",", 1);
+		  prev_was_comma = 0;
+		}
+
+	      /* Insert the token.  If it is a parameter, insert the
+		 argument.  If it is a comma, treat it specially.  */
+	      if (tok.len == 1 && tok.text[0] == ',')
+		prev_was_comma = 1;
+	      else
+		{
+		  int arg = find_parameter (&tok, is_varargs, va_arg_name,
+					    def->argc, def->argv);
+		  if (arg != -1)
+		    appendmem (dest, argv[arg].text, argv[arg].len);
+		  else
+		    appendmem (dest, tok.text, tok.len);
+		}
+
+	      /* Now read another token.  If it is another splice, we
+		 loop.  */
+	      original_rl_start = replacement_list.text;
+	      if (! get_token (&tok, &replacement_list))
+		{
+		  finished = 1;
+		  break;
+		}
+
+	      if (! (tok.len == 2
+		     && tok.text[0] == '#'
+		     && tok.text[1] == '#'))
+		break;
+	    }
+
+	  if (prev_was_comma)
+	    {
+	      /* We saw a comma.  Insert it now.  */
+	      appendmem (dest, ",", 1);
+	    }
+
+          dest->last_token = dest->len;
+	  if (finished)
+	    lookahead_valid = 0;
+	  else
+	    {
+	      /* Set up for the loop iterator.  */
+	      lookahead = tok;
+	      lookahead_rl_start = original_rl_start;
+	      lookahead_valid = 1;
+	    }
+	}
+      else
+	{
+	  /* Is this token an identifier?  */
+	  int substituted = 0;
+	  int arg = find_parameter (&tok, is_varargs, va_arg_name,
+				    def->argc, def->argv);
+
+	  if (arg != -1)
+	    {
+	      struct macro_buffer arg_src;
+
+	      /* Expand any macro invocations in the argument text,
+		 and append the result to dest.  Remember that scan
+		 mutates its source, so we need to scan a new buffer
+		 referring to the argument's text, not the argument
+		 itself.  */
+	      init_shared_buffer (&arg_src, argv[arg].text, argv[arg].len);
+	      scan (dest, &arg_src, no_loop, lookup_func, lookup_baton);
+	      substituted = 1;
+	    }
+
+	  /* If it wasn't a parameter, then just copy it across.  */
+	  if (! substituted)
+	    append_tokens_without_splicing (dest, &tok);
+	}
+
+      if (! lookahead_valid)
+	break;
+
+      tok = lookahead;
+      original_rl_start = lookahead_rl_start;
+
+      lookahead_rl_start = replacement_list.text;
+      lookahead_valid = get_token (&lookahead, &replacement_list);
     }
 }
 
@@ -937,13 +1163,39 @@ expand (const char *id,
       struct macro_buffer *argv = NULL;
       struct macro_buffer substituted;
       struct macro_buffer substituted_src;
-
-      if (def->argc >= 1
-          && strcmp (def->argv[def->argc - 1], "...") == 0)
-        error (_("Varargs macros not implemented yet."));
+      struct macro_buffer va_arg_name;
+      int is_varargs = 0;
+
+      if (def->argc >= 1)
+	{
+	  if (strcmp (def->argv[def->argc - 1], "...") == 0)
+	    {
+	      /* In C99-style varargs, substitution is done using
+		 __VA_ARGS__.  */
+	      init_shared_buffer (&va_arg_name, "__VA_ARGS__",
+				  strlen ("__VA_ARGS__"));
+	      is_varargs = 1;
+	    }
+	  else
+	    {
+	      int len = strlen (def->argv[def->argc - 1]);
+	      if (len > 3
+		  && strcmp (def->argv[def->argc - 1] + len - 3, "...") == 0)
+		{
+		  /* In GNU-style varargs, the name of the
+		     substitution parameter is the name of the formal
+		     argument without the "...".  */
+		  init_shared_buffer (&va_arg_name,
+				      (char *) def->argv[def->argc - 1],
+				      len - 3);
+		  is_varargs = 1;
+		}
+	    }
+	}
 
       make_cleanup (free_current_contents, &argv);
-      argv = gather_arguments (id, src, &argc);
+      argv = gather_arguments (id, src, is_varargs ? def->argc : -1,
+			       &argc);
 
       /* If we couldn't find any argument list, then we don't expand
          this macro.  */
@@ -957,12 +1209,16 @@ expand (const char *id,
          this macro.  */
       if (argc != def->argc)
         {
+	  if (is_varargs && argc >= def->argc - 1)
+	    {
+	      /* Ok.  */
+	    }
           /* Remember that a sequence of tokens like "foo()" is a
              valid invocation of a macro expecting either zero or one
              arguments.  */
-          if (! (argc == 1
-                 && argv[0].len == 0
-                 && def->argc == 0))
+          else if (! (argc == 1
+		      && argv[0].len == 0
+		      && def->argc == 0))
             error (_("Wrong number of arguments to macro `%s' "
                    "(expected %d, got %d)."),
                    id, def->argc, argc);
@@ -976,8 +1232,8 @@ expand (const char *id,
          expand an argument until we see how it's being used.  */
       init_buffer (&substituted, 0);
       make_cleanup (cleanup_macro_buffer, &substituted);
-      substitute_args (&substituted, def, argc, argv, no_loop,
-                       lookup_func, lookup_baton);
+      substitute_args (&substituted, def, is_varargs, &va_arg_name,
+		       argc, argv, no_loop, lookup_func, lookup_baton);
 
       /* Now `substituted' is the macro's replacement list, with all
          argument values substituted into it properly.  Re-scan it for
diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp
index 3424714..d9fd97c 100644
--- a/gdb/testsuite/gdb.base/macscp.exp
+++ b/gdb/testsuite/gdb.base/macscp.exp
@@ -480,7 +480,100 @@ gdb_test "macro undef" \
     "usage: macro undef.*" \
     "macro undef with no arguments"
 
-# Regression test; this used to emit the wrong error.
+# Splicing tests.
+
 gdb_test "macro expand SPLICE(x, y)" \
-  "Token splicing is not implemented yet." \
-  "macro splicing lexes correctly"
+  "expands to: xy" \
+  "basic macro splicing"
+
+gdb_test "macro define robotinvasion 2010" \
+  "" \
+  "define splice helper"
+
+gdb_test "macro expand SPLICE(robot, invasion)" \
+  "expands to: *2010" \
+  "splicing plus expansion"
+
+# Varargs tests.
+
+gdb_test "macro define va_c99(...) fprintf (stderr, __VA_ARGS__)" \
+  "" \
+  "define first varargs helper"
+
+gdb_test "macro define va2_c99(x, y, ...) fprintf (stderr, x, y, __VA_ARGS__)" \
+  "" \
+  "define second varargs helper"
+
+gdb_test "macro define va_gnu(args...) fprintf (stderr, args)" \
+  "" \
+  "define third varargs helper"
+
+gdb_test "macro define va2_gnu(args...) fprintf (stderr, ## args)" \
+  "" \
+  "define fourth varargs helper"
+
+gdb_test "macro expand va_c99(one, two, three)" \
+  "expands to: *fprintf \\(stderr, *one, two, three\\)" \
+  "c99 varargs expansion"
+
+gdb_test "macro expand va_c99()" \
+  "expands to: *fprintf \\(stderr, *\\)" \
+  "c99 varargs expansion without an argument"
+
+gdb_test "macro expand va2_c99(one, two, three, four)" \
+  "expands to: *fprintf \\(stderr, *one, two, three, four\\)" \
+  "c99 varargs expansion, multiple formal arguments"
+
+gdb_test "macro expand va_gnu(one, two, three, four)" \
+  "expands to: *fprintf \\(stderr, *one, two, three, four\\)" \
+  "gnu varargs expansion"
+
+gdb_test "macro expand va_gnu()" \
+  "expands to: *fprintf \\(stderr, *\\)" \
+  "gnu varargs expansion without an argument"
+
+gdb_test "macro expand va2_gnu()" \
+  "expands to: *fprintf \\(stderr\\)" \
+  "gnu varargs expansion special splicing without an argument"
+
+# Stringification tests.
+
+gdb_test "macro define str(x) #x" \
+  "" \
+  "define stringification macro"
+
+gdb_test "macro define maude 5" \
+  "" \
+  "define first stringification helper"
+
+gdb_test "macro define xstr(x) str(x)" \
+  "" \
+  "define second stringification helper"
+
+gdb_test "print str(5)" \
+  " = \"5\"" \
+  "simple stringify"
+
+gdb_test "print str(hi bob)" \
+  " = \"hi bob\"" \
+  "stringify with one space"
+
+gdb_test "print str(  hi  bob  )" \
+  " = \"hi bob\"" \
+  "stringify with many spaces"
+
+gdb_test "print str(hi \"bob\")" \
+  " = \"hi \\\\\"bob\\\\\"\"" \
+  "stringify with quotes"
+
+gdb_test "print str(hi \\bob\\)" \
+  " = \"hi \\\\\\\\bob\\\\\\\\\"" \
+  "stringify with backslashes"
+
+gdb_test "print str(maude)" \
+  " = \"maude\"" \
+  "stringify without substitution"
+
+gdb_test "print xstr(maude)" \
+  " = \"5\"" \
+  "stringify with substitution"

Re: RFA: implement all missing macro expansion features

[Joel Brobecker]

:REVIEWMAIL:

> 2008-08-22  Tom Tromey  <tromey@redhat.com>
> 
> 	* macrocmd.c (extract_identifier): Add is_parameter argument.
> 	(macro_define_command): Update.
> 	(macro_undef_command): Likewise.
> 	* macroexp.c (stringify): New function.
> 	(find_parameter): Likewise.
> 	(gather_arguments): Add nargs argument.  Handle varargs.
> 	(substitute_args): Add is_varargs and va_arg_name arguments.
> 	Handle varargs, splicing, stringification.  Use find_parameter.
> 	(expand): Handle varargs.

Would anyone who is familiar with C macros like to take a look at
this one too? I looked at it, and it looks fine to me. In fact,
I must compliment Tom on his very nice style with lots of nice comments
explaining what he does. Very nice!

Tom, give it a few more days to see if we have any reaction, and
then go ahead and commit (let's not count the weekend, though).

> 2008-08-21  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/macscp.exp: Add tests for stringification, splicing,
> 	and varargs.

This is OK too.

> 
> diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
> index 8213c0d..9a4fdee 100644
> --- a/gdb/macrocmd.c
> +++ b/gdb/macrocmd.c
> @@ -197,18 +197,36 @@ skip_ws (char **expp)
>      ++*expp;
>  }
>  
> +/* Try to find the bounds of an identifier.  If an identifier is
> +   found, returns a newly allocated string; otherwise returns NULL.
> +   EXPP is a pointer to an input string; it is updated to point to the
> +   text following the identifier.  If IS_PARAMETER is true, this
> +   function will also allow "..." forms as used in varargs macro
> +   parameters.  */
>  static char *
> -extract_identifier (char **expp)
> +extract_identifier (char **expp, int is_parameter)

Thanks for adding the comment describing the function. Can you add an
extra empty line before the function declaration?

> +/* A helper function for substitute_args.  If the token TOK is the
> +   name of a parameter, return the parameter's index.  ARGV is a
> +   vector of all the arguments; ARGC is the number of arguments.  If
> +   TOK is not an argument, return -1.  */
> +static int
> +find_parameter (const struct macro_buffer *tok,
> +		int is_varargs, const struct macro_buffer *va_arg_name,
> +		int argc, const char * const *argv)

Same here. And perhaps if you wouldn't mind describing the IS_VARARGS
and VA_ARG_NAME parameters...

> +      /* Is this token the splicing operator?  */
> +      else if (tok.len == 2
> +	       && tok.text[0] == '#'
> +	       && tok.text[1] == '#')
> +	{
> +	  /* Just ignore a stray token splicing operator.  Really this
> +	     is an error, but there's no reason to penalize the
> +	     user.  */
> +	}

I don't see how this would be penalizing the user.  when could this
error actually happen? I assume that the compiler would reject an
incorrect expression, so it would only happen when defining the
macro in GDB? If it were the case, I would rather error out and
tell the user to fix its expression.  Or perhaps can this happen
after expansion?


-- 
Joel

Re: RFA: implement all missing macro expansion features

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Thanks for adding the comment describing the function. Can you add an
Joel> extra empty line before the function declaration?

Sure.  I was not sure which style is the preferred one -- though I see
now that this is in gdbint.texinfo.  That said, both styles are common
in gdb, and in macroexp.c in particular, the blank line only appears
in one case.

Joel> Same here. And perhaps if you wouldn't mind describing the IS_VARARGS
Joel> and VA_ARG_NAME parameters...

Sure.

>> +	  /* Just ignore a stray token splicing operator.  Really this
>> +	     is an error, but there's no reason to penalize the
>> +	     user.  */

Joel> I don't see how this would be penalizing the user.  when could
Joel> this error actually happen?

Some versions of gcc will pass a macro like this through.
Trunk rejects code that tries to do this.  By 'this' I mean error
cases like having '##' at the start or end of the macro definition.

So, it can only happen in very weird circumstances.

I can make it an error.  I doubt anybody will notice.
I'll resubmit the patch a bit later.

Tom

Re: RFA: implement all missing macro expansion features

[Joel Brobecker]

> Joel> Thanks for adding the comment describing the function. Can you add an
> Joel> extra empty line before the function declaration?
> 
> Sure.  I was not sure which style is the preferred one -- though I see
> now that this is in gdbint.texinfo.  That said, both styles are common
> in gdb, and in macroexp.c in particular, the blank line only appears
> in one case.

Yes, that's true. I am not too fussy about this as it doesn't make
a whole lot of difference to me. But I try to correct them when I see
them.

> >> +	  /* Just ignore a stray token splicing operator.  Really this
> >> +	     is an error, but there's no reason to penalize the
> >> +	     user.  */
> 
> Joel> I don't see how this would be penalizing the user.  when could
> Joel> this error actually happen?
> 
> Some versions of gcc will pass a macro like this through.
> Trunk rejects code that tries to do this.  By 'this' I mean error
> cases like having '##' at the start or end of the macro definition.
> 
> So, it can only happen in very weird circumstances.
> 
> I can make it an error.  I doubt anybody will notice.
> I'll resubmit the patch a bit later.

I will let you decide what makes the most sense. If you think that
it makes better sense to accept it, then I'll go with that. GNU tools
in general try to be very open in what they accept so your approach
is reasonable. Perhaps a warning explaining that the splicing operator
has been ignored? (just some thoughts)

-- 
Joel

Re: RFA: implement all missing macro expansion features

[Tom Tromey]

Joel> I will let you decide what makes the most sense. 

I don't really care, so I made it an error.

In the interest of full disclosure, there is another pedantic error
case that this code does not handle: it is invalid to attempt to paste
two things which do not form a token.  I did not implement checking
for this -- but perhaps you would want that?

I think this patch addresses all your concerns.  I also incorporated a
documentation update, which was sitting unsubmitted in my patch queue.

Tom

2008-09-20  Tom Tromey  <tromey@redhat.com>

	* macrocmd.c (extract_identifier): Add is_parameter argument.
	(macro_define_command): Update.
	(macro_undef_command): Likewise.
	* macroexp.c (stringify): New function.
	(find_parameter): Likewise.
	(gather_arguments): Add nargs argument.  Handle varargs.
	(substitute_args): Add is_varargs and va_arg_name arguments.
	Handle varargs, splicing, stringification.  Use find_parameter.
	(expand): Handle varargs.

2008-09-20  Tom Tromey  <tromey@redhat.com>

	* gdb.texinfo (Macros): Remove text about stringification,
	varargs, and splicing.

2008-09-20  Tom Tromey  <tromey@redhat.com>

	* gdb.base/macscp.exp: Add tests for stringification, splicing,
	and varargs.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4a36fec..d8a87ef 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8086,10 +8086,6 @@ uses the macros in scope at that frame's source code line.  Otherwise,
 @value{GDBN} uses the macros in scope at the current listing location;
 see @ref{List}.
 
-At the moment, @value{GDBN} does not support the @code{##}
-token-splicing operator, the @code{#} stringification operator, or
-variable-arity macros.
-
 Whenever @value{GDBN} evaluates an expression, it always expands any
 macro invocations present in the expression.  @value{GDBN} also provides
 the following commands for working with macros explicitly.
diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index 8213c0d..c9ab440 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -197,18 +197,37 @@ skip_ws (char **expp)
     ++*expp;
 }
 
+/* Try to find the bounds of an identifier.  If an identifier is
+   found, returns a newly allocated string; otherwise returns NULL.
+   EXPP is a pointer to an input string; it is updated to point to the
+   text following the identifier.  If IS_PARAMETER is true, this
+   function will also allow "..." forms as used in varargs macro
+   parameters.  */
+
 static char *
-extract_identifier (char **expp)
+extract_identifier (char **expp, int is_parameter)
 {
   char *result;
   char *p = *expp;
   unsigned int len;
-  if (! *p || ! macro_is_identifier_nondigit (*p))
-    return NULL;
-  for (++p;
-       *p && (macro_is_identifier_nondigit (*p) || macro_is_digit (*p));
-       ++p)
-    ;
+
+  if (is_parameter && !strncmp (p, "...", 3))
+    {
+      /* Ok.  */
+    }
+  else
+    {
+      if (! *p || ! macro_is_identifier_nondigit (*p))
+	return NULL;
+      for (++p;
+	   *p && (macro_is_identifier_nondigit (*p) || macro_is_digit (*p));
+	   ++p)
+	;
+    }
+
+  if (is_parameter && !strncmp (p, "...", 3))      
+    p += 3;
+
   len = p - *expp;
   result = (char *) xmalloc (len + 1);
   memcpy (result, *expp, len);
@@ -246,7 +265,7 @@ macro_define_command (char *exp, int from_tty)
   memset (&new_macro, 0, sizeof (struct macro_definition));
 
   skip_ws (&exp);
-  name = extract_identifier (&exp);
+  name = extract_identifier (&exp, 0);
   if (! name)
     error (_("Invalid macro name."));
   if (*exp == '(')
@@ -274,7 +293,7 @@ macro_define_command (char *exp, int from_tty)
 	      /* Must update new_macro as well... */
 	      new_macro.argv = (const char * const *) argv;
 	    }
-	  argv[new_macro.argc] = extract_identifier (&exp);
+	  argv[new_macro.argc] = extract_identifier (&exp, 1);
 	  if (! argv[new_macro.argc])
 	    error (_("Macro is missing an argument."));
 	  ++new_macro.argc;
@@ -317,7 +336,7 @@ macro_undef_command (char *exp, int from_tty)
     error (_("usage: macro undef NAME"));
 
   skip_ws (&exp);
-  name = extract_identifier (&exp);
+  name = extract_identifier (&exp, 0);
   if (! name)
     error (_("Invalid macro name."));
   macro_undef (macro_main (macro_user_macros), -1, name);
diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index 8102bc0..7fb23ce 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -625,6 +625,52 @@ append_tokens_without_splicing (struct macro_buffer *dest,
                   _("unable to avoid splicing tokens during macro expansion"));
 }
 
+/* Stringify an argument, and insert it into DEST.  ARG is the text to
+   stringify; it is LEN bytes long.  */
+
+static void
+stringify (struct macro_buffer *dest, char *arg, int len)
+{
+  /* Trim initial whitespace from ARG.  */
+  while (len > 0 && macro_is_whitespace (*arg))
+    {
+      ++arg;
+      --len;
+    }
+
+  /* Trim trailing whitespace from ARG.  */
+  while (len > 0 && macro_is_whitespace (arg[len - 1]))
+    --len;
+
+  /* Insert the string.  */
+  appendc (dest, '"');
+  while (len > 0)
+    {
+      /* We could try to handle strange cases here, like control
+	 characters, but there doesn't seem to be much point.  */
+      if (macro_is_whitespace (*arg))
+	{
+	  /* Replace a sequence of whitespace with a single space.  */
+	  appendc (dest, ' ');
+	  while (len > 1 && macro_is_whitespace (arg[1]))
+	    {
+	      ++arg;
+	      --len;
+	    }
+	}
+      else if (*arg == '\\' || *arg == '"')
+	{
+	  appendc (dest, '\\');
+	  appendc (dest, *arg);
+	}
+      else
+	appendc (dest, *arg);
+      ++arg;
+      --len;
+    }
+  appendc (dest, '"');
+  dest->last_token = dest->len;
+}
 
 \f
 /* Expanding macros!  */
@@ -674,6 +720,11 @@ currently_rescanning (struct macro_name_list *list, const char *name)
 
    If SRC doesn't contain a properly terminated argument list, then
    raise an error.
+   
+   For a variadic macro, NARGS holds the number of formal arguments to
+   the macro.  For a GNU-style variadic macro, this should be the
+   number of named arguments.  For a non-variadic macro, NARGS should
+   be -1.
 
    Otherwise, return a pointer to the first element of an array of
    macro buffers referring to the argument texts, and set *ARGC_P to
@@ -694,7 +745,8 @@ currently_rescanning (struct macro_name_list *list, const char *name)
    following the invocation.  */
 
 static struct macro_buffer *
-gather_arguments (const char *name, struct macro_buffer *src, int *argc_p)
+gather_arguments (const char *name, struct macro_buffer *src,
+		  int nargs, int *argc_p)
 {
   struct macro_buffer tok;
   int args_len, args_size;
@@ -760,6 +812,20 @@ gather_arguments (const char *name, struct macro_buffer *src, int *argc_p)
                  the end of the argument list.  */
               if (depth == 0)
                 {
+		  /* In the varargs case, the last argument may be
+		     missing.  Add an empty argument in this case.  */
+		  if (nargs != -1 && args_len == nargs - 1)
+		    {
+		      /* Make sure we have room for the argument.  */
+		      if (args_len >= args_size)
+			{
+			  args_size++;
+			  args = xrealloc (args, sizeof (*args) * args_size);
+			}
+		      arg = &args[args_len++];
+		      set_token (arg, src->text, src->text);
+		    }
+
                   discard_cleanups (back_to);
                   *argc_p = args_len;
                   return args;
@@ -769,8 +835,11 @@ gather_arguments (const char *name, struct macro_buffer *src, int *argc_p)
             }
 
           /* If tok is a comma at top level, then that's the end of
-             the current argument.  */
-          else if (tok.len == 1 && tok.text[0] == ',' && depth == 0)
+             the current argument.  However, if we are handling a
+             variadic macro and we are computing the last argument, we
+             want to include the comma and remaining tokens.  */
+          else if (tok.len == 1 && tok.text[0] == ',' && depth == 0
+		   && (nargs == -1 || args_len < nargs))
             break;
 
           /* Extend the current argument to enclose this token.  If
@@ -801,17 +870,57 @@ static void scan (struct macro_buffer *dest,
                   void *lookup_baton);
 
 
+/* A helper function for substitute_args.
+   
+   ARGV is a vector of all the arguments; ARGC is the number of
+   arguments.  IS_VARARGS is true if the macro being substituted is a
+   varargs macro; in this case VA_ARG_NAME is the name of the
+   "variable" argument.  VA_ARG_NAME is ignored if IS_VARARGS is
+   false.
+
+   If the token TOK is the name of a parameter, return the parameter's
+   index.  If TOK is not an argument, return -1.  */
+
+static int
+find_parameter (const struct macro_buffer *tok,
+		int is_varargs, const struct macro_buffer *va_arg_name,
+		int argc, const char * const *argv)
+{
+  int i;
+
+  if (! tok->is_identifier)
+    return -1;
+
+  for (i = 0; i < argc; ++i)
+    if (tok->len == strlen (argv[i]) && ! memcmp (tok->text, argv[i], tok->len))
+      return i;
+
+  if (is_varargs && tok->len == va_arg_name->len
+      && ! memcmp (tok->text, va_arg_name->text, tok->len))
+    return argc - 1;
+
+  return -1;
+}
+ 
 /* Given the macro definition DEF, being invoked with the actual
    arguments given by ARGC and ARGV, substitute the arguments into the
    replacement list, and store the result in DEST.
 
+   IS_VARARGS should be true if DEF is a varargs macro.  In this case,
+   VA_ARG_NAME should be the name of the "variable" argument -- either
+   __VA_ARGS__ for c99-style varargs, or the final argument name, for
+   GNU-style varargs.  If IS_VARARGS is false, this parameter is
+   ignored.
+
    If it is necessary to expand macro invocations in one of the
    arguments, use LOOKUP_FUNC and LOOKUP_BATON to find the macro
    definitions, and don't expand invocations of the macros listed in
    NO_LOOP.  */
+
 static void
 substitute_args (struct macro_buffer *dest, 
                  struct macro_definition *def,
+		 int is_varargs, const struct macro_buffer *va_arg_name,
                  int argc, struct macro_buffer *argv,
                  struct macro_name_list *no_loop,
                  macro_lookup_ftype *lookup_func,
@@ -819,6 +928,17 @@ substitute_args (struct macro_buffer *dest,
 {
   /* A macro buffer for the macro's replacement list.  */
   struct macro_buffer replacement_list;
+  /* The token we are currently considering.  */
+  struct macro_buffer tok;
+  /* The replacement list's pointer from just before TOK was lexed.  */
+  char *original_rl_start;
+  /* We have a single lookahead token to handle token splicing.  */
+  struct macro_buffer lookahead;
+  /* The lookahead token might not be valid.  */
+  int lookahead_valid;
+  /* The replacement list's pointer from just before LOOKAHEAD was
+     lexed.  */
+  char *lookahead_rl_start;
 
   init_shared_buffer (&replacement_list, (char *) def->replacement,
                       strlen (def->replacement));
@@ -826,16 +946,14 @@ substitute_args (struct macro_buffer *dest,
   gdb_assert (dest->len == 0);
   dest->last_token = 0;
 
+  original_rl_start = replacement_list.text;
+  if (! get_token (&tok, &replacement_list))
+    return;
+  lookahead_rl_start = replacement_list.text;
+  lookahead_valid = get_token (&lookahead, &replacement_list);
+
   for (;;)
     {
-      struct macro_buffer tok;
-      char *original_rl_start = replacement_list.text;
-      int substituted = 0;
-      
-      /* Find the next token in the replacement list.  */
-      if (! get_token (&tok, &replacement_list))
-        break;
-
       /* Just for aesthetics.  If we skipped some whitespace, copy
          that to DEST.  */
       if (tok.text > original_rl_start)
@@ -847,46 +965,161 @@ substitute_args (struct macro_buffer *dest,
       /* Is this token the stringification operator?  */
       if (tok.len == 1
           && tok.text[0] == '#')
-        error (_("Stringification is not implemented yet."));
+	{
+	  int arg;
 
-      /* Is this token the splicing operator?  */
-      if (tok.len == 2
-          && tok.text[0] == '#'
-          && tok.text[1] == '#')
-        error (_("Token splicing is not implemented yet."));
+	  if (!lookahead_valid)
+	    error (_("Stringification operator requires an argument."));
 
-      /* Is this token an identifier?  */
-      if (tok.is_identifier)
-        {
-          int i;
-
-          /* Is it the magic varargs parameter?  */
-          if (tok.len == 11
-              && ! memcmp (tok.text, "__VA_ARGS__", 11))
-            error (_("Variable-arity macros not implemented yet."));
-
-          /* Is it one of the parameters?  */
-          for (i = 0; i < def->argc; i++)
-            if (tok.len == strlen (def->argv[i])
-                && ! memcmp (tok.text, def->argv[i], tok.len))
-              {
-                struct macro_buffer arg_src;
-
-                /* Expand any macro invocations in the argument text,
-                   and append the result to dest.  Remember that scan
-                   mutates its source, so we need to scan a new buffer
-                   referring to the argument's text, not the argument
-                   itself.  */
-                init_shared_buffer (&arg_src, argv[i].text, argv[i].len);
-                scan (dest, &arg_src, no_loop, lookup_func, lookup_baton);
-                substituted = 1;
-                break;
-              }
-        }
+	  arg = find_parameter (&lookahead, is_varargs, va_arg_name,
+				def->argc, def->argv);
+	  if (arg == -1)
+	    error (_("Argument to stringification operator must name "
+		     "a macro parameter."));
 
-      /* If it wasn't a parameter, then just copy it across.  */
-      if (! substituted)
-        append_tokens_without_splicing (dest, &tok);
+	  stringify (dest, argv[arg].text, argv[arg].len);
+
+	  /* Read one token and let the loop iteration code handle the
+	     rest.  */
+	  lookahead_rl_start = replacement_list.text;
+	  lookahead_valid = get_token (&lookahead, &replacement_list);
+	}
+      /* Is this token the splicing operator?  */
+      else if (tok.len == 2
+	       && tok.text[0] == '#'
+	       && tok.text[1] == '#')
+	error (_("Stray splicing operator"));
+      /* Is the next token the splicing operator?  */
+      else if (lookahead_valid
+	       && lookahead.len == 2
+	       && lookahead.text[0] == '#'
+	       && lookahead.text[1] == '#')
+	{
+	  int arg, finished = 0;
+	  int prev_was_comma = 0;
+
+	  /* Note that GCC warns if the result of splicing is not a
+	     token.  In the debugger there doesn't seem to be much
+	     benefit from doing this.  */
+
+	  /* Insert the first token.  */
+	  if (tok.len == 1 && tok.text[0] == ',')
+	    prev_was_comma = 1;
+	  else
+	    {
+	      int arg = find_parameter (&tok, is_varargs, va_arg_name,
+					def->argc, def->argv);
+	      if (arg != -1)
+		appendmem (dest, argv[arg].text, argv[arg].len);
+	      else
+		appendmem (dest, tok.text, tok.len);
+	    }
+
+	  /* Apply a possible sequence of ## operators.  */
+	  for (;;)
+	    {
+	      if (! get_token (&tok, &replacement_list))
+		error (_("Splicing operator at end of macro"));
+
+	      /* Handle a comma before a ##.  If we are handling
+		 varargs, and the token on the right hand side is the
+		 varargs marker, and the final argument is empty or
+		 missing, then drop the comma.  This is a GNU
+		 extension.  There is one ambiguous case here,
+		 involving pedantic behavior with an empty argument,
+		 but we settle that in favor of GNU-style (GCC uses an
+		 option).  If we aren't dealing with varargs, we
+		 simply insert the comma.  */
+	      if (prev_was_comma)
+		{
+		  if (! (is_varargs
+			 && tok.len == va_arg_name->len
+			 && !memcmp (tok.text, va_arg_name->text, tok.len)
+			 && argv[argc - 1].len == 0))
+		    appendmem (dest, ",", 1);
+		  prev_was_comma = 0;
+		}
+
+	      /* Insert the token.  If it is a parameter, insert the
+		 argument.  If it is a comma, treat it specially.  */
+	      if (tok.len == 1 && tok.text[0] == ',')
+		prev_was_comma = 1;
+	      else
+		{
+		  int arg = find_parameter (&tok, is_varargs, va_arg_name,
+					    def->argc, def->argv);
+		  if (arg != -1)
+		    appendmem (dest, argv[arg].text, argv[arg].len);
+		  else
+		    appendmem (dest, tok.text, tok.len);
+		}
+
+	      /* Now read another token.  If it is another splice, we
+		 loop.  */
+	      original_rl_start = replacement_list.text;
+	      if (! get_token (&tok, &replacement_list))
+		{
+		  finished = 1;
+		  break;
+		}
+
+	      if (! (tok.len == 2
+		     && tok.text[0] == '#'
+		     && tok.text[1] == '#'))
+		break;
+	    }
+
+	  if (prev_was_comma)
+	    {
+	      /* We saw a comma.  Insert it now.  */
+	      appendmem (dest, ",", 1);
+	    }
+
+          dest->last_token = dest->len;
+	  if (finished)
+	    lookahead_valid = 0;
+	  else
+	    {
+	      /* Set up for the loop iterator.  */
+	      lookahead = tok;
+	      lookahead_rl_start = original_rl_start;
+	      lookahead_valid = 1;
+	    }
+	}
+      else
+	{
+	  /* Is this token an identifier?  */
+	  int substituted = 0;
+	  int arg = find_parameter (&tok, is_varargs, va_arg_name,
+				    def->argc, def->argv);
+
+	  if (arg != -1)
+	    {
+	      struct macro_buffer arg_src;
+
+	      /* Expand any macro invocations in the argument text,
+		 and append the result to dest.  Remember that scan
+		 mutates its source, so we need to scan a new buffer
+		 referring to the argument's text, not the argument
+		 itself.  */
+	      init_shared_buffer (&arg_src, argv[arg].text, argv[arg].len);
+	      scan (dest, &arg_src, no_loop, lookup_func, lookup_baton);
+	      substituted = 1;
+	    }
+
+	  /* If it wasn't a parameter, then just copy it across.  */
+	  if (! substituted)
+	    append_tokens_without_splicing (dest, &tok);
+	}
+
+      if (! lookahead_valid)
+	break;
+
+      tok = lookahead;
+      original_rl_start = lookahead_rl_start;
+
+      lookahead_rl_start = replacement_list.text;
+      lookahead_valid = get_token (&lookahead, &replacement_list);
     }
 }
 
@@ -937,13 +1170,39 @@ expand (const char *id,
       struct macro_buffer *argv = NULL;
       struct macro_buffer substituted;
       struct macro_buffer substituted_src;
-
-      if (def->argc >= 1
-          && strcmp (def->argv[def->argc - 1], "...") == 0)
-        error (_("Varargs macros not implemented yet."));
+      struct macro_buffer va_arg_name;
+      int is_varargs = 0;
+
+      if (def->argc >= 1)
+	{
+	  if (strcmp (def->argv[def->argc - 1], "...") == 0)
+	    {
+	      /* In C99-style varargs, substitution is done using
+		 __VA_ARGS__.  */
+	      init_shared_buffer (&va_arg_name, "__VA_ARGS__",
+				  strlen ("__VA_ARGS__"));
+	      is_varargs = 1;
+	    }
+	  else
+	    {
+	      int len = strlen (def->argv[def->argc - 1]);
+	      if (len > 3
+		  && strcmp (def->argv[def->argc - 1] + len - 3, "...") == 0)
+		{
+		  /* In GNU-style varargs, the name of the
+		     substitution parameter is the name of the formal
+		     argument without the "...".  */
+		  init_shared_buffer (&va_arg_name,
+				      (char *) def->argv[def->argc - 1],
+				      len - 3);
+		  is_varargs = 1;
+		}
+	    }
+	}
 
       make_cleanup (free_current_contents, &argv);
-      argv = gather_arguments (id, src, &argc);
+      argv = gather_arguments (id, src, is_varargs ? def->argc : -1,
+			       &argc);
 
       /* If we couldn't find any argument list, then we don't expand
          this macro.  */
@@ -957,12 +1216,16 @@ expand (const char *id,
          this macro.  */
       if (argc != def->argc)
         {
+	  if (is_varargs && argc >= def->argc - 1)
+	    {
+	      /* Ok.  */
+	    }
           /* Remember that a sequence of tokens like "foo()" is a
              valid invocation of a macro expecting either zero or one
              arguments.  */
-          if (! (argc == 1
-                 && argv[0].len == 0
-                 && def->argc == 0))
+          else if (! (argc == 1
+		      && argv[0].len == 0
+		      && def->argc == 0))
             error (_("Wrong number of arguments to macro `%s' "
                    "(expected %d, got %d)."),
                    id, def->argc, argc);
@@ -976,8 +1239,8 @@ expand (const char *id,
          expand an argument until we see how it's being used.  */
       init_buffer (&substituted, 0);
       make_cleanup (cleanup_macro_buffer, &substituted);
-      substitute_args (&substituted, def, argc, argv, no_loop,
-                       lookup_func, lookup_baton);
+      substitute_args (&substituted, def, is_varargs, &va_arg_name,
+		       argc, argv, no_loop, lookup_func, lookup_baton);
 
       /* Now `substituted' is the macro's replacement list, with all
          argument values substituted into it properly.  Re-scan it for
diff --git a/gdb/testsuite/gdb.base/macscp.exp b/gdb/testsuite/gdb.base/macscp.exp
index 3424714..d9fd97c 100644
--- a/gdb/testsuite/gdb.base/macscp.exp
+++ b/gdb/testsuite/gdb.base/macscp.exp
@@ -480,7 +480,100 @@ gdb_test "macro undef" \
     "usage: macro undef.*" \
     "macro undef with no arguments"
 
-# Regression test; this used to emit the wrong error.
+# Splicing tests.
+
 gdb_test "macro expand SPLICE(x, y)" \
-  "Token splicing is not implemented yet." \
-  "macro splicing lexes correctly"
+  "expands to: xy" \
+  "basic macro splicing"
+
+gdb_test "macro define robotinvasion 2010" \
+  "" \
+  "define splice helper"
+
+gdb_test "macro expand SPLICE(robot, invasion)" \
+  "expands to: *2010" \
+  "splicing plus expansion"
+
+# Varargs tests.
+
+gdb_test "macro define va_c99(...) fprintf (stderr, __VA_ARGS__)" \
+  "" \
+  "define first varargs helper"
+
+gdb_test "macro define va2_c99(x, y, ...) fprintf (stderr, x, y, __VA_ARGS__)" \
+  "" \
+  "define second varargs helper"
+
+gdb_test "macro define va_gnu(args...) fprintf (stderr, args)" \
+  "" \
+  "define third varargs helper"
+
+gdb_test "macro define va2_gnu(args...) fprintf (stderr, ## args)" \
+  "" \
+  "define fourth varargs helper"
+
+gdb_test "macro expand va_c99(one, two, three)" \
+  "expands to: *fprintf \\(stderr, *one, two, three\\)" \
+  "c99 varargs expansion"
+
+gdb_test "macro expand va_c99()" \
+  "expands to: *fprintf \\(stderr, *\\)" \
+  "c99 varargs expansion without an argument"
+
+gdb_test "macro expand va2_c99(one, two, three, four)" \
+  "expands to: *fprintf \\(stderr, *one, two, three, four\\)" \
+  "c99 varargs expansion, multiple formal arguments"
+
+gdb_test "macro expand va_gnu(one, two, three, four)" \
+  "expands to: *fprintf \\(stderr, *one, two, three, four\\)" \
+  "gnu varargs expansion"
+
+gdb_test "macro expand va_gnu()" \
+  "expands to: *fprintf \\(stderr, *\\)" \
+  "gnu varargs expansion without an argument"
+
+gdb_test "macro expand va2_gnu()" \
+  "expands to: *fprintf \\(stderr\\)" \
+  "gnu varargs expansion special splicing without an argument"
+
+# Stringification tests.
+
+gdb_test "macro define str(x) #x" \
+  "" \
+  "define stringification macro"
+
+gdb_test "macro define maude 5" \
+  "" \
+  "define first stringification helper"
+
+gdb_test "macro define xstr(x) str(x)" \
+  "" \
+  "define second stringification helper"
+
+gdb_test "print str(5)" \
+  " = \"5\"" \
+  "simple stringify"
+
+gdb_test "print str(hi bob)" \
+  " = \"hi bob\"" \
+  "stringify with one space"
+
+gdb_test "print str(  hi  bob  )" \
+  " = \"hi bob\"" \
+  "stringify with many spaces"
+
+gdb_test "print str(hi \"bob\")" \
+  " = \"hi \\\\\"bob\\\\\"\"" \
+  "stringify with quotes"
+
+gdb_test "print str(hi \\bob\\)" \
+  " = \"hi \\\\\\\\bob\\\\\\\\\"" \
+  "stringify with backslashes"
+
+gdb_test "print str(maude)" \
+  " = \"maude\"" \
+  "stringify without substitution"
+
+gdb_test "print xstr(maude)" \
+  " = \"5\"" \
+  "stringify with substitution"

Re: RFA: implement all missing macro expansion features

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Tom Tromey <tromey@redhat.com>
> Date: Sat, 20 Sep 2008 15:54:51 -0600
> 
> In the interest of full disclosure, there is another pedantic error
> case that this code does not handle: it is invalid to attempt to paste
> two things which do not form a token.  I did not implement checking
> for this -- but perhaps you would want that?
> 
> I think this patch addresses all your concerns.  I also incorporated a
> documentation update, which was sitting unsubmitted in my patch queue.

Thanks.  I think we also need a NEWS entry.

Re: RFA: implement all missing macro expansion features

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Thanks.  I think we also need a NEWS entry.

Please review.

I added this to the overall patch.  I can resend the entire thing if
necessary.

Tom

2008-09-20  Tom Tromey  <tromey@redhat.com>

	* NEWS: Update.

diff --git a/gdb/NEWS b/gdb/NEWS
index 4c9af82..a6c8adc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -22,6 +22,10 @@ For instance, consider:
 If the user types TAB at the end of this command line, the available
 completions will be "f1" and "f2".
 
+* GDB now supports the token-splicing (##) and stringification (#)
+operators when expanding macros.  It also now supports variable-arity
+macros.
+
 * New remote packets
 
 qSearch:memory:

Re: RFA: implement all missing macro expansion features

[Joel Brobecker]

> I think this patch addresses all your concerns.

Agreed.

> 2008-09-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* macrocmd.c (extract_identifier): Add is_parameter argument.
> 	(macro_define_command): Update.
> 	(macro_undef_command): Likewise.
> 	* macroexp.c (stringify): New function.
> 	(find_parameter): Likewise.
> 	(gather_arguments): Add nargs argument.  Handle varargs.
> 	(substitute_args): Add is_varargs and va_arg_name arguments.
> 	Handle varargs, splicing, stringification.  Use find_parameter.
> 	(expand): Handle varargs.

> 2008-09-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/macscp.exp: Add tests for stringification, splicing,
> 	and varargs.

Please wait another couple of days to give others a final chance
to provide their feedback, and then commit.

-- 
Joel

Re: RFA: implement all missing macro expansion features

[Eli Zaretskii]

> Cc: brobecker@adacore.com, gdb-patches@sourceware.org
> From: Tom Tromey <tromey@redhat.com>
> Date: Sat, 20 Sep 2008 21:23:39 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Thanks.  I think we also need a NEWS entry.
> 
> Please review.

Thanks.

> +* GDB now supports the token-splicing (##) and stringification (#)
> +operators when expanding macros.  It also now supports variable-arity
> +macros.                                   ^^^

Think this "now" should be removed.  The previous sentence already
says "now".

Otherwise, this is fine.

Re: RFA: implement all missing macro expansion features

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Please wait another couple of days to give others a final chance
Joel> to provide their feedback, and then commit.

It has been a week, so I am going to check it in.

Tom