tracing through indir

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Now that I'm one of the autoconf maintainers, I decided to look through
the TODO file.  I found a complaint about this m4 bug:

$ echo 'changequote([,])foo
indir([foo])' | m4 -t foo -Dfoo=bar
m4trace: -1- foo
bar
bar

In other words, since autoconf depends so heavily on tracing, it is unable
to use indir reliably without losing trace information, and some
constructs that would be nice with indir have had to be rewritten as
direct calls.  Furthermore, autoconf also uses internal macros with names
such as `AC_LANG_SOURCE(C)', intentionally named so that only indir can
invoke them, but debugging these macros is difficult since such macros are
currently untraceable.  Therefore, I think this is probably worth fixing
to some degree on the branch (as the fix is backwards compatible - the
only difference is that it adds new trace output where before nothing was
done).  Here's a patch I've been playing with for the branch; it lacks
documentation, and would also need to be ported to HEAD, before I could
apply it, but I wanted to get comments for now.  Once applied, the above
example becomes:

$ echo 'changequote([,])foo
indir([foo])' | ~/m4/build/src/m4 -t foo -Dfoo=bar
m4trace: -1- foo
bar
m4trace: -1- foo
bar

Or more verbosely,

$ echo 'changequote([,])foo
indir([foo])' | ~/m4/build/src/m4 -t foo -Dfoo=bar -dV
m4debug: input read from stdin
m4trace:stdin:1: -1- id 1: changequote ...
m4trace:stdin:1: -1- id 1: changequote(`[', `]') -> ???
m4trace:stdin:1: -1- id 1: changequote(...)
m4trace:stdin:1: -1- id 2: foo ...
m4trace:stdin:1: -1- id 2: foo -> ???
m4trace:stdin:1: -1- id 2: foo -> [bar]
bar
m4trace:stdin:2: -1- id 3: indir ...
m4trace:stdin:2: -1- id 3: indir([foo]) -> ???
m4trace:stdin:2: -1- id 3: foo ...
m4trace:stdin:2: -1- id 3: foo -> ???
m4trace:stdin:2: -1- id 3: foo(...) -> [bar]
m4trace:stdin:2: -1- id 3: indir(...) -> [bar]
bar
m4debug:stdin:3: input exhausted


