branch-1_4 off-by-one in line reporting

Eric Blake <[email protected]>
Newsgroups gmane.comp.gnu.m4.patches
Message-ID <[email protected]>
$ m4
`oops
^D
m4:stdin:1: ERROR: end of file in string
$ m4
dnl
`oops
^D
m4:stdin:1: ERROR: end of file in string

Oops - for every line except line 1, if an unterminated quote or comment began 
immediately after \n, the error message is off by one.

While I was at it, I decided that it would be nice for macro expansion warning 
messages to report the line where the macro name was encountered, rather than 
where the closing ) was encountered.  This applies when a macro name is 
encountered directly in a file and argument collection spans multiple lines.  
And this is one step closer to having m4wrap remember locations rather than 
resulting in a blank __file__.

2006-10-11  Eric Blake  <[email protected]>

	* src/macro.c (expand_macro): In macro expansion errors, report
	line number at open parenthesis.
	* src/input.c (next_token): Fix off-by-one bug in reporting end
	of file in unterminated comment and string.
	(file_names): New obstack, necessary since expand_macro now hangs
	on to file names longer than the files remain open.
	(input_init): Initialize new obstack.
	(push_file): Use new obstack.  Delay updates to current_file
	until after expand_macro has restored state.
	(peek_input, next_char_1): Update current_file if necessary.
	(pop_wrapup): Release memory.
	* doc/m4.texinfo (Macro Arguments, Changequote, Changecom): Catch
	the off-by-one bug.
	(Dnl): Update to the new location reporting rules.
	* NEWS: Document these changes.

Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.1.1.1.2.68
diff -u -r1.1.1.1.2.68 NEWS
--- NEWS	10 Oct 2006 03:54:26 -0000	1.1.1.1.2.68
+++ NEWS	11 Oct 2006 16:44:07 -0000
@@ -13,6 +13,9 @@
   overflow.
 * Problems encountered when writing to standard error, such as with the
   `errprint' macro, now always cause a non-zero exit status.
+* Warnings and errors issued during macro expansion are now consistently
+  reported at the line where the macro name was detected, rather than
+  where the close parenthesis resides.
 
 Version 1.4.7 - 25 September 2006, by Eric Blake  (CVS version 1.4.6a)
 
Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.1.1.1.2.80
diff -u -r1.1.1.1.2.80 m4.texinfo
--- doc/m4.texinfo	10 Oct 2006 03:27:15 -0000	1.1.1.1.2.80
+++ doc/m4.texinfo	11 Oct 2006 16:44:07 -0000
@@ -1104,9 +1104,11 @@
 It is an error if the end of file occurs while collecting arguments.
 
 @example
+hello world
+@result{}hello world
 define(
 ^D
-@error{}m4:stdin:1: ERROR: end of file in argument list
+@error{}m4:stdin:2: ERROR: end of file in argument list
 @end example
 
 @node Quoting Arguments
@@ -2453,7 +2455,7 @@
 @example
 dnl(`args are ignored, but side effects occur',
 define(`foo', `like this')) while this text is ignored: undefine(`foo')
