Re: Proposed patch: allow __line__ and __file__ to be changed

[Eric Blake]

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

According to Raphael 'Kena' Poss on 7/7/2009 3:14 AM:
> Hi Eric, all,
> 
> Following our recent conversation on m4-discuss, I propose the change
> below that provides the feature I was looking for. It basically provides
> what Eric suggested, i.e. __file__ and __line__ take an optional
> argument which causes resp. the input title and line counter to be
> updated, and a syncline to be emitted at the next following newline.
> 
> Comments, suggestions etc welcome.

Patches are easier to review when sent inline, rather than
application/octet-stream.

First, there's the matter of copyright assignment, that we are working
off-list.  After that, here's my first review for style matters (with very
little thought about functional).

I don't want to apply this to m4 1.4.x (ie. branch-1.4 in m4.git), for the
very reason that it is supposed to be a stable branch.  You would need to
regenerate this patch against branch-1.6 in git.  Git also makes it easier
to send inline patches.  You may want to read
http://git.savannah.gnu.org/cgit/coreutils.git/tree/HACKING; although not
all of the concepts in that document are appropriate to m4, a lot of the
advice on using git is.

> diff -ur m4-1.4.13-orig/src/builtin.c m4-1.4.13/src/builtin.c
> --- m4-1.4.13-orig/src/builtin.c      2009-03-05 12:46:01.000000000 +0000
> +++ m4-1.4.13/src/builtin.c   2009-07-07 10:05:01.000000000 +0100
> @@ -1486,19 +1486,39 @@
>  static void
>  m4___file__ (struct obstack *obs, int argc, token_data **argv)
>  {
> -  if (bad_argc (argv[0], argc, 1, 1))
> +  if (bad_argc (argv[0], argc, 1, 2))
>      return;
> -  obstack_grow (obs, lquote.string, lquote.length);
> -  obstack_grow (obs, current_file, strlen (current_file));
> -  obstack_grow (obs, rquote.string, rquote.length);
> +  if (*ARG (1)) 

This line introduces trailing whitespace.  Also, on branch-1.6, where m4
can handle NUL bytes, it is better to check on the number of arguments
than on whether the first argument is empty.

> +    {
> +      isp->file = (char *) obstack_copy0 (&file_names, 
> +                                       ARG (1), strlen (ARG (1)));
> +      output_current_line = -1;
> +      input_change = 1;
> +    }
> +  else 
> +    {
> +      obstack_grow (obs, lquote.string, lquote.length);
> +      obstack_grow (obs, current_file, strlen (current_file));
> +      obstack_grow (obs, rquote.string, rquote.length);
> +    }
>  }
>  
>  static void
>  m4___line__ (struct obstack *obs, int argc, token_data **argv)
>  {
> -  if (bad_argc (argv[0], argc, 1, 1))
> +  int value;
> +
> +  if (bad_argc (argv[0], argc, 1, 2))
>      return;
> -  shipout_int (obs, current_line);
> +  if (*ARG (1)) 

More trailing whitespace.

> +    {
> +      if (!numeric_arg (argv[0], ARG (1), &value))
> +     return;
> +      isp->line = value;
> +      input_change = 1;
> +    }
> +  else 
> +    shipout_int (obs, current_line);
>  }
>  
>  static void
> diff -ur m4-1.4.13-orig/src/input.c m4-1.4.13/src/input.c
> --- m4-1.4.13-orig/src/input.c        2009-02-23 04:36:37.000000000 +0000
> +++ m4-1.4.13/src/input.c     2009-07-07 09:59:26.000000000 +0100
> @@ -64,44 +64,6 @@
>  #include "regex.h"
>  #endif
>  
> -enum input_type
> -{
> -  INPUT_STRING,              /* String resulting from macro expansion.  */
> -  INPUT_FILE,                /* File from command line or include.  */
> -  INPUT_MACRO                /* Builtin resulting from defn.  */
> -};
> -
> -typedef enum input_type input_type;
> -
> -struct input_block
> -{
> -  struct input_block *prev;  /* previous input_block on the input stack */
> -  input_type type;           /* see enum values */
> -  const char *file;          /* file where this input is from */
> -  int line;                  /* line where this input is from */
> -  union
> -    {
> -      struct
> -     {
> -       char *string;         /* remaining string value */
> -       char *end;            /* terminating NUL of string */
> -     }
> -     u_s;    /* INPUT_STRING */
> -      struct
> -     {
> -       FILE *fp;                  /* input file handle */
> -       bool_bitfield end : 1;     /* true if peek has seen EOF */
> -       bool_bitfield close : 1;   /* true if we should close file on pop */
> -       bool_bitfield advance : 1; /* track previous start_of_input_line */
> -     }
> -     u_f;    /* INPUT_FILE */
> -      builtin_func *func;    /* pointer to macro's function */
> -    }
> -  u;
> -};
> -
> -typedef struct input_block input_block;
> -
>  
>  /* Current input file name.  */
>  const char *current_file;
> @@ -113,7 +75,7 @@
>  static struct obstack token_stack;
>  
>  /* Obstack for storing file names.  */
> -static struct obstack file_names;
> +struct obstack file_names;
>  
>  /* Wrapup input stack.  */
>  static struct obstack *wrapup_stack;
> @@ -125,7 +87,7 @@
>  static void *token_bottom;
>  
>  /* Pointer to top of current_input.  */
> -static input_block *isp;
> +input_block *isp;
>  
>  /* Pointer to top of wrapup_stack.  */
>  static input_block *wsp;
> @@ -137,7 +99,7 @@
>  static bool start_of_input_line;
>  
>  /* Flag for next_char () to recognize change in input block.  */
> -static bool input_change;
> +bool input_change;
>  
>  #define CHAR_EOF     256     /* character return on EOF */
>  #define CHAR_MACRO   257     /* character return for MACRO token */
> diff -ur m4-1.4.13-orig/src/m4.h m4-1.4.13/src/m4.h
> --- m4-1.4.13-orig/src/m4.h   2008-12-12 13:11:39.000000000 +0000
> +++ m4-1.4.13/src/m4.h        2009-07-07 09:59:02.000000000 +0100
> @@ -481,3 +481,52 @@
>  #else
>  # define to_uchar(C) ((unsigned char) (C))
>  #endif
> +
> +
> +/* File: input.c -- input sources */  
> +enum input_type
> +{
> +  INPUT_STRING,              /* String resulting from macro expansion.  */
> +  INPUT_FILE,                /* File from command line or include.  */
> +  INPUT_MACRO                /* Builtin resulting from defn.  */
> +};
> +
> +typedef enum input_type input_type;
> +
> +struct input_block
> +{
> +  struct input_block *prev;  /* previous input_block on the input stack */
> +  input_type type;           /* see enum values */
> +  const char *file;          /* file where this input is from */
> +  int line;                  /* line where this input is from */
> +  union
> +    {
> +      struct
> +     {
> +       char *string;         /* remaining string value */
> +       char *end;            /* terminating NUL of string */
> +     }
> +     u_s;    /* INPUT_STRING */
> +      struct
> +     {
> +       FILE *fp;                  /* input file handle */
> +       bool_bitfield end : 1;     /* true if peek has seen EOF */
> +       bool_bitfield close : 1;   /* true if we should close file on pop */
> +       bool_bitfield advance : 1; /* track previous start_of_input_line */
> +     }
> +     u_f;    /* INPUT_FILE */
> +      builtin_func *func;    /* pointer to macro's function */
> +    }
> +  u;
> +};
> +
> +typedef struct input_block input_block;

I'm still thinking that it may be better to keep this as an opaque type,
available only via accessors from m4.h rather than moving the entire
struct definition to be public in m4.h.  In particular, it will make
porting this concept to the master branch (where the input engine is
already an opaque structure accessible only to clients of m4private.h, but
not to clients of m4module.h).

> +
> +/* Pointer to top of current_input.  */
> +extern input_block *isp;
> +
> +/* Flag for next_char () to recognize change in input block.  */
> +extern bool input_change;
> +
> +/* Obstack for storing file names.  */
> +extern struct obstack file_names;

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

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

iEYEARECAAYFAkpTPNsACgkQ84KuGfSFAYD6HwCdHsn5e4ahzOQ9MzXpfWKoiQ3c
45YAni33ZuGZKd25TCmk9wMupxHYSgjJ
=OA7t
-----END PGP SIGNATURE-----