Re: [PATCH] macro hooks

[Ian Lance Taylor <ian at zembu dot com>]

   Date: Wed, 23 Feb 2000 11:16:45 -0500
   From: Timothy Wall <twall@alum.mit.edu>

   *************** input_scrub_include_sb (from, position)
   *** 259,267 ****
	  sb *from;
	  char *position;
     {
   !   if (macro_nest > max_macro_nest)
   !     as_fatal (_("macros nested too deeply"));
   !   ++macro_nest;

       next_saved_file = input_scrub_push (position);

   --- 259,279 ----
	  sb *from;
	  char *position;
     {
   !   /* I put this flag so that it's possible to do a wholesale
   !      replace of a single line w/o disrupting the input stream; if the checking
   !      is enabled, substitutions on lines like " .if SUBS" will fail, thinking
   !      they have to see ".endif" before the end of the single-line sb
   !      twall@cygnus.com 
   !   */
   !   if (!from->no_macro_check)
   !     {
   !       if (macro_nest > max_macro_nest)
   !         as_fatal (_("macros nested too deeply"));
   !       ++macro_nest;
   ! #ifdef md_macro_start
   !       md_macro_start ();
   ! #endif
   !     }

       next_saved_file = input_scrub_push (position);

I don't like this approach.  struct sb is a simple structure that
represents a line in memory.  I think that adding a field like
no_macro_check is inappropriately mixing levels.  struct sb is not
about macros.  It is about strings stored in memory.

As far as I can tell, you are adding this flag to avoid calling your
new macros, and to avoid calling cond_finish_check.  That is
reasonable, but the place to store this information is in a new
variable which is saved and restored by input_scrub_push and
input_scrub_pop.  This variable could then be set by
input_scrub_include_sb, or perhaps even by a new function which called
input_scrub_include_sb.

Note that I don't see why this new flag should control macro_nest.
macro_nest seems just as appropriate for a single line as for an
entire macro.  I do see why it should control cond_finish_check.

Also, this is a minor point, but I think it's bad style to name flags
in the negative, like no_macro_check.  That makes it harder to
understand how to use them correctly.  Name the flag positively,
macro_check, and reverse the uses.

     int
   ! check_macro (line, expand, comment_char, error, info)
	  const char *line;
	  sb *expand;
	  int comment_char;
	  const char **error;
   +      void **info;

What is the point of this?  Nobody can use the returned information.
If you need to add a parameter like this, don't you need to move the
struct definition into macro.h?  Even if you don't, there is no need
to lose the type information.  The info parameter should be struct
macro_struct **, not void **.

   +   /* export the macro information if requested */

Comments should be complete sentences with correct capitalization and
punctuation.

   +   sb newline;
   +   sb_new (&newline);
   +   while (*line != 0)
   +     sb_add_char (&newline, *line++);

Use sb_add_string.

   + /* Insert a (usually automatically generated) file into the input stream;
   +    the path must resolve to an actual file; no include path searching or
   +    dependency registering is performed.  
   +  */

The */ at the end of a comment should go on the same line as the last
sentence in the comment.  I think you did this a few other places as
well.

   + void
   + insert_file (path)
   +   const char *path;

Perhaps s_include should be changed to call insert_file.

I think input_scrub_insert_file might be a better name.  Also
input_scrub_insert_line.  insert_line and insert_file are a trifle too
generic.

Ian