Re: list of GNU indent versions in gdb_indent.sh

[David Carlton <carlton@math.stanford.edu>]

[ Side note: I just to send a version of this including the
2.2.6/2.2.7 diffs, which is almost 7000 lines long and 200000
characters long.  This time, my mailer doesn't try to split it up; and
sources.redhat.com's daemon (quite reasonably) rejects the message
because it's more than 100000 characters long.  Sigh; I think it's not
my day with the technicalities of e-mail today. ]

On Thu, 12 Sep 2002 16:01:15 -0400, Andrew Cagney
<ac131313@ges.redhat.com> said:

> Can you please find out exactly what was changed?

The NEWS/ChangeLog files were uninformative, so I just copied all of
gdb/*.{h,c} to test directories and ran gdb_indent on them with the
three versions of indent.  Here's the scoop, as I see it:

2.2.6 and 2.2.7 are pretty much the same: the main difference is that
2.2.6 is bad at handling string constants with embedded newlines.
(Where by "bad" I mean "reformats them, so the meaning of your code
changes".)

The main difference between 2.2.7 and 2.2.8 seems to be that 2.2.8
tries to handle comments like

/* 
 * Beginning of a very long sentence that continues to the next line,
 * which starts with an asterisk as well.
 */

well, whereas 2.2.7 doesn't.  The changes in 2.2.8 end up breaking
some GDB comments like this:

/* Here are some items to consider:
   * Item 1.
   * Item 2.
   That's it.  */