Note that tracing the contents of builtin() presents a bit of a challenge
- - whereas indir(`divnum') can check whether traceon(`divnum') has been
called, builtin(`divnum') cannot assume that a macro named `divnum'
currently exists, so in the proposed patch, builtin() can only trace its
target if global tracing is enabled.  I was thinking for this case, on
HEAD only, that we make traceon(defn(`divnum')) a special case -
currently, this syntax flattens all builtin tokens to the empty string,
and results in tracing the user macro named `', but I am proposing making
it attach a trace attribute to the builtin itself, so that
builtin(`divnum') outputs a trace.  The question then is if the user did
traceon(defn(`divnum')), should divnum in isolation or indir(`divnum') be
traced, since the trace attribute was not attached to the `divnum' name
but a traced builtin was invoked, or should only builtin(`divnum') be
traced?  Also, this special-casing of traceon cannot be done via the
command-line option '--trace/-t'; I wonder if we would need to add
something like '--trace-builtin' (no short option).

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG3VvY84KuGfSFAYARAmytAJ9w7dWV1FRwtYmmdUAJEvTg9EiI3wCgjiwB
1TmbH9zZHrpHGpuWVaD/oU4=
=qY/Q
-----END PGP SIGNATURE-----

Index: src/builtin.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/builtin.c,v
retrieving revision 1.1.1.1.2.62
diff -u -p -r1.1.1.1.2.62 builtin.c
--- src/builtin.c       9 Aug 2007 13:39:10 -0000       1.1.1.1.2.62
+++ src/builtin.c       31 Aug 2007 23:41:11 -0000
@@ -827,6 +827,8 @@ m4_builtin (struct obstack *obs, int arg
   else
     {
       int i;
+      bool traced = debug_level & DEBUG_TRACE_ALL;
+      unsigned int trace_start = 0;
       if (! bp->groks_macro_args)
        for (i = 2; i < argc; i++)
          if (TOKEN_DATA_TYPE (argv[i]) != TOKEN_TEXT)
@@ -834,7 +836,16 @@ m4_builtin (struct obstack *obs, int arg
              TOKEN_DATA_TYPE (argv[i]) = TOKEN_TEXT;
              TOKEN_DATA_TEXT (argv[i]) = (char *) "";
            }
+      if (traced)
+       {
+         if (debug_level & DEBUG_TRACE_CALL)
+           trace_prepre (name, macro_call_id);
+         trace_start = trace_pre (name, macro_call_id, argc - 1, argv + 1);
+       }
       bp->func (obs, argc - 1, argv + 1);
+      if (traced)
+       trace_post (name, macro_call_id, argc + 1, argv - 1,
+                   push_string_peek (), trace_start);
     }
 }
 
@@ -868,6 +879,8 @@ m4_indir (struct obstack *obs, int argc,
   else
     {
       int i;
+      bool traced = (debug_level & DEBUG_TRACE_ALL) || SYMBOL_TRACED (s);
+      unsigned int trace_start = 0;
       if (! SYMBOL_MACRO_ARGS (s))
        for (i = 2; i < argc; i++)
          if (TOKEN_DATA_TYPE (argv[i]) != TOKEN_TEXT)
@@ -875,7 +888,16 @@ m4_indir (struct obstack *obs, int argc,
              TOKEN_DATA_TYPE (argv[i]) = TOKEN_TEXT;
              TOKEN_DATA_TEXT (argv[i]) = (char *) "";
            }
+      if (traced)
+       {
+         if (debug_level & DEBUG_TRACE_CALL)
+           trace_prepre (name, macro_call_id);
+         trace_start = trace_pre (name, macro_call_id, argc - 1, argv + 1);
+       }
       call_macro (s, argc - 1, argv + 1, obs);
+      if (traced)
+       trace_post (name, macro_call_id, argc + 1, argv - 1,
+                   push_string_peek (), trace_start);
     }
 }
 
Index: src/debug.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/debug.c,v
retrieving revision 1.1.1.1.2.15
diff -u -p -r1.1.1.1.2.15 debug.c
--- src/debug.c 4 Aug 2007 20:40:11 -0000       1.1.1.1.2.15
+++ src/debug.c 31 Aug 2007 23:41:11 -0000
@@ -30,8 +30,6 @@ FILE *debug = NULL;
 /* Obstack for trace messages.  */
 static struct obstack trace;
 
-extern int expansion_level;
-
 static void debug_set_file (FILE *);
 
 /*----------------------------------.
@@ -323,9 +321,10 @@ trace_format (const char *fmt, ...)
 | Format the standard header attached to all tracing output lines.  |
 `------------------------------------------------------------------*/
 
-static void
+static unsigned int
 trace_header (int id)
 {
+  unsigned int result = obstack_object_size (&trace);
   trace_format ("m4trace:");
   if (current_line)
     {
@@ -337,6 +336,7 @@ trace_header (int id)
   trace_format (" -%d- ", expansion_level);
   if (debug_level & DEBUG_TRACE_CALLID)
     trace_format ("id %d: ", id);
+  return result;
 }
 
 /*----------------------------------------------------.
@@ -344,41 +344,43 @@ trace_header (int id)
 `----------------------------------------------------*/
 
 static void
-trace_flush (void)
+trace_flush (unsigned int start)
 {
   char *line;
 
   obstack_1grow (&trace, '\0');
-  line = (char *) obstack_finish (&trace);
-  DEBUG_PRINT1 ("%s\n", line);
-  obstack_free (&trace, line);
+  line = (char *) obstack_base (&trace);
+  DEBUG_PRINT1 ("%s\n", &line[start]);
+  obstack_blank (&trace, start - obstack_object_size (&trace));
 }
 
-/*-------------------------------------------------------------.
-| Do pre-argument-collction tracing for macro NAME.  Used from |
-| expand_macro ().                                            |
-`-------------------------------------------------------------*/
+/*--------------------------------------------------------------.
+| Do pre-argument-collection tracing for macro NAME.  Used from |
+| expand_macro ().                                              |
+`--------------------------------------------------------------*/
 
 void
 trace_prepre (const char *name, int id)
 {
-  trace_header (id);
+  unsigned int start = trace_header (id);
   trace_format ("%s ...", name);
-  trace_flush ();
+  trace_flush (start);
 }
 
-/*-----------------------------------------------------------------------.
-| Format the parts of a trace line, that can be made before the macro is |
-| actually expanded.  Used from expand_macro ().                        |
-`-----------------------------------------------------------------------*/
+/*-----------------------------------------------------------------.
+| Format the parts of a trace line, that can be made before the    |
+| macro is actually expanded.  Used from expand_macro ().  Return  |
+| the start of the current trace, in case other traces are printed |
+| before this trace completes trace_post.                          |
+`-----------------------------------------------------------------*/
 
-void
+unsigned int
 trace_pre (const char *name, int id, int argc, token_data **argv)
 {
   int i;
   const builtin *bp;
 
-  trace_header (id);
+  unsigned int start = trace_header (id);
   trace_format ("%s", name);
 
   if (argc > 1 && (debug_level & DEBUG_TRACE_ARGS))
@@ -420,8 +422,9 @@ INTERNAL ERROR: builtin not found in bui
   if (debug_level & DEBUG_TRACE_CALL)
     {
       trace_format (" -> ???");
-      trace_flush ();
+      trace_flush (start);
     }
+  return start;
 }
 
 /*-------------------------------------------------------------------.
@@ -431,7 +434,7 @@ INTERNAL ERROR: builtin not found in bui
 
 void
 trace_post (const char *name, int id, int argc, token_data **argv,
-           const char *expanded)
+           const char *expanded, unsigned int start)
 {
   if (debug_level & DEBUG_TRACE_CALL)
     {
@@ -441,5 +444,5 @@ trace_post (const char *name, int id, in
 
   if (expanded && (debug_level & DEBUG_TRACE_EXPANSION))
     trace_format (" -> %l%S%r", expanded);
-  trace_flush ();
+  trace_flush (start);
 }
Index: src/input.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/input.c,v
retrieving revision 1.1.1.1.2.37
diff -u -p -r1.1.1.1.2.37 input.c
--- src/input.c 4 Aug 2007 20:40:12 -0000       1.1.1.1.2.37
+++ src/input.c 31 Aug 2007 23:41:11 -0000
@@ -257,6 +257,25 @@ push_string_init (void)
   return current_input;
 }
 
+/*-------------------------------------------------------------------.
+| Peek at the current push_string () status.  If next is now NULL, a |
+| call to push_file () has invalidated the previous call to          |
+| push_string_init (), so we just give up.  If the new object is     |
+| void, we do nothing.  Otherwise, we append a NUL byte and return   |
+| the start of the current unfinished storage; the byte can be       |
+| removed with obstack_blank (current_input, -1) if needed.          |
+`-------------------------------------------------------------------*/
+
+const char *
+push_string_peek (void)
+{
+  if (next == NULL || obstack_object_size (current_input) == 0)
+    return NULL;
+
+  obstack_1grow (current_input, '\0');
+  return (char *) obstack_base (current_input);
+}
+
 /*------------------------------------------------------------------------.
 | Last half of push_string ().  If next is now NULL, a call to push_file  |
 | () has invalidated the previous call to push_string_init (), so we just |
Index: src/m4.h
===================================================================
RCS file: /sources/m4/m4/src/m4.h,v
retrieving revision 1.1.1.1.2.45
diff -u -p -r1.1.1.1.2.45 m4.h
--- src/m4.h    4 Aug 2007 20:40:12 -0000       1.1.1.1.2.45
+++ src/m4.h    31 Aug 2007 23:41:11 -0000
@@ -229,8 +229,9 @@ bool debug_set_output (const char *);
 void debug_message_prefix (void);
 
 void trace_prepre (const char *, int);
-void trace_pre (const char *, int, int, token_data **);
-void trace_post (const char *, int, int, token_data **, const char *);
+unsigned int trace_pre (const char *, int, int, token_data **);
+void trace_post (const char *, int, int, token_data **, const char *,
+                 unsigned int);
 
 /* File: input.c  --- lexical definitions.  */
 
@@ -292,6 +293,7 @@ void skip_line (void);
 void push_file (FILE *, const char *, bool);
 void push_macro (builtin_func *);
 struct obstack *push_string_init (void);
+const char *push_string_peek (void);
 const char *push_string_finish (void);
 void push_wrapup (const char *);
 bool pop_wrapup (void);
@@ -382,6 +384,9 @@ void hack_all_symbols (hack_symbol *, vo
 
 /* File: macro.c  --- macro expansion.  */
 
+int expansion_level;
+int macro_call_id;
+
 void expand_input (void);
 void call_macro (symbol *, int, token_data **, struct obstack *);
 
Index: src/macro.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/macro.c,v
retrieving revision 1.1.1.1.2.20
diff -u -p -r1.1.1.1.2.20 macro.c
--- src/macro.c 4 Aug 2007 20:40:13 -0000       1.1.1.1.2.20
+++ src/macro.c 31 Aug 2007 23:41:11 -0000
@@ -31,7 +31,7 @@ static void expand_token (struct obstack
 int expansion_level = 0;
 
 /* The number of the current call of expand_macro ().  */
-static int macro_call_id = 0;
+int macro_call_id = 0;
 
 /* The shared stack of collected arguments for macro calls; as each
    argument is collected, it is finished and its location stored in
@@ -309,8 +309,14 @@ expand_macro (symbol *sym)
   int argc;
   struct obstack *expansion;
   const char *expanded;
+  /* Trace macros using the call id at the start of this expansion,
+     even if nested expansions are encountered during argument
+     collection.  Temporarily reset the call id so that indir and
+     builtin can use the same id.  */
   bool traced;
   int my_call_id;
+  int tmp_call_id;
+  unsigned int trace_start = 0;
 
   /* Report errors at the location where the open parenthesis (if any)
      was found, but after expansion, restore global state back to the
@@ -359,19 +365,23 @@ expand_macro (symbol *sym)
   loc_close_line = current_line;
   current_file = loc_open_file;
   current_line = loc_open_line;
+  tmp_call_id = macro_call_id;
+  macro_call_id = my_call_id;
 
   if (traced)
-    trace_pre (SYMBOL_NAME (sym), my_call_id, argc, argv);
+    trace_start = trace_pre (SYMBOL_NAME (sym), my_call_id, argc, argv);
 
   expansion = push_string_init ();
   call_macro (sym, argc, argv, expansion);
   expanded = push_string_finish ();
 
   if (traced)
-    trace_post (SYMBOL_NAME (sym), my_call_id, argc, argv, expanded);
+    trace_post (SYMBOL_NAME (sym), my_call_id, argc, argv, expanded,
+               trace_start);
 
   current_file = loc_close_file;
   current_line = loc_close_line;
+  macro_call_id = tmp_call_id;
 
   --expansion_level;
   --SYMBOL_PENDING_EXPANSIONS (sym);