corner cases with defn

[Eric Blake]

Stumbled on this when trying to clean up memory usage on my $@ ref work - we 
weren't handling the concatenation of builtins with text very consistently.  
Since it was previously undocumented, and very rare in practice, I don't feel 
too bad about the semantics change that I just introduced.

before:
define(foo,defn(`defn')`a') => foo: <defn>, no warning
define(foo,`a'defn(`defn')) => foo: a, no warning

after:
define(foo,defn(`defn')`a') => foo: a, plus a warning
define(foo,`a'defn(`defn')) => foo: a, plus a warning

Solaris:
define(foo,defn(`defn')`a') => foo: <defn>a, no warning
define(foo,`a'defn(`defn')) => foo: a<defn>, no warning

Besides, I still want to eventually reach the point in m4 2.0 where we can 
emulate Solaris m4 semantics of passing builtin tokens through user macros, as 
well as allowing the concatenation of text and builtins in a single macro 
definition (although such concatenation doesn't seem to be very useful, I also 
don't see anything in POSIX that forbids it).  Both the before and after 
semantics differ from Solaris, but at least the after semantics warn you about 
it.

From: Eric Blake <address@hidden>
Date: Wed, 14 Nov 2007 15:15:31 -0700
Subject: [PATCH] Avoid builtin concatenation in macro arguments.

* doc/m4.texinfo (Defn): Add tests.
* src/macro.c (warn_builtin_concat): New function.
(expand_argument): Use it to warn on more corner cases.
* NEWS: Mention new warning.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog      |    8 ++++++++
 NEWS           |    8 +++++---
 doc/m4.texinfo |   37 +++++++++++++++++++++++++++++--------
 src/macro.c    |   52 +++++++++++++++++++++++++++++++++++-----------------
 4 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 068148d..ae487a0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2007-11-14  Eric Blake  <address@hidden>
+
+       Avoid builtin concatenation in macro arguments.
+       * doc/m4.texinfo (Defn): Add tests.
+       * src/macro.c (warn_builtin_concat): New function.
+       (expand_argument): Use it to warn on more corner cases.
+       * NEWS: Mention new warning.
+
 2007-11-13  Eric Blake  <address@hidden>
 
        Simplify previous patch.
diff --git a/NEWS b/NEWS
index b92630b..d988adf 100644
--- a/NEWS
+++ b/NEWS
@@ -7,11 +7,13 @@ Version 1.4.11 - ?? ??? 2007, by ????  (git version 1.4.10a-*)
 * Fix core dump in 'm4 -F file -t undefined', present since -F was
   introduced in 1.3.
 * Fix regression introduced in 1.4.9b in the `divert' builtin when more
-  than 512 kibibytes are saved in diversions on platforms like NetBSD
-  where fopen(name,"a+") seeks to the end of the file.
+  than 512 kibibytes are saved in diversions on platforms like NetBSD where
+  fopen(name,"a+") seeks to the end of the file.
 * Enhance the `defn' builtin to support concatenation of multiple text
   arguments, as required by POSIX.  However, at this time, it is not
-  possible to concatenate a builtin macro with anything else.
+  possible to concatenate a builtin macro with anything else; a warning is
+  now issued if this is attempted, although a future version of M4 may lift
+  this restriction to match other implementations.
 * Several improvements in `index', `regexp', and `patsubst' builtins to
   speed up typical Autoconf usage.
 * Memory usage is greatly reduced in recursive macros.
diff --git a/doc/m4.texinfo b/doc/m4.texinfo
index f028e3d..6caa7c3 100644
--- a/doc/m4.texinfo
+++ b/doc/m4.texinfo
@@ -2077,9 +2077,9 @@ and it is a builtin, the
 expansion is a special token, which points to the builtin's internal
 definition.  This token is only meaningful as the second argument to
 @code{define} (and @code{pushdef}), and is silently converted to an
-empty string in most other contexts.  Combining a builtin with anything
-else is not supported; a warning is issued and the builtin is omitted
-from the final expansion.
+empty string in most other contexts.  Using multiple @var{name} to
+combine a builtin with anything else is not supported; a warning is
+issued and the builtin is omitted from the final expansion.
 
 The macro @code{defn} is recognized only with parameters.
 @end deffn
@@ -2186,18 +2186,39 @@ define(defn(`divnum'), `cannot redefine a builtin 
token')
 @result{}
 divnum
 @result{}0
+define(`echo', `$@@')
address@hidden
+define(`mydivnum', echo(defn(`divnum')))
address@hidden
+mydivnum
address@hidden
 @end example
 
 Also note that @code{defn} with multiple arguments can only join text
-macros, not builtins, although a future version of @acronym{GNU} M4 may
-lift this restriction.
+macros, not builtins.  Likewise, when collecting macro arguments, a
+builtin token is preserved only when it occurs in isolation.  A future
+version of @acronym{GNU} M4 may lift these restrictions.
 
 @example
-define(`a', `A')
+define(`a', `A')define(`AA', `b')
 @result{}
-defn(`a', `defn', `a')
address@hidden:stdin:2: Warning: cannot concatenate builtin `defn'
+defn(`a', `divnum', `a')
address@hidden:stdin:2: Warning: cannot concatenate builtin `divnum'
 @result{}AA
+define(`mydivnum', defn(`divnum', `divnum'))mydivnum
address@hidden:stdin:3: Warning: cannot concatenate builtin `divnum'
address@hidden:stdin:3: Warning: cannot concatenate builtin `divnum'
address@hidden
+define(`mydivnum', defn(`divnum')defn(`divnum'))mydivnum
address@hidden:stdin:4: Warning: cannot concatenate builtin `divnum'
address@hidden:stdin:4: Warning: cannot concatenate builtin `divnum'
address@hidden
+define(`mydivnum', defn(`divnum')`a')mydivnum
address@hidden:stdin:5: Warning: cannot concatenate builtin `divnum'
address@hidden
+define(`mydivnum', `a'defn(`divnum'))mydivnum
address@hidden:stdin:6: Warning: cannot concatenate builtin `divnum'
address@hidden
 @end example
 
 @node Pushdef
diff --git a/src/macro.c b/src/macro.c
index 76f9276..9a3123a 100644
--- a/src/macro.c
+++ b/src/macro.c
@@ -125,15 +125,28 @@ expand_token (struct obstack *obs, token_type t, 
token_data *td, int line)
 }
 
 
+/*---------------------------------------------------------------.
+| Helper function to print warning about concatenating FUNC with |
+| text.                                                          |
+`---------------------------------------------------------------*/
+static void
+warn_builtin_concat (builtin_func *func)
+{
+  const builtin *bp = find_builtin_by_addr (func);
+  assert (bp);
+  M4ERROR ((warning_status, 0, "Warning: cannot concatenate builtin `%s'",
+           bp->name));
+}
+
 /*-------------------------------------------------------------------.
 | This function parses one argument to a macro call.  It expects the |
 | first left parenthesis or the separating comma to have been read   |
-| by the caller.  It skips leading whitespace, then reads and       |
+| by the caller.  It skips leading whitespace, then reads and        |
 | expands tokens, until it finds a comma or right parenthesis at the |
 | same level of parentheses.  It returns a flag indicating whether   |
-| the argument read is the last for the active macro call.  The             |
-| argument is built on the obstack OBS, indirectly through          |
-| expand_token ().  Report errors on behalf of CALLER.              |
+| the argument read is the last for the active macro call.  The      |
+| argument is built on the obstack OBS, indirectly through           |
+| expand_token ().  Report errors on behalf of CALLER.               |
 `-------------------------------------------------------------------*/
 
 static bool
@@ -141,7 +154,6 @@ expand_argument (struct obstack *obs, token_data *argp, 
const char *caller)
 {
   token_type t;
   token_data td;
-  char *text;
   int paren_level;
   const char *file = current_file;
   int line = current_line;
@@ -166,25 +178,23 @@ expand_argument (struct obstack *obs, token_data *argp, 
const char *caller)
        case TOKEN_CLOSE:
          if (paren_level == 0)
            {
-             /* The argument MUST be finished, whether we want it or not.  */
-             obstack_1grow (obs, '\0');
-             text = (char *) obstack_finish (obs);
-
-             if (TOKEN_DATA_TYPE (argp) == TOKEN_VOID)
+             if (TOKEN_DATA_TYPE (argp) == TOKEN_FUNC)
                {
-                 TOKEN_DATA_TYPE (argp) = TOKEN_TEXT;
-                 TOKEN_DATA_TEXT (argp) = text;
+                 if (obstack_object_size (obs) == 0)
+                   return t == TOKEN_COMMA;
+                 warn_builtin_concat (TOKEN_DATA_FUNC (argp));
                }
+             obstack_1grow (obs, '\0');
+             TOKEN_DATA_TYPE (argp) = TOKEN_TEXT;
+             TOKEN_DATA_TEXT (argp) = (char *) obstack_finish (obs);
              return t == TOKEN_COMMA;
            }
          /* fallthru */
        case TOKEN_OPEN:
        case TOKEN_SIMPLE:
-         text = TOKEN_DATA_TEXT (&td);
-
-         if (*text == '(')
+         if (t == TOKEN_OPEN)
            paren_level++;
-         else if (*text == ')')
+         else if (t == TOKEN_CLOSE)
            paren_level--;
          expand_token (obs, t, &td, line);
          break;
@@ -202,11 +212,19 @@ expand_argument (struct obstack *obs, token_data *argp, 
const char *caller)
          break;
 
        case TOKEN_MACDEF:
-         if (obstack_object_size (obs) == 0)
+         if (TOKEN_DATA_TYPE (argp) == TOKEN_VOID
+             && obstack_object_size (obs) == 0)
            {
              TOKEN_DATA_TYPE (argp) = TOKEN_FUNC;
              TOKEN_DATA_FUNC (argp) = TOKEN_DATA_FUNC (&td);
            }
+         else
+           {
+             if (TOKEN_DATA_TYPE (argp) == TOKEN_FUNC)
+               warn_builtin_concat (TOKEN_DATA_FUNC (argp));
+             warn_builtin_concat (TOKEN_DATA_FUNC (&td));
+             TOKEN_DATA_TYPE (argp) = TOKEN_TEXT;
+           }
          break;
 
        default:
-- 
1.5.3.2