Because of the initial asterisks (and I'm not sure I exactly
understand what are the necessary and sufficient conditions to
triggers this, 2.2.8 reformats that like

/* Here are some items to consider:
 * Item 1.
 * Item 2.
 That's it.  */

Another difference that 2.2.8 gets right and 2.2.7 gets wrong is that
2.2.7 has problems in situations like this:

 /* comment 1 *//* comment 2*/

(Most of the time in GDB's sources, one of the comments is an OBSOLETE
comment, but not always.)

I'll stick in the 2.2.7/2.2.8 diffs at the end of this message;
2.2.6/2.2.7 diffs available on request.

I would recommend switching away from 2.2.6.  I'm more agnostic on the
2.2.7/8 difference; some of the places where 2.2.8 gets it wrong annoy
me, but on the other hand if we're going to switch then we might as
well switch to the newest version.

The difference between 7 and 8 only affects something like 35
locations in all of the GDB source; also note that some of those
locations are situations containing box-style comments (where 2.2.8
gets it right and 2.2.7 gets it wrong), and some of those situations
involve OBSOLETE stuff that probably won't get run through
gdb_indent.sh anyways.

David Carlton
carlton@math.stanford.edu

Here are the 2.2.7/2.2.8 diffs:

diff -u 2.2.7/breakpoint.c 2.2.8/breakpoint.c
--- 2.2.7/breakpoint.c	Thu Sep 12 13:34:00 2002
+++ 2.2.8/breakpoint.c	Thu Sep 12 13:29:12 2002
@@ -2382,9 +2382,9 @@
   if (within_current_scope)
     {
       /* We use value_{,free_to_}mark because it could be a
-         *long* time before we return to the command level and
-         call free_all_values.  We can't call free_all_values because
-         we might be in the middle of evaluating a function call.  */
+       *long* time before we return to the command level and
+       call free_all_values.  We can't call free_all_values because
+       we might be in the middle of evaluating a function call.  */
 
       struct value *mark = value_mark ();
       struct value *new_val = evaluate_expression (bs->breakpoint_at->exp);
diff -u 2.2.7/command.h 2.2.8/command.h
--- 2.2.7/command.h	Thu Sep 12 13:33:59 2002
+++ 2.2.8/command.h	Thu Sep 12 13:29:11 2002
@@ -78,10 +78,10 @@
      *VAR is a malloc'd string, or NULL if the string is empty.  */
   var_string,
   /* String which stores what the user types verbatim.
-     *VAR is a malloc'd string, or NULL if the string is empty.  */
+   *VAR is a malloc'd string, or NULL if the string is empty.  */
   var_string_noescape,
   /* String which stores a filename.
-     *VAR is a malloc'd string, or NULL if the string is empty.  */
+   *VAR is a malloc'd string, or NULL if the string is empty.  */
   var_filename,
   /* ZeroableInteger.  *VAR is an int.  Like Unsigned Integer except
      that zero really means zero.  */
diff -u 2.2.7/defs.h 2.2.8/defs.h
--- 2.2.7/defs.h	Thu Sep 12 13:33:59 2002
+++ 2.2.8/defs.h	Thu Sep 12 13:29:11 2002
@@ -1,4 +1,4 @@
-/* *INDENT-OFF* *//* ATTR_FORMAT confuses indent, avoid running it for now */
+		   /* *INDENT-OFF* *//* ATTR_FORMAT confuses indent, avoid running it for now */
 /* Basic, host-specific, and target-specific definitions for GDB.
    Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996,
    1997, 1998, 1999, 2000, 2001, 2002
diff -u 2.2.7/event-top.c 2.2.8/event-top.c
--- 2.2.7/event-top.c	Thu Sep 12 13:34:02 2002
+++ 2.2.8/event-top.c	Thu Sep 12 13:29:14 2002
@@ -568,7 +568,7 @@
   long space_at_cmd_start = arg->next->data.longint;
 
   bpstat_do_actions (&stop_bpstat);
-/*do_cleanups (old_chain); *//*?????FIXME????? */
+  /*do_cleanups (old_chain); *//*?????FIXME????? */
 
   if (display_time)
     {
diff -u 2.2.7/gdbarch.c 2.2.8/gdbarch.c
--- 2.2.7/gdbarch.c	Thu Sep 12 13:34:02 2002
+++ 2.2.8/gdbarch.c	Thu Sep 12 13:29:14 2002
@@ -1,4 +1,4 @@
-/* *INDENT-OFF* *//* THIS FILE IS GENERATED */
+		   /* *INDENT-OFF* *//* THIS FILE IS GENERATED */
 
 /* Dynamic architecture support for GDB, the GNU debugger.
    Copyright 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
diff -u 2.2.7/gdbarch.h 2.2.8/gdbarch.h
--- 2.2.7/gdbarch.h	Thu Sep 12 13:33:59 2002
+++ 2.2.8/gdbarch.h	Thu Sep 12 13:29:11 2002
@@ -1,4 +1,4 @@
-/* *INDENT-OFF* *//* THIS FILE IS GENERATED */
+		   /* *INDENT-OFF* *//* THIS FILE IS GENERATED */
 
 /* Dynamic architecture support for GDB, the GNU debugger.
    Copyright 1998, 1999, 2000, 2001, 2002 Free Software Foundation, Inc.
diff -u 2.2.7/hpread.c 2.2.8/hpread.c
--- 2.2.7/hpread.c	Thu Sep 12 13:34:03 2002
+++ 2.2.8/hpread.c	Thu Sep 12 13:29:15 2002
@@ -5700,7 +5700,7 @@
 	case DNTT_TYPE_CLASS_SCOPE:
 
 	  /* pai: FIXME Not handling nested classes for now -- must
-	     * maintain a stack */
+	   * maintain a stack */
 	  class_scope_name = NULL;
 
 #if 0
Binary files 2.2.7/indent and 2.2.8/indent differ
diff -u 2.2.7/inftarg.c 2.2.8/inftarg.c
--- 2.2.7/inftarg.c	Thu Sep 12 13:34:03 2002
+++ 2.2.8/inftarg.c	Thu Sep 12 13:29:16 2002
@@ -219,7 +219,7 @@
 	  return pid_to_ptid (pid);
 	}
 
-/*##  } while (pid != PIDGET (inferior_ptid)); ## *//* Some other child died or stopped */
+      /*##  } while (pid != PIDGET (inferior_ptid)); ## *//* Some other child died or stopped */
 /* hack for thread testing */
     }
   while ((pid != PIDGET (inferior_ptid)) && not_same_real_pid);
diff -u 2.2.7/language.c 2.2.8/language.c
--- 2.2.7/language.c	Thu Sep 12 13:34:04 2002
+++ 2.2.8/language.c	Thu Sep 12 13:29:16 2002
@@ -570,7 +570,7 @@
       return l1 > l2 ? VALUE_TYPE (v1) : VALUE_TYPE (v2);
       break;
       /* OBSOLETE case language_chill: */
-/* OBSOLETE    error ("Missing Chill support in function binop_result_check.");  *//*FIXME */
+      /* OBSOLETE    error ("Missing Chill support in function binop_result_check.");  *//*FIXME */
     }
   internal_error (__FILE__, __LINE__, "failed internal consistency check");
   return (struct type *) 0;	/* For lint */