-@error{}m4:stdin:2: Warning: excess arguments to builtin `dnl' ignored
+@error{}m4:stdin:1: Warning: excess arguments to builtin `dnl' ignored
 See how `foo' was defined, foo?
 @result{}See how foo was defined, like this?
 @end example
@@ -2634,9 +2636,11 @@
 It is an error if the end of file occurs within a quoted string.
 
 @example
+`hello world'
+@result{}hello world
 `dangling quote
 ^D
-@error{}m4:stdin:1: ERROR: end of file in string
+@error{}m4:stdin:2: ERROR: end of file in string
 @end example
 
 @node Changecom
@@ -2763,7 +2767,7 @@
 @result{}
 /*dangling comment
 ^D
-@error{}m4:stdin:1: ERROR: end of file in comment
+@error{}m4:stdin:2: ERROR: end of file in comment
 @end example
 
 @node Changeword
Index: src/input.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/input.c,v
retrieving revision 1.1.1.1.2.23
diff -u -r1.1.1.1.2.23 input.c
--- src/input.c	7 Sep 2006 22:48:15 -0000	1.1.1.1.2.23
+++ src/input.c	11 Oct 2006 16:44:07 -0000
@@ -107,6 +107,9 @@
 /* Obstack for storing individual tokens.  */
 static struct obstack token_stack;
 
+/* Obstack for storing file names.  */
+static struct obstack file_names;
+
 /* Wrapup input stack.  */
 static struct obstack *wrapup_stack;
 
@@ -185,14 +188,24 @@
 
   i->u.u_f.end = FALSE;
   i->u.u_f.close = close;
-  i->u.u_f.name = current_file;
-  i->u.u_f.lineno = current_line;
   i->u.u_f.out_lineno = output_current_line;
   i->u.u_f.advance_line = start_of_input_line;
-  current_file = obstack_copy0 (current_input, title, strlen (title));
-  current_line = 1;
   output_current_line = -1;
 
+  /* current_file and current_line may be temporarily inaccurate due
+     to expand_macro remembering where the include or sinclude builtin
+     invocation began, so don't modify them here.  However, we are
+     guaranteed that they are consistent in the context of read or
+     peek.  So we use u_f.lineno == 0 and a non-empty u_f.name as a
+     key that this file is newly pushed, and that current_file/line
+     still needs an update (lineno == 0 and an empty name imply that
+     when we pop this file, there is no more input to return to).
+     Also, we must save title on a separate obstack, again since
+     expand_macro hangs on to file names even after the file is
+     closed.  */
+  i->u.u_f.name = obstack_copy0 (&file_names, title, strlen (title));
+  i->u.u_f.lineno = 0;
+
   i->u.u_f.file = fp;
   i->prev = isp;
   isp = i;
@@ -383,6 +396,10 @@
 
   if (wsp == NULL)
     {
+      /* End of the program.  Free all memory even though we are about
+	 to exit, since it makes leak detection easier.  */
+      obstack_free (&token_stack, NULL);
+      obstack_free (&file_names, NULL);
       obstack_free (wrapup_stack, NULL);
       free (wrapup_stack);
       return FALSE;
@@ -444,6 +461,16 @@
 	  break;
 
 	case INPUT_FILE:
+	  /* See comments in push_file.  */
+	  if (isp->u.u_f.lineno == 0 && isp->u.u_f.name[0] != '\0')
+	    {
+	      const char *tmp = isp->u.u_f.name;
+	      isp->u.u_f.name = current_file;
+	      isp->u.u_f.lineno = current_line;
+	      current_file = tmp;
+	      current_line = 1;
+	    }
+
 	  ch = getc (isp->u.u_f.file);
 	  if (ch != EOF)
 	    {
@@ -510,6 +537,16 @@
 	  break;
 
 	case INPUT_FILE:
+	  /* See comments in push_file.  */
+	  if (isp->u.u_f.lineno == 0 && isp->u.u_f.name[0] != '\0')
+	    {
+	      const char *tmp = isp->u.u_f.name;
+	      isp->u.u_f.name = current_file;
+	      isp->u.u_f.lineno = current_line;
+	      current_file = tmp;
+	      current_line = 1;
+	    }
+
 	  /* If stdin is a terminal, calling getc after peek_input
 	     already called it would make the user have to hit ^D
 	     twice to quit.  */
@@ -638,15 +675,19 @@
   current_file = "";
   current_line = 0;
 
-  obstack_init (&token_stack);
-
   current_input = (struct obstack *) xmalloc (sizeof (struct obstack));
   obstack_init (current_input);
   wrapup_stack = (struct obstack *) xmalloc (sizeof (struct obstack));
   obstack_init (wrapup_stack);
 
-  obstack_1grow (&token_stack, '\0');
-  token_bottom = obstack_finish (&token_stack);
+  obstack_init (&file_names);
+
+  /* Allocate an object in the current chunk, so that obstack_free
+     will always work even if the first token parsed spills to a new
+     chunk.  */
+  obstack_init (&token_stack);
+  obstack_alloc (&token_stack, 1);
+  token_bottom = obstack_base (&token_stack);
 
   isp = NULL;
   wsp = NULL;
@@ -789,12 +830,10 @@
   int startpos;
   char *orig_text = 0;
 #endif
-  const char *file = current_file;
-  int line = current_line;
+  const char *file;
+  int line;
 
   obstack_free (&token_stack, token_bottom);
-  obstack_1grow (&token_stack, '\0');
-  token_bottom = obstack_finish (&token_stack);
 
  /* Can't consume character until after CHAR_MACRO is handled.  */
   ch = peek_input ();
@@ -818,6 +857,8 @@
     }
 
   next_char (); /* Consume character we already peeked at.  */
+  file = current_file;
+  line = current_line;
   if (MATCH (ch, bcomm.string, TRUE))
     {
       obstack_grow (&token_stack, bcomm.string, bcomm.length);
Index: src/macro.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/macro.c,v
retrieving revision 1.1.1.1.2.13
diff -u -r1.1.1.1.2.13 macro.c
--- src/macro.c	10 Oct 2006 16:35:23 -0000	1.1.1.1.2.13
+++ src/macro.c	11 Oct 2006 16:44:07 -0000
@@ -310,6 +310,17 @@
   boolean traced;
   int my_call_id;
 
+  /* Report errors at the location where the open parenthesis (if any)
+     was found, but after expansion, restore global state back to the
+     location of the close parenthesis.  This is safe since we
+     guarantee that macro expansion does not alter the state of
+     current_file/current_line (dnl, include, and sinclude are special
+     cased in the input engine to ensure this fact).  */
+  const char *loc_open_file = current_file;
+  int loc_open_line = current_line;
+  const char *loc_close_file;
+  int loc_close_line;
+
   SYMBOL_PENDING_EXPANSIONS (sym)++;
   expansion_level++;
   if (nesting_limit > 0 && expansion_level > nesting_limit)
@@ -342,6 +353,11 @@
 	  / sizeof (token_data *));
   argv = (token_data **) (obstack_base (&argv_stack) + argv_base);
 
+  loc_close_file = current_file;
+  loc_close_line = current_line;
+  current_file = loc_open_file;
+  current_line = loc_open_line;
+
   if (traced)
     trace_pre (SYMBOL_NAME (sym), my_call_id, argc, argv);
 
@@ -352,6 +368,9 @@
   if (traced)
     trace_post (SYMBOL_NAME (sym), my_call_id, argc, argv, expanded);
 
+  current_file = loc_close_file;
+  current_line = loc_close_line;
+
   --expansion_level;
   --SYMBOL_PENDING_EXPANSIONS (sym);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.