@@ -793,7 +793,7 @@
     case language_pascal:
       return TYPE_CODE (type) != TYPE_CODE_INT ? 0 : 1;
       /* OBSOLETE case language_chill: */
-/* OBSOLETE   error ("Missing Chill support in function integral_type."); *//*FIXME */
+      /* OBSOLETE   error ("Missing Chill support in function integral_type."); *//*FIXME */
     default:
       error ("Language not supported.");
     }
@@ -917,7 +917,7 @@
 	(TYPE_CODE (type) == TYPE_CODE_SET) ||
 	(TYPE_CODE (type) == TYPE_CODE_ARRAY);
       /* OBSOLETE case language_chill: */
-/* OBSOLETE     error ("Missing Chill support in function structured_type.");     *//*FIXME */
+      /* OBSOLETE     error ("Missing Chill support in function structured_type.");     *//*FIXME */
     default:
       return (0);
     }
@@ -1175,7 +1175,7 @@
 
 #ifdef _LANG_chill		/* OBSOLETE */
 	  /* OBSOLETE case language_chill: */
-/* OBSOLETE   error ("Missing Chill support in function binop_type_check.");   *//*FIXME */
+	  /* OBSOLETE   error ("Missing Chill support in function binop_type_check.");   *//*FIXME */
 #endif
 
 	}
diff -u 2.2.7/macrotab.c 2.2.8/macrotab.c
--- 2.2.7/macrotab.c	Thu Sep 12 13:34:05 2002
+++ 2.2.8/macrotab.c	Thu Sep 12 13:29:17 2002
@@ -456,8 +456,8 @@
     }
 
   /* At this point, we know that LINE is an unused line number, and
-     *LINK points to the entry an #inclusion at that line should
-     precede.  */
+   *LINK points to the entry an #inclusion at that line should
+   precede.  */
   new = new_source_file (source->table, included);
   new->included_by = source;
   new->included_at_line = line;
diff -u 2.2.7/main.c 2.2.8/main.c
--- 2.2.7/main.c	Thu Sep 12 13:34:05 2002
+++ 2.2.8/main.c	Thu Sep 12 13:29:17 2002
@@ -532,9 +532,9 @@
   warning_pre_print = _("\nwarning: ");
 
   /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
-     *before* all the command line arguments are processed; it sets
-     global parameters, which are independent of what file you are
-     debugging or what directory you are in.  */
+   *before* all the command line arguments are processed; it sets
+   global parameters, which are independent of what file you are
+   debugging or what directory you are in.  */
   homedir = getenv ("HOME");
   if (homedir)
     {
diff -u 2.2.7/remote-rdp.c 2.2.8/remote-rdp.c
--- 2.2.7/remote-rdp.c	Thu Sep 12 13:34:07 2002
+++ 2.2.8/remote-rdp.c	Thu Sep 12 13:29:19 2002
@@ -274,8 +274,8 @@
 	printf_unfiltered ("Trying to connect at %d baud.\n", baudtry);
 
       /*
-         ** It seems necessary to reset an EmbeddedICE to get it going.
-         ** This has the side benefit of displaying the startup banner.
+       ** It seems necessary to reset an EmbeddedICE to get it going.
+       ** This has the side benefit of displaying the startup banner.
        */
       if (cold)
 	{
@@ -689,9 +689,9 @@
 rdp_set_command_line (char *command, char *args)
 {
   /*
-     ** We could use RDP_INFO_SET_CMDLINE to send this, but EmbeddedICE systems
-     ** don't implement that, and get all confused at the unexpected text.
-     ** Instead, just keep a copy, and send it when the target does a SWI_GetEnv
+   ** We could use RDP_INFO_SET_CMDLINE to send this, but EmbeddedICE systems
+   ** don't implement that, and get all confused at the unexpected text.
+   ** Instead, just keep a copy, and send it when the target does a SWI_GetEnv
    */
 
   if (commandline != NULL)
@@ -704,17 +704,17 @@
 rdp_catch_vectors (void)
 {
   /*
-     ** We want the target monitor to intercept the abort vectors
-     ** i.e. stop the program if any of these are used.
+   ** We want the target monitor to intercept the abort vectors
+   ** i.e. stop the program if any of these are used.
    */
   send_rdp ("bww-SZ", RDP_INFO, RDP_INFO_VECTOR_CATCH,
 	    /*
-	       ** Specify a bitmask including
-	       **  the reset vector
-	       **  the undefined instruction vector
-	       **  the prefetch abort vector
-	       **  the data abort vector
-	       **  the address exception vector
+	     ** Specify a bitmask including
+	     **  the reset vector
+	     **  the undefined instruction vector
+	     **  the prefetch abort vector
+	     **  the data abort vector
+	     **  the address exception vector
 	     */
 	    (1 << 0) | (1 << 1) | (1 << 3) | (1 << 4) | (1 << 5));
 }
@@ -1112,8 +1112,8 @@
   rdp_catch_vectors ();
 
   /*
-     ** If it's an EmbeddedICE, we need to set the processor config.
-     ** Assume we can always have ARM7TDI...
+   ** If it's an EmbeddedICE, we need to set the processor config.
+   ** Assume we can always have ARM7TDI...
    */
   send_rdp ("bw-SB", RDP_INFO, RDP_INFO_ICEBREAKER, &not_icebreaker);
   if (!not_icebreaker)
@@ -1332,8 +1332,8 @@
   insert_breakpoints ();	/* Needed to get correct instruction in cache */
 
   /*
-     ** RDP targets don't provide any facility to set the top of memory,
-     ** so we don't bother to look for MEMSIZE in the environment.
+   ** RDP targets don't provide any facility to set the top of memory,
+   ** so we don't bother to look for MEMSIZE in the environment.
    */
 
   /* Let's go! */
diff -u 2.2.7/sh-stub.c 2.2.8/sh-stub.c
--- 2.2.7/sh-stub.c	Thu Sep 12 13:34:07 2002
+++ 2.2.8/sh-stub.c	Thu Sep 12 13:29:20 2002
@@ -577,8 +577,8 @@
 	  if (displacement & 0x80)
 	    displacement |= 0xffffff00;
 	  /*
-	     * Remember PC points to second instr.
-	     * after PC of branch ... so add 4
+	   * Remember PC points to second instr.
+	   * after PC of branch ... so add 4
 	   */
 	  instrMem = (short *) (registers[PC] + displacement + 4);
 	}
@@ -595,8 +595,8 @@
 	  if (displacement & 0x80)
 	    displacement |= 0xffffff00;
 	  /*
-	     * Remember PC points to second instr.
-	     * after PC of branch ... so add 4
+	   * Remember PC points to second instr.
+	   * after PC of branch ... so add 4
 	   */
 	  instrMem = (short *) (registers[PC] + displacement + 4);
 	}
@@ -608,8 +608,8 @@
 	displacement |= 0xfffff000;
 
       /*
-         * Remember PC points to second instr.
-         * after PC of branch ... so add 4
+       * Remember PC points to second instr.
+       * after PC of branch ... so add 4
        */
       instrMem = (short *) (registers[PC] + displacement + 4);
     }
diff -u 2.2.7/symfile.c 2.2.8/symfile.c
--- 2.2.7/symfile.c	Thu Sep 12 13:34:09 2002
+++ 2.2.8/symfile.c	Thu Sep 12 13:29:21 2002
@@ -736,7 +736,7 @@
       /* Map section offsets in "addr" back to the object's 
          sections by comparing the section names with bfd's 
          section names.  Then adjust the section address by
-   the offset. *//* for gdb/13815 */
+         the offset. *//* for gdb/13815 */
 
       ALL_OBJFILE_OSECTIONS (objfile, s)
       {
diff -u 2.2.7/valops.c 2.2.8/valops.c
--- 2.2.7/valops.c	Thu Sep 12 13:34:10 2002
+++ 2.2.8/valops.c	Thu Sep 12 13:29:22 2002
@@ -389,7 +389,7 @@
   /* OBSOLETE     int count1, count2; */
   /* OBSOLETE     LONGEST low_bound, high_bound; */
   /* OBSOLETE     char *valaddr, *valaddr_data; */
-/* OBSOLETE     *//* For lint warning about eltype2 possibly uninitialized: */
+  /* OBSOLETE     *//* For lint warning about eltype2 possibly uninitialized: */
   /* OBSOLETE     eltype2 = NULL; */
   /* OBSOLETE     if (code2 == TYPE_CODE_BITSTRING) */
   /* OBSOLETE       error ("not implemented: converting bitstring to varying type"); */
@@ -397,7 +397,7 @@
   /* OBSOLETE         || (eltype1 = check_typedef (TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type, 1))), */
   /* OBSOLETE       eltype2 = check_typedef (TYPE_TARGET_TYPE (type2)), */
   /* OBSOLETE                                (TYPE_LENGTH (eltype1) != TYPE_LENGTH (eltype2) */
-/* OBSOLETE     *//*|| TYPE_CODE (eltype1) != TYPE_CODE (eltype2) *//* ))) */
+  /* OBSOLETE     *//*|| TYPE_CODE (eltype1) != TYPE_CODE (eltype2) *//* ))) */
   /* OBSOLETE      error ("Invalid conversion to varying type"); */
   /* OBSOLETE     range1 = TYPE_FIELD_TYPE (TYPE_FIELD_TYPE (type, 1), 0); */
   /* OBSOLETE     range2 = TYPE_FIELD_TYPE (type2, 0); */
@@ -406,7 +406,7 @@
   /* OBSOLETE     else */
   /* OBSOLETE       count1 = high_bound - low_bound + 1; */
   /* OBSOLETE     if (get_discrete_bounds (range2, &low_bound, &high_bound) < 0) */
-/* OBSOLETE       count1 = -1, count2 = 0;    *//* To force error before */
+  /* OBSOLETE       count1 = -1, count2 = 0;    *//* To force error before */
   /* OBSOLETE     else */
   /* OBSOLETE       count2 = high_bound - low_bound + 1; */
   /* OBSOLETE     if (count2 > count1) */
@@ -414,13 +414,13 @@
   /* OBSOLETE     val = allocate_value (type); */
   /* OBSOLETE     valaddr = VALUE_CONTENTS_RAW (val); */
   /* OBSOLETE     valaddr_data = valaddr + TYPE_FIELD_BITPOS (type, 1) / 8; */
-/* OBSOLETE     *//* Set val's __var_length field to count2. */
+  /* OBSOLETE     *//* Set val's __var_length field to count2. */
   /* OBSOLETE     store_signed_integer (valaddr, TYPE_LENGTH (TYPE_FIELD_TYPE (type, 0)), */
   /* OBSOLETE       count2); */
-/* OBSOLETE     *//* Set the __var_data field to count2 elements copied from arg2. */
+  /* OBSOLETE     *//* Set the __var_data field to count2 elements copied from arg2. */
   /* OBSOLETE     memcpy (valaddr_data, VALUE_CONTENTS (arg2), */
   /* OBSOLETE      count2 * TYPE_LENGTH (eltype2)); */
-/* OBSOLETE     *//* Zero the rest of the __var_data field of val. */
+  /* OBSOLETE     *//* Zero the rest of the __var_data field of val. */
   /* OBSOLETE     memset (valaddr_data + count2 * TYPE_LENGTH (eltype2), '\0', */
   /* OBSOLETE      (count1 - count2) * TYPE_LENGTH (eltype2)); */
   /* OBSOLETE     return val; */
@@ -3241,7 +3241,7 @@
     return argp;
 
   /* If we have the full object, but for some reason the enclosing
-   type is wrong, set it *//* pai: FIXME -- sounds iffy */
+     type is wrong, set it *//* pai: FIXME -- sounds iffy */
   if (full)
     {
       argp = value_change_enclosing_type (argp, real_type);
diff -u 2.2.7/wince.c 2.2.8/wince.c
--- 2.2.7/wince.c	Thu Sep 12 13:34:10 2002
+++ 2.2.8/wince.c	Thu Sep 12 13:29:23 2002
@@ -882,8 +882,8 @@
 	  if (displacement & 0x80)
 	    displacement |= 0xffffff00;
 	  /*
-	     * Remember PC points to second instr.
-	     * after PC of branch ... so add 4
+	   * Remember PC points to second instr.
+	   * after PC of branch ... so add 4
 	   */
 	  instrMem = (short *) (c->Fir + displacement + 4);
 	}
@@ -900,8 +900,8 @@
 	  if (displacement & 0x80)
 	    displacement |= 0xffffff00;
 	  /*
-	     * Remember PC points to second instr.
-	     * after PC of branch ... so add 4
+	   * Remember PC points to second instr.
+	   * after PC of branch ... so add 4
 	   */
 	  instrMem = (short *) (c->Fir + displacement + 4);
 	}
@@ -913,8 +913,8 @@
 	displacement |= 0xfffff000;
 
       /*
-         * Remember PC points to second instr.
-         * after PC of branch ... so add 4
+       * Remember PC points to second instr.
+       * after PC of branch ... so add 4
        */
       instrMem = (short *) (c->Fir + displacement + 4);
     }