[rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

This patch changes no functionality; it replaces for loops with an
ALL_BLOCK_SYMBOLS macro in places where it is entirely unambiguous to do so. 
I consider it fairly obvious, but I would greatly appreciate another set of
eyes, since I touch a fair amount of code.  The large printcmd changes are
(should be?) just indentation updates.

Is this OK?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-10-01  Daniel Jacobowitz  <drow@mvista.com>

	* symtab.h (struct block): (ALL_BLOCK_SYMBOLS): New macro.

	* symtab.c (find_pc_sect_symtab): Use ALL_BLOCK_SYMBOLS.
	(make_symbol_completion_list): Likewise.
	(make_symbol_overload_list): Likewise.
	* symmisc.c (dump_symtab): Likewise.
	* breakpoint.c (get_catch_sals):  Likewise.
	* objfiles.c (objfile_relocate): Likewise.
	* stack.c (print_block_frame_locals): Likewise.
	(print_block_frame_labels): Likewise.
	(print_frame_arg_vars): Likewise.
	* tracepoint.c (add_local_symbols): Likewise.
	(scope_info): Likewise.
	* printcmd.c (print_frame_args): Likewise.

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.53
diff -u -p -r1.53 breakpoint.c
--- breakpoint.c	2001/09/18 05:00:48	1.53
+++ breakpoint.c	2001/10/01 22:20:38
@@ -5871,15 +5871,11 @@ get_catch_sals (int this_level_only)
 	  if (blocks_searched[index] == 0)
 	    {
 	      struct block *b = BLOCKVECTOR_BLOCK (bl, index);
-	      int nsyms;
 	      register int i;
 	      register struct symbol *sym;
 
-	      nsyms = BLOCK_NSYMS (b);
-
-	      for (i = 0; i < nsyms; i++)
+	      ALL_BLOCK_SYMBOLS (b, i, sym)
 		{
-		  sym = BLOCK_SYM (b, i);
 		  if (STREQ (SYMBOL_NAME (sym), "default"))
 		    {
 		      if (have_default)
Index: gdb/objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.16
diff -u -p -r1.16 objfiles.c
--- objfiles.c	2001/07/05 21:32:39	1.16
+++ objfiles.c	2001/10/01 22:20:53
@@ -557,16 +557,15 @@ objfile_relocate (struct objfile *objfil
       for (i = 0; i < BLOCKVECTOR_NBLOCKS (bv); ++i)
 	{
 	  struct block *b;
+	  struct symbol *sym;
 	  int j;
 
 	  b = BLOCKVECTOR_BLOCK (bv, i);
 	  BLOCK_START (b) += ANOFFSET (delta, s->block_line_section);
 	  BLOCK_END (b) += ANOFFSET (delta, s->block_line_section);
 
-	  for (j = 0; j < BLOCK_NSYMS (b); ++j)
+	  ALL_BLOCK_SYMBOLS (b, j, sym)
 	    {
-	      struct symbol *sym = BLOCK_SYM (b, j);
-
 	      fixup_symbol_section (sym, objfile);
 
 	      /* The RS6000 code from which this was taken skipped
Index: gdb/printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.27
diff -u -p -r1.27 printcmd.c
--- printcmd.c	2001/09/12 04:18:08	1.27
+++ printcmd.c	2001/10/01 22:20:54
@@ -1806,168 +1806,168 @@ print_frame_args (struct symbol *func, s
   if (func)
     {
       b = SYMBOL_BLOCK_VALUE (func);
       nsyms = BLOCK_NSYMS (b);
-    }
 
-  for (i = 0; i < nsyms; i++)
-    {
-      QUIT;
-      sym = BLOCK_SYM (b, i);
+      ALL_BLOCK_SYMBOLS (b, i, sym)
+        {
+	  QUIT;
+	  sym = BLOCK_SYM (b, i);
 
-      /* Keep track of the highest stack argument offset seen, and
-         skip over any kinds of symbols we don't care about.  */
+	  /* Keep track of the highest stack argument offset seen, and
+	     skip over any kinds of symbols we don't care about.  */
 
-      switch (SYMBOL_CLASS (sym))
-	{
-	case LOC_ARG:
-	case LOC_REF_ARG:
-	  {
-	    long current_offset = SYMBOL_VALUE (sym);
-	    arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
+	  switch (SYMBOL_CLASS (sym))
+	    {
+	    case LOC_ARG:
+	    case LOC_REF_ARG:
+	      {
+		long current_offset = SYMBOL_VALUE (sym);
+		arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
 
-	    /* Compute address of next argument by adding the size of
-	       this argument and rounding to an int boundary.  */
-	    current_offset =
-	      ((current_offset + arg_size + sizeof (int) - 1)
-		 & ~(sizeof (int) - 1));
-
-	    /* If this is the highest offset seen yet, set highest_offset.  */
-	    if (highest_offset == -1
-		|| (current_offset > highest_offset))
-	      highest_offset = current_offset;
+		/* Compute address of next argument by adding the size of
+		   this argument and rounding to an int boundary.  */
+		current_offset =
+		  ((current_offset + arg_size + sizeof (int) - 1)
+		   & ~(sizeof (int) - 1));
+
+		/* If this is the highest offset seen yet, set highest_offset.  */
+		if (highest_offset == -1
+		    || (current_offset > highest_offset))
+		  highest_offset = current_offset;
 
-	    /* Add the number of ints we're about to print to args_printed.  */
-	    args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
-	  }
+		/* Add the number of ints we're about to print to args_printed.  */
+		args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
+	      }
 
-	  /* We care about types of symbols, but don't need to keep track of
-	     stack offsets in them.  */
-	case LOC_REGPARM:
-	case LOC_REGPARM_ADDR:
-	case LOC_LOCAL_ARG:
-	case LOC_BASEREG_ARG:
-	  break;
-
-	  /* Other types of symbols we just skip over.  */
-	default:
-	  continue;
-	}
+	      /* We care about types of symbols, but don't need to keep track of
+		 stack offsets in them.  */
+	    case LOC_REGPARM:
+	    case LOC_REGPARM_ADDR:
+	    case LOC_LOCAL_ARG:
+	    case LOC_BASEREG_ARG:
+	      break;
+
+	    /* Other types of symbols we just skip over.  */
+	    default:
+	      continue;
+	    }
 
-      /* We have to look up the symbol because arguments can have
-         two entries (one a parameter, one a local) and the one we
-         want is the local, which lookup_symbol will find for us.
-         This includes gcc1 (not gcc2) on the sparc when passing a
-         small structure and gcc2 when the argument type is float
-         and it is passed as a double and converted to float by
-         the prologue (in the latter case the type of the LOC_ARG
-         symbol is double and the type of the LOC_LOCAL symbol is
-         float).  */
-      /* But if the parameter name is null, don't try it.
-         Null parameter names occur on the RS/6000, for traceback tables.
-         FIXME, should we even print them?  */
+	  /* We have to look up the symbol because arguments can have
+	     two entries (one a parameter, one a local) and the one we
+	     want is the local, which lookup_symbol will find for us.
+	     This includes gcc1 (not gcc2) on the sparc when passing a
+	     small structure and gcc2 when the argument type is float
+	     and it is passed as a double and converted to float by
+	     the prologue (in the latter case the type of the LOC_ARG
+	     symbol is double and the type of the LOC_LOCAL symbol is
+	     float).  */
+	  /* But if the parameter name is null, don't try it.
+	     Null parameter names occur on the RS/6000, for traceback tables.
+	     FIXME, should we even print them?  */
 
-      if (*SYMBOL_NAME (sym))
-	{
-	  struct symbol *nsym;
-	  nsym = lookup_symbol
-	    (SYMBOL_NAME (sym),
-	     b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
-	  if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
+	  if (*SYMBOL_NAME (sym))
 	    {
-	      /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
-	         it was passed on the stack and loaded into a register,
-	         or passed in a register and stored in a stack slot.
-	         GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
-
-	         Reasons for using the LOC_ARG:
-	         (1) because find_saved_registers may be slow for remote
-	         debugging,
-	         (2) because registers are often re-used and stack slots
-	         rarely (never?) are.  Therefore using the stack slot is
-	         much less likely to print garbage.
-
-	         Reasons why we might want to use the LOC_REGISTER:
-	         (1) So that the backtrace prints the same value as
-	         "print foo".  I see no compelling reason why this needs
-	         to be the case; having the backtrace print the value which
-	         was passed in, and "print foo" print the value as modified
-	         within the called function, makes perfect sense to me.
-
-	         Additional note:  It might be nice if "info args" displayed
-	         both values.
-	         One more note:  There is a case with sparc structure passing
-	         where we need to use the LOC_REGISTER, but this is dealt with
-	         by creating a single LOC_REGPARM in symbol reading.  */
-
-	      /* Leave sym (the LOC_ARG) alone.  */
-	      ;
+	      struct symbol *nsym;
+	      nsym = lookup_symbol
+		(SYMBOL_NAME (sym),
+		 b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
+	      if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
+		{
+		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
+		     it was passed on the stack and loaded into a register,
+		     or passed in a register and stored in a stack slot.
+		     GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
+
+		     Reasons for using the LOC_ARG:
+		     (1) because find_saved_registers may be slow for remote
+		     debugging,
+		     (2) because registers are often re-used and stack slots
+		     rarely (never?) are.  Therefore using the stack slot is
+		     much less likely to print garbage.
+
+		     Reasons why we might want to use the LOC_REGISTER:
+		     (1) So that the backtrace prints the same value as
+		     "print foo".  I see no compelling reason why this needs
+		     to be the case; having the backtrace print the value which
+		     was passed in, and "print foo" print the value as modified
+		     within the called function, makes perfect sense to me.
+
+		     Additional note:  It might be nice if "info args" displayed
+		     both values.
+		     One more note:  There is a case with sparc structure passing
+		     where we need to use the LOC_REGISTER, but this is dealt with
+		     by creating a single LOC_REGPARM in symbol reading.  */
+
+		  /* Leave sym (the LOC_ARG) alone.  */
+		  ;
+		}
+	      else
+		sym = nsym;
 	    }
-	  else
-	    sym = nsym;
-	}
 
 #ifdef UI_OUT
-      /* Print the current arg.  */
-      if (!first)
-	ui_out_text (uiout, ", ");
-      ui_out_wrap_hint (uiout, "    ");
-
-      annotate_arg_begin ();
-
-      list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-      fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
-			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
-      ui_out_field_stream (uiout, "name", stb);
-      annotate_arg_name_end ();
-      ui_out_text (uiout, "=");
+	  /* Print the current arg.  */
+	  if (!first)
+	    ui_out_text (uiout, ", ");
+	  ui_out_wrap_hint (uiout, "    ");
+
+	  annotate_arg_begin ();
+
+	  list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+	  fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
+				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
+	  ui_out_field_stream (uiout, "name", stb);
+	  annotate_arg_name_end ();
+	  ui_out_text (uiout, "=");
 #else
-      /* Print the current arg.  */
-      if (!first)
-	fprintf_filtered (stream, ", ");
-      wrap_here ("    ");
-
-      annotate_arg_begin ();
-
-      fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
-			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
-      annotate_arg_name_end ();
-      fputs_filtered ("=", stream);
+	  /* Print the current arg.  */
+	  if (!first)
+	    fprintf_filtered (stream, ", ");
+	  wrap_here ("    ");
+
+	  annotate_arg_begin ();
+
+	  fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
+				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
+	  annotate_arg_name_end ();
+	  fputs_filtered ("=", stream);
 #endif
 
-      /* Avoid value_print because it will deref ref parameters.  We just
-         want to print their addresses.  Print ??? for args whose address
-         we do not know.  We pass 2 as "recurse" to val_print because our
-         standard indentation here is 4 spaces, and val_print indents
-         2 for each recurse.  */
-      val = read_var_value (sym, fi);
+	  /* Avoid value_print because it will deref ref parameters.  We just
+	     want to print their addresses.  Print ??? for args whose address
+	     we do not know.  We pass 2 as "recurse" to val_print because our
+	     standard indentation here is 4 spaces, and val_print indents
+	     2 for each recurse.  */
+	  val = read_var_value (sym, fi);
 
-      annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
+	  annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
 
-      if (val)
-	{
+	  if (val)
+	    {
 #ifdef UI_OUT
-	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
-		     VALUE_ADDRESS (val),
-		     stb->stream, 0, 0, 2, Val_no_prettyprint);
-	  ui_out_field_stream (uiout, "value", stb);
-	}
-      else
-	ui_out_text (uiout, "???");
+	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
+			 VALUE_ADDRESS (val),
+			 stb->stream, 0, 0, 2, Val_no_prettyprint);
+	      ui_out_field_stream (uiout, "value", stb);
+	    }
+	  else
+	    ui_out_text (uiout, "???");
 
-      /* Invoke ui_out_tuple_end.  */
-      do_cleanups (list_chain);
+	  /* Invoke ui_out_tuple_end.  */
+	  do_cleanups (list_chain);
 #else
-	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
-		     VALUE_ADDRESS (val),
-		     stream, 0, 0, 2, Val_no_prettyprint);
-	}
-      else
-	fputs_filtered ("???", stream);
+	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
+			 VALUE_ADDRESS (val),
+			 stream, 0, 0, 2, Val_no_prettyprint);
+	    }
+	  else
+	    fputs_filtered ("???", stream);
 #endif
 
-      annotate_arg_end ();
+	  annotate_arg_end ();
 
-      first = 0;
+	  first = 0;
+	}
     }
 
   /* Don't print nameless args in situations where we don't know
Index: gdb/stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.22
diff -u -p -r1.22 stack.c
--- stack.c	2001/07/14 18:59:07	1.22
+++ stack.c	2001/10/01 22:20:55
@@ -1207,16 +1207,12 @@ static int
 print_block_frame_locals (struct block *b, register struct frame_info *fi,
 			  int num_tabs, register struct ui_file *stream)
 {
-  int nsyms;
   register int i, j;
   register struct symbol *sym;
   register int values_printed = 0;
 
-  nsyms = BLOCK_NSYMS (b);
-
-  for (i = 0; i < nsyms; i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       switch (SYMBOL_CLASS (sym))
 	{
 	case LOC_LOCAL:
@@ -1246,16 +1242,12 @@ static int
 print_block_frame_labels (struct block *b, int *have_default,
 			  register struct ui_file *stream)
 {
-  int nsyms;
   register int i;
   register struct symbol *sym;
   register int values_printed = 0;
-
-  nsyms = BLOCK_NSYMS (b);
 
-  for (i = 0; i < nsyms; i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       if (STREQ (SYMBOL_NAME (sym), "default"))
 	{
 	  if (*have_default)
@@ -1432,7 +1424,6 @@ print_frame_arg_vars (register struct fr
 {
   struct symbol *func = get_frame_function (fi);
   register struct block *b;
-  int nsyms;
   register int i;
   register struct symbol *sym, *sym2;
   register int values_printed = 0;
@@ -1444,11 +1435,8 @@ print_frame_arg_vars (register struct fr
     }
 
   b = SYMBOL_BLOCK_VALUE (func);
-  nsyms = BLOCK_NSYMS (b);
-
-  for (i = 0; i < nsyms; i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       switch (SYMBOL_CLASS (sym))
 	{
 	case LOC_ARG:
@@ -1483,7 +1471,6 @@ print_frame_arg_vars (register struct fr
 	  break;
 	}
     }
-
   if (!values_printed)
     {
       fprintf_filtered (stream, "No arguments.\n");
Index: gdb/symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.5
diff -u -p -r1.5 symmisc.c
--- symmisc.c	2001/03/06 08:21:17	1.5
+++ symmisc.c	2001/10/01 22:20:56
@@ -410,6 +416,7 @@ dump_symtab (struct objfile *objfile, st
   int len, blen;
   register struct linetable *l;
   struct blockvector *bv;
+  struct symbol *sym;
   register struct block *b;
   int depth;
 
@@ -471,11 +478,12 @@ dump_symtab (struct objfile *objfile, st
 	  if (BLOCK_GCC_COMPILED (b))
 	    fprintf_filtered (outfile, ", compiled with gcc%d", BLOCK_GCC_COMPILED (b));
 	  fprintf_filtered (outfile, "\n");
-	  /* Now print each symbol in this block */
-	  for (j = 0; j < blen; j++)
+	  /* Now print each symbol in this block.  */
+	  /* FIXMED: Sort?  */
+	  ALL_BLOCK_SYMBOLS (b, j, sym)
 	    {
 	      struct print_symbol_args s;
-	      s.symbol = BLOCK_SYM (b, j);
+	      s.symbol = sym;
 	      s.depth = depth + 1;
 	      s.outfile = outfile;
 	      catch_errors (print_symbol, &s, "Error printing symbol:\n",
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.42
diff -u -p -r1.42 symtab.c
--- symtab.c	2001/07/07 17:19:50	1.42
+++ symtab.c	2001/10/01 22:20:57
@@ -3030,9 +3051,8 @@ make_symbol_completion_list (char *text,
       /* Also catch fields of types defined in this places which match our
          text string.  Only complete on types visible from current context. */
 
-      for (i = 0; i < BLOCK_NSYMS (b); i++)
+      ALL_BLOCK_SYMBOLS (b, i, sym)
 	{
-	  sym = BLOCK_SYM (b, i);
 	  COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 	  if (SYMBOL_CLASS (sym) == LOC_TYPEDEF)
 	    {
@@ -3061,23 +3081,22 @@ make_symbol_completion_list (char *text,
   {
     QUIT;
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
       }
   }
 
   ALL_SYMTABS (objfile, s)
   {
+    struct symbol *sym;
     QUIT;
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
     /* Don't do this block twice.  */
     if (b == surrounding_static_block)
       continue;
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
       }
   }
@@ -3181,16 +3200,14 @@ make_file_symbol_completion_list (char *
      symbols which match.  */
 
   b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-  for (i = 0; i < BLOCK_NSYMS (b); i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
     }
 
   b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
-  for (i = 0; i < BLOCK_NSYMS (b); i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
     }
 
@@ -3568,9 +3585,8 @@ make_symbol_overload_list (struct symbol
       /* Also catch fields of types defined in this places which match our
          text string.  Only complete on types visible from current context. */
 
-      for (i = 0; i < BLOCK_NSYMS (b); i++)
+      ALL_BLOCK_SYMBOLS (b, i, sym)
 	{
-	  sym = BLOCK_SYM (b, i);
 	  overload_list_add_symbol (sym, oload_name);
 	}
     }
@@ -3582,9 +3598,8 @@ make_symbol_overload_list (struct symbol
   {
     QUIT;
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	overload_list_add_symbol (sym, oload_name);
       }
   }
@@ -3596,9 +3611,8 @@ make_symbol_overload_list (struct symbol
     /* Don't do this block twice.  */
     if (b == surrounding_static_block)
       continue;
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	overload_list_add_symbol (sym, oload_name);
       }
   }
Index: gdb/symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.24
diff -u -p -r1.24 symtab.h
--- symtab.h	Tue Oct  9 11:21:41 2001
+++ symtab.h	Tue Oct  9 11:23:14 2001
@@ -467,6 +467,14 @@
 #define BLOCK_SUPERBLOCK(bl)	(bl)->superblock
 #define BLOCK_GCC_COMPILED(bl)	(bl)->gcc_compile_flag
 
+/* Macro to loop through all symbols in a block BL.
+   i counts which symbol we are looking at, and sym points to the current
+   symbol.  */
+#define ALL_BLOCK_SYMBOLS(bl, i, sym)			\
+	for ((i) = 0, (sym) = BLOCK_SYM ((bl), (i));	\
+	     (i) < BLOCK_NSYMS ((bl));			\
+	     ++(i), (sym) = BLOCK_SYM ((bl), (i)))
+
 /* Nonzero if symbols of block BL should be sorted alphabetically.
    Don't sort a block which corresponds to a function.  If we did the
    sorting would have to preserve the order of the symbols for the
Index: gdb/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.26
diff -u -p -r1.26 tracepoint.c
--- tracepoint.c	2001/08/23 20:31:36	1.26
+++ tracepoint.c	2001/10/01 22:20:59
@@ -1296,16 +1296,14 @@ add_local_symbols (struct collection_lis
 {
   struct symbol *sym;
   struct block *block;
-  int i, nsyms, count = 0;
+  int i, count = 0;
 
   block = block_for_pc (pc);
   while (block != 0)
     {
       QUIT;			/* allow user to bail out with ^C */
-      nsyms = BLOCK_NSYMS (block);
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (block, i, sym)
 	{
-	  sym = BLOCK_SYM (block, i);
 	  switch (SYMBOL_CLASS (sym))
 	    {
 	    default:
@@ -2335,7 +2333,7 @@ scope_info (char *args, int from_tty)
   struct minimal_symbol *msym;
   struct block *block;
   char **canonical, *symname, *save_args = args;
-  int i, j, nsyms, count = 0;
+  int i, j, count = 0;
 
   if (args == 0 || *args == 0)
     error ("requires an argument (function, line or *addr) to define a scope");
@@ -2351,14 +2349,13 @@ scope_info (char *args, int from_tty)
   while (block != 0)
     {
       QUIT;			/* allow user to bail out with ^C */
-      nsyms = BLOCK_NSYMS (block);
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (block, i, sym)
 	{
 	  QUIT;			/* allow user to bail out with ^C */
 	  if (count == 0)
 	    printf_filtered ("Scope for %s:\n", save_args);
 	  count++;
-	  sym = BLOCK_SYM (block, i);
+
 	  symname = SYMBOL_NAME (sym);
 	  if (symname == NULL || *symname == '\0')
 	    continue;		/* probably botched, certainly useless */

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Daniel,
Thanks so much for doing this. It makes it so much easier.

Yes, I looked ths over and it seems to work, except that I would really
prefer the change to printcmd.c split in two. The first bit to
rationalize that "if (func)..."  code. This would have with it all
the indentation changes as well. The code as it is now doesn't really
make much sense. So, that looks a good change to me. But it has nothing
to do with the new macro.  After that change is in, you can introduce
the macro in printcmd.c w/o having all the indent changes.
It also makes it easier to distinguish a no-op change (the macro) from
the other one.

The cahnge is printcmd.c needs to delete also the 
	  sym = BLOCK_SYM (b, i);
line.

[Note that I don't maintain printcmd.c, so, I should just shut up :-)]

I applied your patch as is to my sources, and did a grep for BLOCK_SYM,
and found a few more for loops that could be converted:


This one in buildsym.c:
line ~280:

	  struct symbol *sym;
	  for (i = 0; i < BLOCK_NSYMS (block); i++)
	    {
	      sym = BLOCK_SYM (block, i);

And this one in symtab.c:
line ~1500:

	top = BLOCK_NSYMS (block);
	for (bot = 0; bot < top; bot++)
	  {
	    sym = BLOCK_SYM (block, bot);

Another one is in mi/mi-stack-cmd.c.
There are some also in gdbtk/generic/gdbtk-cmds.c and gdbtk-stack.c.

Some tricky ones are left, for a second pass. In mdebugread.c: it is
actually a while loop, in mylookup_symbol, similarly in coffread.c:
patch_opaque_types(), the for loop is reversed.
(Were these in your original patch? I don't remember).

I cannot spot any others, can you?

Elena

Daniel Jacobowitz writes:
 > This patch changes no functionality; it replaces for loops with an
 > ALL_BLOCK_SYMBOLS macro in places where it is entirely unambiguous to do so. 
 > I consider it fairly obvious, but I would greatly appreciate another set of
 > eyes, since I touch a fair amount of code.  The large printcmd changes are
 > (should be?) just indentation updates.
 > 
 > Is this OK?
 > 
 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2001-10-01  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* symtab.h (struct block): (ALL_BLOCK_SYMBOLS): New macro.
 > 
 > 	* symtab.c (find_pc_sect_symtab): Use ALL_BLOCK_SYMBOLS.
 > 	(make_symbol_completion_list): Likewise.
 > 	(make_symbol_overload_list): Likewise.
 > 	* symmisc.c (dump_symtab): Likewise.
 > 	* breakpoint.c (get_catch_sals):  Likewise.
 > 	* objfiles.c (objfile_relocate): Likewise.
 > 	* stack.c (print_block_frame_locals): Likewise.
 > 	(print_block_frame_labels): Likewise.
 > 	(print_frame_arg_vars): Likewise.
 > 	* tracepoint.c (add_local_symbols): Likewise.
 > 	(scope_info): Likewise.
 > 	* printcmd.c (print_frame_args): Likewise.
 > 
 > Index: gdb/breakpoint.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/breakpoint.c,v
 > retrieving revision 1.53
 > diff -u -p -r1.53 breakpoint.c
 > --- breakpoint.c	2001/09/18 05:00:48	1.53
 > +++ breakpoint.c	2001/10/01 22:20:38
 > @@ -5871,15 +5871,11 @@ get_catch_sals (int this_level_only)
 >  	  if (blocks_searched[index] == 0)
 >  	    {
 >  	      struct block *b = BLOCKVECTOR_BLOCK (bl, index);
 > -	      int nsyms;
 >  	      register int i;
 >  	      register struct symbol *sym;
 >  
 > -	      nsyms = BLOCK_NSYMS (b);
 > -
 > -	      for (i = 0; i < nsyms; i++)
 > +	      ALL_BLOCK_SYMBOLS (b, i, sym)
 >  		{
 > -		  sym = BLOCK_SYM (b, i);
 >  		  if (STREQ (SYMBOL_NAME (sym), "default"))
 >  		    {
 >  		      if (have_default)
 > Index: gdb/objfiles.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/objfiles.c,v
 > retrieving revision 1.16
 > diff -u -p -r1.16 objfiles.c
 > --- objfiles.c	2001/07/05 21:32:39	1.16
 > +++ objfiles.c	2001/10/01 22:20:53
 > @@ -557,16 +557,15 @@ objfile_relocate (struct objfile *objfil
 >        for (i = 0; i < BLOCKVECTOR_NBLOCKS (bv); ++i)
 >  	{
 >  	  struct block *b;
 > +	  struct symbol *sym;
 >  	  int j;
 >  
 >  	  b = BLOCKVECTOR_BLOCK (bv, i);
 >  	  BLOCK_START (b) += ANOFFSET (delta, s->block_line_section);
 >  	  BLOCK_END (b) += ANOFFSET (delta, s->block_line_section);
 >  
 > -	  for (j = 0; j < BLOCK_NSYMS (b); ++j)
 > +	  ALL_BLOCK_SYMBOLS (b, j, sym)
 >  	    {
 > -	      struct symbol *sym = BLOCK_SYM (b, j);
 > -
 >  	      fixup_symbol_section (sym, objfile);
 >  
 >  	      /* The RS6000 code from which this was taken skipped
 > Index: gdb/printcmd.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/printcmd.c,v
 > retrieving revision 1.27
 > diff -u -p -r1.27 printcmd.c
 > --- printcmd.c	2001/09/12 04:18:08	1.27
 > +++ printcmd.c	2001/10/01 22:20:54
 > @@ -1806,168 +1806,168 @@ print_frame_args (struct symbol *func, s
 >    if (func)
 >      {
 >        b = SYMBOL_BLOCK_VALUE (func);
 >        nsyms = BLOCK_NSYMS (b);
 > -    }
 >  
 > -  for (i = 0; i < nsyms; i++)
 > -    {
 > -      QUIT;
 > -      sym = BLOCK_SYM (b, i);
 > +      ALL_BLOCK_SYMBOLS (b, i, sym)
 > +        {
 > +	  QUIT;
 > +	  sym = BLOCK_SYM (b, i);
 >  
 > -      /* Keep track of the highest stack argument offset seen, and
 > -         skip over any kinds of symbols we don't care about.  */
 > +	  /* Keep track of the highest stack argument offset seen, and
 > +	     skip over any kinds of symbols we don't care about.  */
 >  
 > -      switch (SYMBOL_CLASS (sym))
 > -	{
 > -	case LOC_ARG:
 > -	case LOC_REF_ARG:
 > -	  {
 > -	    long current_offset = SYMBOL_VALUE (sym);
 > -	    arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
 > +	  switch (SYMBOL_CLASS (sym))
 > +	    {
 > +	    case LOC_ARG:
 > +	    case LOC_REF_ARG:
 > +	      {
 > +		long current_offset = SYMBOL_VALUE (sym);
 > +		arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
 >  
 > -	    /* Compute address of next argument by adding the size of
 > -	       this argument and rounding to an int boundary.  */
 > -	    current_offset =
 > -	      ((current_offset + arg_size + sizeof (int) - 1)
 > -		 & ~(sizeof (int) - 1));
 > -
 > -	    /* If this is the highest offset seen yet, set highest_offset.  */
 > -	    if (highest_offset == -1
 > -		|| (current_offset > highest_offset))
 > -	      highest_offset = current_offset;
 > +		/* Compute address of next argument by adding the size of
 > +		   this argument and rounding to an int boundary.  */
 > +		current_offset =
 > +		  ((current_offset + arg_size + sizeof (int) - 1)
 > +		   & ~(sizeof (int) - 1));
 > +
 > +		/* If this is the highest offset seen yet, set highest_offset.  */
 > +		if (highest_offset == -1
 > +		    || (current_offset > highest_offset))
 > +		  highest_offset = current_offset;
 >  
 > -	    /* Add the number of ints we're about to print to args_printed.  */
 > -	    args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
 > -	  }
 > +		/* Add the number of ints we're about to print to args_printed.  */
 > +		args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
 > +	      }
 >  
 > -	  /* We care about types of symbols, but don't need to keep track of
 > -	     stack offsets in them.  */
 > -	case LOC_REGPARM:
 > -	case LOC_REGPARM_ADDR:
 > -	case LOC_LOCAL_ARG:
 > -	case LOC_BASEREG_ARG:
 > -	  break;
 > -
 > -	  /* Other types of symbols we just skip over.  */
 > -	default:
 > -	  continue;
 > -	}
 > +	      /* We care about types of symbols, but don't need to keep track of
 > +		 stack offsets in them.  */
 > +	    case LOC_REGPARM:
 > +	    case LOC_REGPARM_ADDR:
 > +	    case LOC_LOCAL_ARG:
 > +	    case LOC_BASEREG_ARG:
 > +	      break;
 > +
 > +	    /* Other types of symbols we just skip over.  */
 > +	    default:
 > +	      continue;
 > +	    }
 >  
 > -      /* We have to look up the symbol because arguments can have
 > -         two entries (one a parameter, one a local) and the one we
 > -         want is the local, which lookup_symbol will find for us.
 > -         This includes gcc1 (not gcc2) on the sparc when passing a
 > -         small structure and gcc2 when the argument type is float
 > -         and it is passed as a double and converted to float by
 > -         the prologue (in the latter case the type of the LOC_ARG
 > -         symbol is double and the type of the LOC_LOCAL symbol is
 > -         float).  */
 > -      /* But if the parameter name is null, don't try it.
 > -         Null parameter names occur on the RS/6000, for traceback tables.
 > -         FIXME, should we even print them?  */
 > +	  /* We have to look up the symbol because arguments can have
 > +	     two entries (one a parameter, one a local) and the one we
 > +	     want is the local, which lookup_symbol will find for us.
 > +	     This includes gcc1 (not gcc2) on the sparc when passing a
 > +	     small structure and gcc2 when the argument type is float
 > +	     and it is passed as a double and converted to float by
 > +	     the prologue (in the latter case the type of the LOC_ARG
 > +	     symbol is double and the type of the LOC_LOCAL symbol is
 > +	     float).  */
 > +	  /* But if the parameter name is null, don't try it.
 > +	     Null parameter names occur on the RS/6000, for traceback tables.
 > +	     FIXME, should we even print them?  */
 >  
 > -      if (*SYMBOL_NAME (sym))
 > -	{
 > -	  struct symbol *nsym;
 > -	  nsym = lookup_symbol
 > -	    (SYMBOL_NAME (sym),
 > -	     b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
 > -	  if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
 > +	  if (*SYMBOL_NAME (sym))
 >  	    {
 > -	      /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
 > -	         it was passed on the stack and loaded into a register,
 > -	         or passed in a register and stored in a stack slot.
 > -	         GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
 > -
 > -	         Reasons for using the LOC_ARG:
 > -	         (1) because find_saved_registers may be slow for remote
 > -	         debugging,
 > -	         (2) because registers are often re-used and stack slots
 > -	         rarely (never?) are.  Therefore using the stack slot is
 > -	         much less likely to print garbage.
 > -
 > -	         Reasons why we might want to use the LOC_REGISTER:
 > -	         (1) So that the backtrace prints the same value as
 > -	         "print foo".  I see no compelling reason why this needs
 > -	         to be the case; having the backtrace print the value which
 > -	         was passed in, and "print foo" print the value as modified
 > -	         within the called function, makes perfect sense to me.
 > -
 > -	         Additional note:  It might be nice if "info args" displayed
 > -	         both values.
 > -	         One more note:  There is a case with sparc structure passing
 > -	         where we need to use the LOC_REGISTER, but this is dealt with
 > -	         by creating a single LOC_REGPARM in symbol reading.  */
 > -
 > -	      /* Leave sym (the LOC_ARG) alone.  */
 > -	      ;
 > +	      struct symbol *nsym;
 > +	      nsym = lookup_symbol
 > +		(SYMBOL_NAME (sym),
 > +		 b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
 > +	      if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
 > +		{
 > +		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
 > +		     it was passed on the stack and loaded into a register,
 > +		     or passed in a register and stored in a stack slot.
 > +		     GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
 > +
 > +		     Reasons for using the LOC_ARG:
 > +		     (1) because find_saved_registers may be slow for remote
 > +		     debugging,
 > +		     (2) because registers are often re-used and stack slots
 > +		     rarely (never?) are.  Therefore using the stack slot is
 > +		     much less likely to print garbage.
 > +
 > +		     Reasons why we might want to use the LOC_REGISTER:
 > +		     (1) So that the backtrace prints the same value as
 > +		     "print foo".  I see no compelling reason why this needs
 > +		     to be the case; having the backtrace print the value which
 > +		     was passed in, and "print foo" print the value as modified
 > +		     within the called function, makes perfect sense to me.
 > +
 > +		     Additional note:  It might be nice if "info args" displayed
 > +		     both values.
 > +		     One more note:  There is a case with sparc structure passing
 > +		     where we need to use the LOC_REGISTER, but this is dealt with
 > +		     by creating a single LOC_REGPARM in symbol reading.  */
 > +
 > +		  /* Leave sym (the LOC_ARG) alone.  */
 > +		  ;
 > +		}
 > +	      else
 > +		sym = nsym;
 >  	    }
 > -	  else
 > -	    sym = nsym;
 > -	}
 >  
 >  #ifdef UI_OUT
 > -      /* Print the current arg.  */
 > -      if (!first)
 > -	ui_out_text (uiout, ", ");
 > -      ui_out_wrap_hint (uiout, "    ");
 > -
 > -      annotate_arg_begin ();
 > -
 > -      list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 > -      fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
 > -			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > -      ui_out_field_stream (uiout, "name", stb);
 > -      annotate_arg_name_end ();
 > -      ui_out_text (uiout, "=");
 > +	  /* Print the current arg.  */
 > +	  if (!first)
 > +	    ui_out_text (uiout, ", ");
 > +	  ui_out_wrap_hint (uiout, "    ");
 > +
 > +	  annotate_arg_begin ();
 > +
 > +	  list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 > +	  fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
 > +				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > +	  ui_out_field_stream (uiout, "name", stb);
 > +	  annotate_arg_name_end ();
 > +	  ui_out_text (uiout, "=");
 >  #else
 > -      /* Print the current arg.  */
 > -      if (!first)
 > -	fprintf_filtered (stream, ", ");
 > -      wrap_here ("    ");
 > -
 > -      annotate_arg_begin ();
 > -
 > -      fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
 > -			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > -      annotate_arg_name_end ();
 > -      fputs_filtered ("=", stream);
 > +	  /* Print the current arg.  */
 > +	  if (!first)
 > +	    fprintf_filtered (stream, ", ");
 > +	  wrap_here ("    ");
 > +
 > +	  annotate_arg_begin ();
 > +
 > +	  fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
 > +				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > +	  annotate_arg_name_end ();
 > +	  fputs_filtered ("=", stream);
 >  #endif
 >  
 > -      /* Avoid value_print because it will deref ref parameters.  We just
 > -         want to print their addresses.  Print ??? for args whose address
 > -         we do not know.  We pass 2 as "recurse" to val_print because our
 > -         standard indentation here is 4 spaces, and val_print indents
 > -         2 for each recurse.  */
 > -      val = read_var_value (sym, fi);
 > +	  /* Avoid value_print because it will deref ref parameters.  We just
 > +	     want to print their addresses.  Print ??? for args whose address
 > +	     we do not know.  We pass 2 as "recurse" to val_print because our
 > +	     standard indentation here is 4 spaces, and val_print indents
 > +	     2 for each recurse.  */
 > +	  val = read_var_value (sym, fi);
 >  
 > -      annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
 > +	  annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
 >  
 > -      if (val)
 > -	{
 > +	  if (val)
 > +	    {
 >  #ifdef UI_OUT
 > -	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > -		     VALUE_ADDRESS (val),
 > -		     stb->stream, 0, 0, 2, Val_no_prettyprint);
 > -	  ui_out_field_stream (uiout, "value", stb);
 > -	}
 > -      else
 > -	ui_out_text (uiout, "???");
 > +	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > +			 VALUE_ADDRESS (val),
 > +			 stb->stream, 0, 0, 2, Val_no_prettyprint);
 > +	      ui_out_field_stream (uiout, "value", stb);
 > +	    }
 > +	  else
 > +	    ui_out_text (uiout, "???");
 >  
 > -      /* Invoke ui_out_tuple_end.  */
 > -      do_cleanups (list_chain);
 > +	  /* Invoke ui_out_tuple_end.  */
 > +	  do_cleanups (list_chain);
 >  #else
 > -	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > -		     VALUE_ADDRESS (val),
 > -		     stream, 0, 0, 2, Val_no_prettyprint);
 > -	}
 > -      else
 > -	fputs_filtered ("???", stream);
 > +	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > +			 VALUE_ADDRESS (val),
 > +			 stream, 0, 0, 2, Val_no_prettyprint);
 > +	    }
 > +	  else
 > +	    fputs_filtered ("???", stream);
 >  #endif
 >  
 > -      annotate_arg_end ();
 > +	  annotate_arg_end ();
 >  
 > -      first = 0;
 > +	  first = 0;
 > +	}
 >      }
 >  
 >    /* Don't print nameless args in situations where we don't know
 > Index: gdb/stack.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/stack.c,v
 > retrieving revision 1.22
 > diff -u -p -r1.22 stack.c
 > --- stack.c	2001/07/14 18:59:07	1.22
 > +++ stack.c	2001/10/01 22:20:55
 > @@ -1207,16 +1207,12 @@ static int
 >  print_block_frame_locals (struct block *b, register struct frame_info *fi,
 >  			  int num_tabs, register struct ui_file *stream)
 >  {
 > -  int nsyms;
 >    register int i, j;
 >    register struct symbol *sym;
 >    register int values_printed = 0;
 >  
 > -  nsyms = BLOCK_NSYMS (b);
 > -
 > -  for (i = 0; i < nsyms; i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        switch (SYMBOL_CLASS (sym))
 >  	{
 >  	case LOC_LOCAL:
 > @@ -1246,16 +1242,12 @@ static int
 >  print_block_frame_labels (struct block *b, int *have_default,
 >  			  register struct ui_file *stream)
 >  {
 > -  int nsyms;
 >    register int i;
 >    register struct symbol *sym;
 >    register int values_printed = 0;
 > -
 > -  nsyms = BLOCK_NSYMS (b);
 >  
 > -  for (i = 0; i < nsyms; i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        if (STREQ (SYMBOL_NAME (sym), "default"))
 >  	{
 >  	  if (*have_default)
 > @@ -1432,7 +1424,6 @@ print_frame_arg_vars (register struct fr
 >  {
 >    struct symbol *func = get_frame_function (fi);
 >    register struct block *b;
 > -  int nsyms;
 >    register int i;
 >    register struct symbol *sym, *sym2;
 >    register int values_printed = 0;
 > @@ -1444,11 +1435,8 @@ print_frame_arg_vars (register struct fr
 >      }
 >  
 >    b = SYMBOL_BLOCK_VALUE (func);
 > -  nsyms = BLOCK_NSYMS (b);
 > -
 > -  for (i = 0; i < nsyms; i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        switch (SYMBOL_CLASS (sym))
 >  	{
 >  	case LOC_ARG:
 > @@ -1483,7 +1471,6 @@ print_frame_arg_vars (register struct fr
 >  	  break;
 >  	}
 >      }
 > -
 >    if (!values_printed)
 >      {
 >        fprintf_filtered (stream, "No arguments.\n");
 > Index: gdb/symmisc.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symmisc.c,v
 > retrieving revision 1.5
 > diff -u -p -r1.5 symmisc.c
 > --- symmisc.c	2001/03/06 08:21:17	1.5
 > +++ symmisc.c	2001/10/01 22:20:56
 > @@ -410,6 +416,7 @@ dump_symtab (struct objfile *objfile, st
 >    int len, blen;
 >    register struct linetable *l;
 >    struct blockvector *bv;
 > +  struct symbol *sym;
 >    register struct block *b;
 >    int depth;
 >  
 > @@ -471,11 +478,12 @@ dump_symtab (struct objfile *objfile, st
 >  	  if (BLOCK_GCC_COMPILED (b))
 >  	    fprintf_filtered (outfile, ", compiled with gcc%d", BLOCK_GCC_COMPILED (b));
 >  	  fprintf_filtered (outfile, "\n");
 > -	  /* Now print each symbol in this block */
 > -	  for (j = 0; j < blen; j++)
 > +	  /* Now print each symbol in this block.  */
 > +	  /* FIXMED: Sort?  */
 > +	  ALL_BLOCK_SYMBOLS (b, j, sym)
 >  	    {
 >  	      struct print_symbol_args s;
 > -	      s.symbol = BLOCK_SYM (b, j);
 > +	      s.symbol = sym;
 >  	      s.depth = depth + 1;
 >  	      s.outfile = outfile;
 >  	      catch_errors (print_symbol, &s, "Error printing symbol:\n",
 > Index: gdb/symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.42
 > diff -u -p -r1.42 symtab.c
 > --- symtab.c	2001/07/07 17:19:50	1.42
 > +++ symtab.c	2001/10/01 22:20:57
 > @@ -3030,9 +3051,8 @@ make_symbol_completion_list (char *text,
 >        /* Also catch fields of types defined in this places which match our
 >           text string.  Only complete on types visible from current context. */
 >  
 > -      for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +      ALL_BLOCK_SYMBOLS (b, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (b, i);
 >  	  COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >  	  if (SYMBOL_CLASS (sym) == LOC_TYPEDEF)
 >  	    {
 > @@ -3061,23 +3081,22 @@ make_symbol_completion_list (char *text,
 >    {
 >      QUIT;
 >      b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >        }
 >    }
 >  
 >    ALL_SYMTABS (objfile, s)
 >    {
 > +    struct symbol *sym;
 >      QUIT;
 >      b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
 >      /* Don't do this block twice.  */
 >      if (b == surrounding_static_block)
 >        continue;
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >        }
 >    }
 > @@ -3181,16 +3200,14 @@ make_file_symbol_completion_list (char *
 >       symbols which match.  */
 >  
 >    b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > -  for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >      }
 >  
 >    b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
 > -  for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >      }
 >  
 > @@ -3568,9 +3585,8 @@ make_symbol_overload_list (struct symbol
 >        /* Also catch fields of types defined in this places which match our
 >           text string.  Only complete on types visible from current context. */
 >  
 > -      for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +      ALL_BLOCK_SYMBOLS (b, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (b, i);
 >  	  overload_list_add_symbol (sym, oload_name);
 >  	}
 >      }
 > @@ -3582,9 +3598,8 @@ make_symbol_overload_list (struct symbol
 >    {
 >      QUIT;
 >      b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	overload_list_add_symbol (sym, oload_name);
 >        }
 >    }
 > @@ -3596,9 +3611,8 @@ make_symbol_overload_list (struct symbol
 >      /* Don't do this block twice.  */
 >      if (b == surrounding_static_block)
 >        continue;
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	overload_list_add_symbol (sym, oload_name);
 >        }
 >    }
 > Index: gdb/symtab.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.h,v
 > retrieving revision 1.24
 > diff -u -p -r1.24 symtab.h
 > --- symtab.h	Tue Oct  9 11:21:41 2001
 > +++ symtab.h	Tue Oct  9 11:23:14 2001
 > @@ -467,6 +467,14 @@
 >  #define BLOCK_SUPERBLOCK(bl)	(bl)->superblock
 >  #define BLOCK_GCC_COMPILED(bl)	(bl)->gcc_compile_flag
 >  
 > +/* Macro to loop through all symbols in a block BL.
 > +   i counts which symbol we are looking at, and sym points to the current
 > +   symbol.  */
 > +#define ALL_BLOCK_SYMBOLS(bl, i, sym)			\
 > +	for ((i) = 0, (sym) = BLOCK_SYM ((bl), (i));	\
 > +	     (i) < BLOCK_NSYMS ((bl));			\
 > +	     ++(i), (sym) = BLOCK_SYM ((bl), (i)))
 > +
 >  /* Nonzero if symbols of block BL should be sorted alphabetically.
 >     Don't sort a block which corresponds to a function.  If we did the
 >     sorting would have to preserve the order of the symbols for the
 > Index: gdb/tracepoint.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/tracepoint.c,v
 > retrieving revision 1.26
 > diff -u -p -r1.26 tracepoint.c
 > --- tracepoint.c	2001/08/23 20:31:36	1.26
 > +++ tracepoint.c	2001/10/01 22:20:59
 > @@ -1296,16 +1296,14 @@ add_local_symbols (struct collection_lis
 >  {
 >    struct symbol *sym;
 >    struct block *block;
 > -  int i, nsyms, count = 0;
 > +  int i, count = 0;
 >  
 >    block = block_for_pc (pc);
 >    while (block != 0)
 >      {
 >        QUIT;			/* allow user to bail out with ^C */
 > -      nsyms = BLOCK_NSYMS (block);
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (block, i);
 >  	  switch (SYMBOL_CLASS (sym))
 >  	    {
 >  	    default:
 > @@ -2335,7 +2333,7 @@ scope_info (char *args, int from_tty)
 >    struct minimal_symbol *msym;
 >    struct block *block;
 >    char **canonical, *symname, *save_args = args;
 > -  int i, j, nsyms, count = 0;
 > +  int i, j, count = 0;
 >  
 >    if (args == 0 || *args == 0)
 >      error ("requires an argument (function, line or *addr) to define a scope");
 > @@ -2351,14 +2349,13 @@ scope_info (char *args, int from_tty)
 >    while (block != 0)
 >      {
 >        QUIT;			/* allow user to bail out with ^C */
 > -      nsyms = BLOCK_NSYMS (block);
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	{
 >  	  QUIT;			/* allow user to bail out with ^C */
 >  	  if (count == 0)
 >  	    printf_filtered ("Scope for %s:\n", save_args);
 >  	  count++;
 > -	  sym = BLOCK_SYM (block, i);
 > +
 >  	  symname = SYMBOL_NAME (sym);
 >  	  if (symname == NULL || *symname == '\0')
 >  	    continue;		/* probably botched, certainly useless */

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Berlin]

On Thursday, October 11, 2001, at 07:46  PM, Elena Zannoni wrote:

>
> Daniel,
> Thanks so much for doing this. It makes it so much easier.
>
> Yes, I looked ths over and it seems to work, except that I would really
> prefer the change to printcmd.c split in two. The first bit to
> rationalize that "if (func)..."  code. This would have with it all
> the indentation changes as well. The code as it is now doesn't really
> make much sense. So, that looks a good change to me. But it has nothing
> to do with the new macro.  After that change is in, you can introduce
> the macro in printcmd.c w/o having all the indent changes.
> It also makes it easier to distinguish a no-op change (the macro) from
> the other one.
>
> The cahnge is printcmd.c needs to delete also the
> 	  sym = BLOCK_SYM (b, i);
> line.
>
> [Note that I don't maintain printcmd.c, so, I should just shut up :-)]
>
> I applied your patch as is to my sources, and did a grep for BLOCK_SYM,
> and found a few more for loops that could be converted:
>
>
> This one in buildsym.c:
> line ~280:
>
> 	  struct symbol *sym;
> 	  for (i = 0; i < BLOCK_NSYMS (block); i++)
> 	    {
> 	      sym = BLOCK_SYM (block, i);
>
> And this one in symtab.c:
> line ~1500:
>
> 	top = BLOCK_NSYMS (block);
> 	for (bot = 0; bot < top; bot++)
> 	  {
> 	    sym = BLOCK_SYM (block, bot);
>

Both of these loops get changed by the next patch i'm sure he'll submit, 
to introduce hashed blocks, which is probably why he didn't change them.

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Daniel Berlin writes:
 > 
 > On Thursday, October 11, 2001, at 07:46  PM, Elena Zannoni wrote:
 > 
 > >
 > > Daniel,
 > > Thanks so much for doing this. It makes it so much easier.
 > >
 > > Yes, I looked ths over and it seems to work, except that I would really
 > > prefer the change to printcmd.c split in two. The first bit to
 > > rationalize that "if (func)..."  code. This would have with it all
 > > the indentation changes as well. The code as it is now doesn't really
 > > make much sense. So, that looks a good change to me. But it has nothing
 > > to do with the new macro.  After that change is in, you can introduce
 > > the macro in printcmd.c w/o having all the indent changes.
 > > It also makes it easier to distinguish a no-op change (the macro) from
 > > the other one.
 > >
 > > The cahnge is printcmd.c needs to delete also the
 > > 	  sym = BLOCK_SYM (b, i);
 > > line.
 > >
 > > [Note that I don't maintain printcmd.c, so, I should just shut up :-)]
 > >
 > > I applied your patch as is to my sources, and did a grep for BLOCK_SYM,
 > > and found a few more for loops that could be converted:
 > >
 > >
 > > This one in buildsym.c:
 > > line ~280:
 > >
 > > 	  struct symbol *sym;
 > > 	  for (i = 0; i < BLOCK_NSYMS (block); i++)
 > > 	    {
 > > 	      sym = BLOCK_SYM (block, i);
 > >
 > > And this one in symtab.c:
 > > line ~1500:
 > >
 > > 	top = BLOCK_NSYMS (block);
 > > 	for (bot = 0; bot < top; bot++)
 > > 	  {
 > > 	    sym = BLOCK_SYM (block, bot);
 > >
 > 
 > Both of these loops get changed by the next patch i'm sure he'll submit, 
 > to introduce hashed blocks, which is probably why he didn't change them.

Ok, thanks!

Elena

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

On Thu, Oct 11, 2001 at 07:46:33PM -0400, Elena Zannoni wrote:
> 
> Daniel,
> Thanks so much for doing this. It makes it so much easier.
> 
> Yes, I looked ths over and it seems to work, except that I would really
> prefer the change to printcmd.c split in two. The first bit to
> rationalize that "if (func)..."  code. This would have with it all
> the indentation changes as well. The code as it is now doesn't really
> make much sense. So, that looks a good change to me. But it has nothing
> to do with the new macro.  After that change is in, you can introduce
> the macro in printcmd.c w/o having all the indent changes.
> It also makes it easier to distinguish a no-op change (the macro) from
> the other one.

OK.  Would you prefer I resubmit this patch broken up further, then? 
I could do that.

There's a double-edged sword here; every patch in this sequence except
for the hashing change is predicated on the previous patches.  So while
I understand that breaking them up does make reviewing much easier,
with the current length of the patch review cycle, every time I
decompose this further I add two or three more days to its eventual
(hopeful) approval.  I'm sure you can understand that it's a little
frustrating.

> The cahnge is printcmd.c needs to delete also the 
> 	  sym = BLOCK_SYM (b, i);
> line.

... that's what I get for moving too fast between trees.  Thanks for noticing.

> 
> [Note that I don't maintain printcmd.c, so, I should just shut up :-)]
> 
> I applied your patch as is to my sources, and did a grep for BLOCK_SYM,
> and found a few more for loops that could be converted:
> 
> 
> This one in buildsym.c:
> line ~280:
> 
> 	  struct symbol *sym;
> 	  for (i = 0; i < BLOCK_NSYMS (block); i++)
> 	    {
> 	      sym = BLOCK_SYM (block, i);

I could change this one too.  It's from a case where the symbols will
never be hashable (i.e. order-sensitive), so I didn't touch it the
first time through.  Consistency is good, though.

> And this one in symtab.c:
> line ~1500:
> 
> 	top = BLOCK_NSYMS (block);
> 	for (bot = 0; bot < top; bot++)
> 	  {
> 	    sym = BLOCK_SYM (block, bot);
> 

And this one's #if 0'd out.  I could fix it (just the ALL_BLOCK_SYMBOLS
change, I mean),  or delete it per the comment above it.

> Another one is in mi/mi-stack-cmd.c.
> There are some also in gdbtk/generic/gdbtk-cmds.c and gdbtk-stack.c.

These I missed; I didn't think to check further down than the top
level.  I'll get them when I repost this, which it looks like I'll need
to do.

> Some tricky ones are left, for a second pass. In mdebugread.c: it is
> actually a while loop, in mylookup_symbol, similarly in coffread.c:
> patch_opaque_types(), the for loop is reversed.
> (Were these in your original patch? I don't remember).

The one in coffread worries me; I can not tell from the comments if it
is order-sensitive or not.  The one in mdebugread does not, because
mdebugread does dastardly things to blocks.  I deliberately left those
unhashed.

> I cannot spot any others, can you?

Nope.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Andrew Cagney]

> 
> OK.  Would you prefer I resubmit this patch broken up further, then? 
> I could do that.
> 
> There's a double-edged sword here; every patch in this sequence except
> for the hashing change is predicated on the previous patches.  So while
> I understand that breaking them up does make reviewing much easier,
> with the current length of the patch review cycle, every time I
> decompose this further I add two or three more days to its eventual
> (hopeful) approval.  I'm sure you can understand that it's a little
> frustrating.

Daniel,

As you said, it is a double-edged sword.  The other edge has a very 
unusual feature.  Identify simple mechanical self contained changes and 
often go in as obvious.  The review cycle goes down and can often be 
reduced to zero.

While this can mean an increased workload for you as an individual it 
does dramatically reduce the work load for the entire GDB community.

My reading of Elena's comment:

> Yes, I looked ths over and it seems to work, except that I would really
> prefer the change to printcmd.c split in two. The first bit to
> rationalize that "if (func)..."  code. This would have with it all
> the indentation changes as well. The code as it is now doesn't really
> make much sense. So, that looks a good change to me. But it has nothing
> to do with the new macro.  After that change is in, you can introduce
> the macro in printcmd.c w/o having all the indent changes.
> It also makes it easier to distinguish a no-op change (the macro) from
> the other one.

is that you're all approved.

Your first commit fixes some messed up logic.  It is a cleanup (but 
pretty obvious).  It doesn't have anything to do with the (ULGH) macro. 
  By keeping it separate it makes it possible to better isolate the 
breakage it could cause when we have to go back (in 6 months) to find a 
bug ;-)

Your second commit is this new (ULGH) macro.  The macro (ULGH) shouldn't 
break anything but it is however still a (ULGH) macro.  Just include the 
extra tweeks you found.

(If you haven't figured it out, breakpoint.h has a similar (ULGH) macro 
so I'm biteing my tongue on this change :-)

Andrew

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Berlin]

>
> Your second commit is this new (ULGH) macro.  The macro (ULGH) 
> shouldn't break anything but it is however still a (ULGH) macro.  Just 
> include the extra tweeks you found.
>
> (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro 
> so I'm biteing my tongue on this change :-)
>
Unfortunately, it can't be done without a macro here, without 
introducing about 30 new functions, spread out across a whole bunch of 
files.
The real problem is so many things feel an uncontrollable urge to walk 
an entire block (or the entire symbol table).

> Andrew
>

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Andrew Cagney]

> Your second commit is this new (ULGH) macro.  The macro (ULGH) shouldn't break anything but it is however still a (ULGH) macro.  Just include the extra tweeks you found.
> 
> (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro so I'm biteing my tongue on this change [:-)]
> 
> Unfortunately, it can't be done without a macro here, without introducing about 30 new functions, spread out across a whole bunch of files.
> The real problem is so many things feel an uncontrollable urge to walk an entire block (or the entire symbol table).


Yes, like I said, I'm biteing my tongue :-)

Andrew

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

On Thu, Oct 11, 2001 at 08:42:41PM -0400, Andrew Cagney wrote:
> As you said, it is a double-edged sword.  The other edge has a very 
> unusual feature.  Identify simple mechanical self contained changes and 
> often go in as obvious.  The review cycle goes down and can often be 
> reduced to zero.

The problem is that I'm working entirely on intrusive changes in code
owned by other people.  There are no parts I'm willing to commit as
obvious, and every time I break them up further I introduce
intermediate stages that I have to adequately test.

> My reading of Elena's comment:
> 
> >Yes, I looked ths over and it seems to work, except that I would really
> >prefer the change to printcmd.c split in two. The first bit to
> >rationalize that "if (func)..."  code. This would have with it all
> >the indentation changes as well. The code as it is now doesn't really
> >make much sense. So, that looks a good change to me. But it has nothing
> >to do with the new macro.  After that change is in, you can introduce
> >the macro in printcmd.c w/o having all the indent changes.
> >It also makes it easier to distinguish a no-op change (the macro) from
> >the other one.
> 
> is that you're all approved.

Well, I need to repost the patch anyway after Elena's comments, so I'll
wait on assuming that.

> Your first commit fixes some messed up logic.  It is a cleanup (but 
> pretty obvious).  It doesn't have anything to do with the (ULGH) macro. 
>  By keeping it separate it makes it possible to better isolate the 
> breakage it could cause when we have to go back (in 6 months) to find a 
> bug ;-)
> 
> Your second commit is this new (ULGH) macro.  The macro (ULGH) shouldn't 
> break anything but it is however still a (ULGH) macro.  Just include the 
> extra tweeks you found.
> 
> (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro 
> so I'm biteing my tongue on this change :-)

Heh.  I wonder what Andrew thinks of macros?

Seriously, there's nothing to be done without adding the complexity of
iterators.  I want these structures to be treated opaquely, damn it. 
For now, macros it is.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Andrew Cagney]

> On Thu, Oct 11, 2001 at 08:42:41PM -0400, Andrew Cagney wrote:
> 
>> As you said, it is a double-edged sword.  The other edge has a very 
>> unusual feature.  Identify simple mechanical self contained changes and 
>> often go in as obvious.  The review cycle goes down and can often be 
>> reduced to zero.
> 
> 
> The problem is that I'm working entirely on intrusive changes in code
> owned by other people.  There are no parts I'm willing to commit as
> obvious, and every time I break them up further I introduce
> intermediate stages that I have to adequately test.

I do this when it is code I maintain.

I'm about to attack target.[hc] and that is going to get real messy. 
The only way I can do it is in two passes.  First prototype the changes 
I want - proving they are possible, and then go back and break the 
change down into digestable chunks so that I can explain them.

As for obvious.  The most common is indentation.  After that comes small 
logic changes - the ``I think this is obvious but I want someone to 
double check'' strategy is often useful.  The last would be mechanical 
changes across files - there pre-approval is a good strategy.  The word 
``obvious'' is like a red flag to a bull, it gets everyones attention 
and is carefully examined :-)

enjoy,
Andrew

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Thu, Oct 11, 2001 at 08:42:41PM -0400, Andrew Cagney wrote:
 > > As you said, it is a double-edged sword.  The other edge has a very 
 > > unusual feature.  Identify simple mechanical self contained changes and 
 > > often go in as obvious.  The review cycle goes down and can often be 
 > > reduced to zero.
 > 
 > The problem is that I'm working entirely on intrusive changes in code
 > owned by other people.  There are no parts I'm willing to commit as
 > obvious, and every time I break them up further I introduce
 > intermediate stages that I have to adequately test.

Yes, true. In the printcmd.c file case though I would think that if
you did a test run with both changes in, the splitting would be ok.

 > 
 > > My reading of Elena's comment:
 > > 
 > > >Yes, I looked ths over and it seems to work, except that I would really
 > > >prefer the change to printcmd.c split in two. The first bit to
 > > >rationalize that "if (func)..."  code. This would have with it all
 > > >the indentation changes as well. The code as it is now doesn't really
 > > >make much sense. So, that looks a good change to me. But it has nothing
 > > >to do with the new macro.  After that change is in, you can introduce
 > > >the macro in printcmd.c w/o having all the indent changes.
 > > >It also makes it easier to distinguish a no-op change (the macro) from
 > > >the other one.
 > > 
 > > is that you're all approved.
 > 
 > Well, I need to repost the patch anyway after Elena's comments, so I'll
 > wait on assuming that.
 > 

Just repost the extra conversions to use the macro that we identified.

 > > Your first commit fixes some messed up logic.  It is a cleanup (but 
 > > pretty obvious).  It doesn't have anything to do with the (ULGH) macro. 
 > >  By keeping it separate it makes it possible to better isolate the 
 > > breakage it could cause when we have to go back (in 6 months) to find a 
 > > bug ;-)
 > > 
 > > Your second commit is this new (ULGH) macro.  The macro (ULGH) shouldn't 
 > > break anything but it is however still a (ULGH) macro.  Just include the 
 > > extra tweeks you found.
 > > 
 > > (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro 
 > > so I'm biteing my tongue on this change :-)
 > 
 > Heh.  I wonder what Andrew thinks of macros?
 > 
 > Seriously, there's nothing to be done without adding the complexity of
 > iterators.  I want these structures to be treated opaquely, damn it. 
 > For now, macros it is.
 > 

Yes, no problem.

Elena


 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Andrew Cagney writes:
 > > 
 > > OK.  Would you prefer I resubmit this patch broken up further, then? 
 > > I could do that.
 > > 
 > > There's a double-edged sword here; every patch in this sequence except
 > > for the hashing change is predicated on the previous patches.  So while
 > > I understand that breaking them up does make reviewing much easier,
 > > with the current length of the patch review cycle, every time I
 > > decompose this further I add two or three more days to its eventual
 > > (hopeful) approval.  I'm sure you can understand that it's a little
 > > frustrating.
 > 
 > Daniel,
 > 
 > As you said, it is a double-edged sword.  The other edge has a very 
 > unusual feature.  Identify simple mechanical self contained changes and 
 > often go in as obvious.  The review cycle goes down and can often be 
 > reduced to zero.
 > 
 > While this can mean an increased workload for you as an individual it 
 > does dramatically reduce the work load for the entire GDB community.
 > 
 > My reading of Elena's comment:
 > 
 > > Yes, I looked ths over and it seems to work, except that I would really
 > > prefer the change to printcmd.c split in two. The first bit to
 > > rationalize that "if (func)..."  code. This would have with it all
 > > the indentation changes as well. The code as it is now doesn't really
 > > make much sense. So, that looks a good change to me. But it has nothing
 > > to do with the new macro.  After that change is in, you can introduce
 > > the macro in printcmd.c w/o having all the indent changes.
 > > It also makes it easier to distinguish a no-op change (the macro) from
 > > the other one.
 > 
 > is that you're all approved.

Yes, sorry if that wasn't clear.

 > 
 > Your first commit fixes some messed up logic.  It is a cleanup (but 
 > pretty obvious).  It doesn't have anything to do with the (ULGH) macro. 
 >   By keeping it separate it makes it possible to better isolate the 
 > breakage it could cause when we have to go back (in 6 months) to find a 
 > bug ;-)
 > 
 > Your second commit is this new (ULGH) macro.  The macro (ULGH) shouldn't 
 > break anything but it is however still a (ULGH) macro.  Just include the 
 > extra tweeks you found.
 > 

Yes. 

Elena

 > (If you haven't figured it out, breakpoint.h has a similar (ULGH) macro 
 > so I'm biteing my tongue on this change :-)
 > 
 > Andrew
 > 

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

On Thu, Oct 11, 2001 at 07:54:50PM -0400, Daniel Jacobowitz wrote:
> On Thu, Oct 11, 2001 at 07:46:33PM -0400, Elena Zannoni wrote:
> > 
> > Daniel,
> > Thanks so much for doing this. It makes it so much easier.
> > 
> > Yes, I looked ths over and it seems to work, except that I would really
> > prefer the change to printcmd.c split in two. The first bit to
> > rationalize that "if (func)..."  code. This would have with it all
> > the indentation changes as well. The code as it is now doesn't really
> > make much sense. So, that looks a good change to me. But it has nothing
> > to do with the new macro.  After that change is in, you can introduce
> > the macro in printcmd.c w/o having all the indent changes.
> > It also makes it easier to distinguish a no-op change (the macro) from
> > the other one.
> 
> OK.  Would you prefer I resubmit this patch broken up further, then? 
> I could do that.

Having reduced it to truly obvious, I'm going to commit the attached
patch, unless someone objects strenuously in the near future.  A diff
ignoring whitespace shows that I am only moving a brace from before a
for loop to after it; the for loop will never be executed unless the if
is taken, anyway.  This'll shrink the other patch quite a bit.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-10-11  Daniel Jacobowitz  <drow@mvista.com>

	* printcmd.c (print_frame_args):  Move symbol iteration explicitly
	inside the func != NULL block.

Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.27
diff -u -p -r1.27 printcmd.c
--- printcmd.c	2001/09/12 04:18:08	1.27
+++ printcmd.c	2001/10/12 00:59:44
@@ -1807,167 +1807,167 @@ print_frame_args (struct symbol *func, s
     {
       b = SYMBOL_BLOCK_VALUE (func);
       nsyms = BLOCK_NSYMS (b);
-    }
 
-  for (i = 0; i < nsyms; i++)
-    {
-      QUIT;
-      sym = BLOCK_SYM (b, i);
-
-      /* Keep track of the highest stack argument offset seen, and
-         skip over any kinds of symbols we don't care about.  */
-
-      switch (SYMBOL_CLASS (sym))
-	{
-	case LOC_ARG:
-	case LOC_REF_ARG:
-	  {
-	    long current_offset = SYMBOL_VALUE (sym);
-	    arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
-
-	    /* Compute address of next argument by adding the size of
-	       this argument and rounding to an int boundary.  */
-	    current_offset =
-	      ((current_offset + arg_size + sizeof (int) - 1)
-		 & ~(sizeof (int) - 1));
-
-	    /* If this is the highest offset seen yet, set highest_offset.  */
-	    if (highest_offset == -1
-		|| (current_offset > highest_offset))
-	      highest_offset = current_offset;
-
-	    /* Add the number of ints we're about to print to args_printed.  */
-	    args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
-	  }
-
-	  /* We care about types of symbols, but don't need to keep track of
-	     stack offsets in them.  */
-	case LOC_REGPARM:
-	case LOC_REGPARM_ADDR:
-	case LOC_LOCAL_ARG:
-	case LOC_BASEREG_ARG:
-	  break;
-
-	  /* Other types of symbols we just skip over.  */
-	default:
-	  continue;
-	}
+      for (i = 0; i < nsyms; i++)
+        {
+	  QUIT;
+	  sym = BLOCK_SYM (b, i);
+
+	  /* Keep track of the highest stack argument offset seen, and
+	     skip over any kinds of symbols we don't care about.  */
 
-      /* We have to look up the symbol because arguments can have
-         two entries (one a parameter, one a local) and the one we
-         want is the local, which lookup_symbol will find for us.
-         This includes gcc1 (not gcc2) on the sparc when passing a
-         small structure and gcc2 when the argument type is float
-         and it is passed as a double and converted to float by
-         the prologue (in the latter case the type of the LOC_ARG
-         symbol is double and the type of the LOC_LOCAL symbol is
-         float).  */
-      /* But if the parameter name is null, don't try it.
-         Null parameter names occur on the RS/6000, for traceback tables.
-         FIXME, should we even print them?  */
-
-      if (*SYMBOL_NAME (sym))
-	{
-	  struct symbol *nsym;
-	  nsym = lookup_symbol
-	    (SYMBOL_NAME (sym),
-	     b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
-	  if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
+	  switch (SYMBOL_CLASS (sym))
 	    {
-	      /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
-	         it was passed on the stack and loaded into a register,
-	         or passed in a register and stored in a stack slot.
-	         GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
-
-	         Reasons for using the LOC_ARG:
-	         (1) because find_saved_registers may be slow for remote
-	         debugging,
-	         (2) because registers are often re-used and stack slots
-	         rarely (never?) are.  Therefore using the stack slot is
-	         much less likely to print garbage.
-
-	         Reasons why we might want to use the LOC_REGISTER:
-	         (1) So that the backtrace prints the same value as
-	         "print foo".  I see no compelling reason why this needs
-	         to be the case; having the backtrace print the value which
-	         was passed in, and "print foo" print the value as modified
-	         within the called function, makes perfect sense to me.
-
-	         Additional note:  It might be nice if "info args" displayed
-	         both values.
-	         One more note:  There is a case with sparc structure passing
-	         where we need to use the LOC_REGISTER, but this is dealt with
-	         by creating a single LOC_REGPARM in symbol reading.  */
+	    case LOC_ARG:
+	    case LOC_REF_ARG:
+	      {
+		long current_offset = SYMBOL_VALUE (sym);
+		arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
+
+		/* Compute address of next argument by adding the size of
+		   this argument and rounding to an int boundary.  */
+		current_offset =
+		  ((current_offset + arg_size + sizeof (int) - 1)
+		   & ~(sizeof (int) - 1));
+
+		/* If this is the highest offset seen yet, set highest_offset.  */
+		if (highest_offset == -1
+		    || (current_offset > highest_offset))
+		  highest_offset = current_offset;
+
+		/* Add the number of ints we're about to print to args_printed.  */
+		args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
+	      }
+
+	      /* We care about types of symbols, but don't need to keep track of
+		 stack offsets in them.  */
+	    case LOC_REGPARM:
+	    case LOC_REGPARM_ADDR:
+	    case LOC_LOCAL_ARG:
+	    case LOC_BASEREG_ARG:
+	      break;
+
+	    /* Other types of symbols we just skip over.  */
+	    default:
+	      continue;
+	    }
+
+	  /* We have to look up the symbol because arguments can have
+	     two entries (one a parameter, one a local) and the one we
+	     want is the local, which lookup_symbol will find for us.
+	     This includes gcc1 (not gcc2) on the sparc when passing a
+	     small structure and gcc2 when the argument type is float
+	     and it is passed as a double and converted to float by
+	     the prologue (in the latter case the type of the LOC_ARG
+	     symbol is double and the type of the LOC_LOCAL symbol is
+	     float).  */
+	  /* But if the parameter name is null, don't try it.
+	     Null parameter names occur on the RS/6000, for traceback tables.
+	     FIXME, should we even print them?  */
 
-	      /* Leave sym (the LOC_ARG) alone.  */
-	      ;
+	  if (*SYMBOL_NAME (sym))
+	    {
+	      struct symbol *nsym;
+	      nsym = lookup_symbol
+		(SYMBOL_NAME (sym),
+		 b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
+	      if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
+		{
+		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
+		     it was passed on the stack and loaded into a register,
+		     or passed in a register and stored in a stack slot.
+		     GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
+
+		     Reasons for using the LOC_ARG:
+		     (1) because find_saved_registers may be slow for remote
+		     debugging,
+		     (2) because registers are often re-used and stack slots
+		     rarely (never?) are.  Therefore using the stack slot is
+		     much less likely to print garbage.
+
+		     Reasons why we might want to use the LOC_REGISTER:
+		     (1) So that the backtrace prints the same value as
+		     "print foo".  I see no compelling reason why this needs
+		     to be the case; having the backtrace print the value which
+		     was passed in, and "print foo" print the value as modified
+		     within the called function, makes perfect sense to me.
+
+		     Additional note:  It might be nice if "info args" displayed
+		     both values.
+		     One more note:  There is a case with sparc structure passing
+		     where we need to use the LOC_REGISTER, but this is dealt with
+		     by creating a single LOC_REGPARM in symbol reading.  */
+
+		  /* Leave sym (the LOC_ARG) alone.  */
+		  ;
+		}
+	      else
+		sym = nsym;
 	    }
-	  else
-	    sym = nsym;
-	}
 
 #ifdef UI_OUT
-      /* Print the current arg.  */
-      if (!first)
-	ui_out_text (uiout, ", ");
-      ui_out_wrap_hint (uiout, "    ");
-
-      annotate_arg_begin ();
-
-      list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-      fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
-			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
-      ui_out_field_stream (uiout, "name", stb);
-      annotate_arg_name_end ();
-      ui_out_text (uiout, "=");
+	  /* Print the current arg.  */
+	  if (!first)
+	    ui_out_text (uiout, ", ");
+	  ui_out_wrap_hint (uiout, "    ");
+
+	  annotate_arg_begin ();
+
+	  list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+	  fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
+				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
+	  ui_out_field_stream (uiout, "name", stb);
+	  annotate_arg_name_end ();
+	  ui_out_text (uiout, "=");
 #else
-      /* Print the current arg.  */
-      if (!first)
-	fprintf_filtered (stream, ", ");
-      wrap_here ("    ");
-
-      annotate_arg_begin ();
-
-      fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
-			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
-      annotate_arg_name_end ();
-      fputs_filtered ("=", stream);
+	  /* Print the current arg.  */
+	  if (!first)
+	    fprintf_filtered (stream, ", ");
+	  wrap_here ("    ");
+
+	  annotate_arg_begin ();
+
+	  fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
+				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
+	  annotate_arg_name_end ();
+	  fputs_filtered ("=", stream);
 #endif
 
-      /* Avoid value_print because it will deref ref parameters.  We just
-         want to print their addresses.  Print ??? for args whose address
-         we do not know.  We pass 2 as "recurse" to val_print because our
-         standard indentation here is 4 spaces, and val_print indents
-         2 for each recurse.  */
-      val = read_var_value (sym, fi);
+	  /* Avoid value_print because it will deref ref parameters.  We just
+	     want to print their addresses.  Print ??? for args whose address
+	     we do not know.  We pass 2 as "recurse" to val_print because our
+	     standard indentation here is 4 spaces, and val_print indents
+	     2 for each recurse.  */
+	  val = read_var_value (sym, fi);
 
-      annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
+	  annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
 
-      if (val)
-	{
+	  if (val)
+	    {
 #ifdef UI_OUT
-	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
-		     VALUE_ADDRESS (val),
-		     stb->stream, 0, 0, 2, Val_no_prettyprint);
-	  ui_out_field_stream (uiout, "value", stb);
-	}
-      else
-	ui_out_text (uiout, "???");
+	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
+			 VALUE_ADDRESS (val),
+			 stb->stream, 0, 0, 2, Val_no_prettyprint);
+	      ui_out_field_stream (uiout, "value", stb);
+	    }
+	  else
+	    ui_out_text (uiout, "???");
 
-      /* Invoke ui_out_tuple_end.  */
-      do_cleanups (list_chain);
+	  /* Invoke ui_out_tuple_end.  */
+	  do_cleanups (list_chain);
 #else
-	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
-		     VALUE_ADDRESS (val),
-		     stream, 0, 0, 2, Val_no_prettyprint);
-	}
-      else
-	fputs_filtered ("???", stream);
+	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
+			 VALUE_ADDRESS (val),
+			 stream, 0, 0, 2, Val_no_prettyprint);
+	    }
+	  else
+	    fputs_filtered ("???", stream);
 #endif
 
-      annotate_arg_end ();
+	  annotate_arg_end ();
 
-      first = 0;
+	  first = 0;
+	}
     }
 
   /* Don't print nameless args in situations where we don't know

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Thu, Oct 11, 2001 at 07:54:50PM -0400, Daniel Jacobowitz wrote:
 > > On Thu, Oct 11, 2001 at 07:46:33PM -0400, Elena Zannoni wrote:
 > > > 
 > > > Daniel,
 > > > Thanks so much for doing this. It makes it so much easier.
 > > > 
 > > > Yes, I looked ths over and it seems to work, except that I would really
 > > > prefer the change to printcmd.c split in two. The first bit to
 > > > rationalize that "if (func)..."  code. This would have with it all
 > > > the indentation changes as well. The code as it is now doesn't really
 > > > make much sense. So, that looks a good change to me. But it has nothing
 > > > to do with the new macro.  After that change is in, you can introduce
 > > > the macro in printcmd.c w/o having all the indent changes.
 > > > It also makes it easier to distinguish a no-op change (the macro) from
 > > > the other one.
 > > 
 > > OK.  Would you prefer I resubmit this patch broken up further, then? 
 > > I could do that.
 > 
 > Having reduced it to truly obvious, I'm going to commit the attached
 > patch, unless someone objects strenuously in the near future.  A diff
 > ignoring whitespace shows that I am only moving a brace from before a
 > for loop to after it; the for loop will never be executed unless the if
 > is taken, anyway.  This'll shrink the other patch quite a bit.
 > 

Yes, thanks!!

Elena


 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2001-10-11  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* printcmd.c (print_frame_args):  Move symbol iteration explicitly
 > 	inside the func != NULL block.
 > 
 > Index: printcmd.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/printcmd.c,v
 > retrieving revision 1.27
 > diff -u -p -r1.27 printcmd.c
 > --- printcmd.c	2001/09/12 04:18:08	1.27
 > +++ printcmd.c	2001/10/12 00:59:44
 > @@ -1807,167 +1807,167 @@ print_frame_args (struct symbol *func, s
 >      {
 >        b = SYMBOL_BLOCK_VALUE (func);
 >        nsyms = BLOCK_NSYMS (b);
 > -    }
 >  
 > -  for (i = 0; i < nsyms; i++)
 > -    {
 > -      QUIT;
 > -      sym = BLOCK_SYM (b, i);
 > -
 > -      /* Keep track of the highest stack argument offset seen, and
 > -         skip over any kinds of symbols we don't care about.  */
 > -
 > -      switch (SYMBOL_CLASS (sym))
 > -	{
 > -	case LOC_ARG:
 > -	case LOC_REF_ARG:
 > -	  {
 > -	    long current_offset = SYMBOL_VALUE (sym);
 > -	    arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
 > -
 > -	    /* Compute address of next argument by adding the size of
 > -	       this argument and rounding to an int boundary.  */
 > -	    current_offset =
 > -	      ((current_offset + arg_size + sizeof (int) - 1)
 > -		 & ~(sizeof (int) - 1));
 > -
 > -	    /* If this is the highest offset seen yet, set highest_offset.  */
 > -	    if (highest_offset == -1
 > -		|| (current_offset > highest_offset))
 > -	      highest_offset = current_offset;
 > -
 > -	    /* Add the number of ints we're about to print to args_printed.  */
 > -	    args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
 > -	  }
 > -
 > -	  /* We care about types of symbols, but don't need to keep track of
 > -	     stack offsets in them.  */
 > -	case LOC_REGPARM:
 > -	case LOC_REGPARM_ADDR:
 > -	case LOC_LOCAL_ARG:
 > -	case LOC_BASEREG_ARG:
 > -	  break;
 > -
 > -	  /* Other types of symbols we just skip over.  */
 > -	default:
 > -	  continue;
 > -	}
 > +      for (i = 0; i < nsyms; i++)
 > +        {
 > +	  QUIT;
 > +	  sym = BLOCK_SYM (b, i);
 > +
 > +	  /* Keep track of the highest stack argument offset seen, and
 > +	     skip over any kinds of symbols we don't care about.  */
 >  
 > -      /* We have to look up the symbol because arguments can have
 > -         two entries (one a parameter, one a local) and the one we
 > -         want is the local, which lookup_symbol will find for us.
 > -         This includes gcc1 (not gcc2) on the sparc when passing a
 > -         small structure and gcc2 when the argument type is float
 > -         and it is passed as a double and converted to float by
 > -         the prologue (in the latter case the type of the LOC_ARG
 > -         symbol is double and the type of the LOC_LOCAL symbol is
 > -         float).  */
 > -      /* But if the parameter name is null, don't try it.
 > -         Null parameter names occur on the RS/6000, for traceback tables.
 > -         FIXME, should we even print them?  */
 > -
 > -      if (*SYMBOL_NAME (sym))
 > -	{
 > -	  struct symbol *nsym;
 > -	  nsym = lookup_symbol
 > -	    (SYMBOL_NAME (sym),
 > -	     b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
 > -	  if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
 > +	  switch (SYMBOL_CLASS (sym))
 >  	    {
 > -	      /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
 > -	         it was passed on the stack and loaded into a register,
 > -	         or passed in a register and stored in a stack slot.
 > -	         GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
 > -
 > -	         Reasons for using the LOC_ARG:
 > -	         (1) because find_saved_registers may be slow for remote
 > -	         debugging,
 > -	         (2) because registers are often re-used and stack slots
 > -	         rarely (never?) are.  Therefore using the stack slot is
 > -	         much less likely to print garbage.
 > -
 > -	         Reasons why we might want to use the LOC_REGISTER:
 > -	         (1) So that the backtrace prints the same value as
 > -	         "print foo".  I see no compelling reason why this needs
 > -	         to be the case; having the backtrace print the value which
 > -	         was passed in, and "print foo" print the value as modified
 > -	         within the called function, makes perfect sense to me.
 > -
 > -	         Additional note:  It might be nice if "info args" displayed
 > -	         both values.
 > -	         One more note:  There is a case with sparc structure passing
 > -	         where we need to use the LOC_REGISTER, but this is dealt with
 > -	         by creating a single LOC_REGPARM in symbol reading.  */
 > +	    case LOC_ARG:
 > +	    case LOC_REF_ARG:
 > +	      {
 > +		long current_offset = SYMBOL_VALUE (sym);
 > +		arg_size = TYPE_LENGTH (SYMBOL_TYPE (sym));
 > +
 > +		/* Compute address of next argument by adding the size of
 > +		   this argument and rounding to an int boundary.  */
 > +		current_offset =
 > +		  ((current_offset + arg_size + sizeof (int) - 1)
 > +		   & ~(sizeof (int) - 1));
 > +
 > +		/* If this is the highest offset seen yet, set highest_offset.  */
 > +		if (highest_offset == -1
 > +		    || (current_offset > highest_offset))
 > +		  highest_offset = current_offset;
 > +
 > +		/* Add the number of ints we're about to print to args_printed.  */
 > +		args_printed += (arg_size + sizeof (int) - 1) / sizeof (int);
 > +	      }
 > +
 > +	      /* We care about types of symbols, but don't need to keep track of
 > +		 stack offsets in them.  */
 > +	    case LOC_REGPARM:
 > +	    case LOC_REGPARM_ADDR:
 > +	    case LOC_LOCAL_ARG:
 > +	    case LOC_BASEREG_ARG:
 > +	      break;
 > +
 > +	    /* Other types of symbols we just skip over.  */
 > +	    default:
 > +	      continue;
 > +	    }
 > +
 > +	  /* We have to look up the symbol because arguments can have
 > +	     two entries (one a parameter, one a local) and the one we
 > +	     want is the local, which lookup_symbol will find for us.
 > +	     This includes gcc1 (not gcc2) on the sparc when passing a
 > +	     small structure and gcc2 when the argument type is float
 > +	     and it is passed as a double and converted to float by
 > +	     the prologue (in the latter case the type of the LOC_ARG
 > +	     symbol is double and the type of the LOC_LOCAL symbol is
 > +	     float).  */
 > +	  /* But if the parameter name is null, don't try it.
 > +	     Null parameter names occur on the RS/6000, for traceback tables.
 > +	     FIXME, should we even print them?  */
 >  
 > -	      /* Leave sym (the LOC_ARG) alone.  */
 > -	      ;
 > +	  if (*SYMBOL_NAME (sym))
 > +	    {
 > +	      struct symbol *nsym;
 > +	      nsym = lookup_symbol
 > +		(SYMBOL_NAME (sym),
 > +		 b, VAR_NAMESPACE, (int *) NULL, (struct symtab **) NULL);
 > +	      if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
 > +		{
 > +		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means that
 > +		     it was passed on the stack and loaded into a register,
 > +		     or passed in a register and stored in a stack slot.
 > +		     GDB 3.x used the LOC_ARG; GDB 4.0-4.11 used the LOC_REGISTER.
 > +
 > +		     Reasons for using the LOC_ARG:
 > +		     (1) because find_saved_registers may be slow for remote
 > +		     debugging,
 > +		     (2) because registers are often re-used and stack slots
 > +		     rarely (never?) are.  Therefore using the stack slot is
 > +		     much less likely to print garbage.
 > +
 > +		     Reasons why we might want to use the LOC_REGISTER:
 > +		     (1) So that the backtrace prints the same value as
 > +		     "print foo".  I see no compelling reason why this needs
 > +		     to be the case; having the backtrace print the value which
 > +		     was passed in, and "print foo" print the value as modified
 > +		     within the called function, makes perfect sense to me.
 > +
 > +		     Additional note:  It might be nice if "info args" displayed
 > +		     both values.
 > +		     One more note:  There is a case with sparc structure passing
 > +		     where we need to use the LOC_REGISTER, but this is dealt with
 > +		     by creating a single LOC_REGPARM in symbol reading.  */
 > +
 > +		  /* Leave sym (the LOC_ARG) alone.  */
 > +		  ;
 > +		}
 > +	      else
 > +		sym = nsym;
 >  	    }
 > -	  else
 > -	    sym = nsym;
 > -	}
 >  
 >  #ifdef UI_OUT
 > -      /* Print the current arg.  */
 > -      if (!first)
 > -	ui_out_text (uiout, ", ");
 > -      ui_out_wrap_hint (uiout, "    ");
 > -
 > -      annotate_arg_begin ();
 > -
 > -      list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 > -      fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
 > -			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > -      ui_out_field_stream (uiout, "name", stb);
 > -      annotate_arg_name_end ();
 > -      ui_out_text (uiout, "=");
 > +	  /* Print the current arg.  */
 > +	  if (!first)
 > +	    ui_out_text (uiout, ", ");
 > +	  ui_out_wrap_hint (uiout, "    ");
 > +
 > +	  annotate_arg_begin ();
 > +
 > +	  list_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 > +	  fprintf_symbol_filtered (stb->stream, SYMBOL_SOURCE_NAME (sym),
 > +				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > +	  ui_out_field_stream (uiout, "name", stb);
 > +	  annotate_arg_name_end ();
 > +	  ui_out_text (uiout, "=");
 >  #else
 > -      /* Print the current arg.  */
 > -      if (!first)
 > -	fprintf_filtered (stream, ", ");
 > -      wrap_here ("    ");
 > -
 > -      annotate_arg_begin ();
 > -
 > -      fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
 > -			    SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > -      annotate_arg_name_end ();
 > -      fputs_filtered ("=", stream);
 > +	  /* Print the current arg.  */
 > +	  if (!first)
 > +	    fprintf_filtered (stream, ", ");
 > +	  wrap_here ("    ");
 > +
 > +	  annotate_arg_begin ();
 > +
 > +	  fprintf_symbol_filtered (stream, SYMBOL_SOURCE_NAME (sym),
 > +				   SYMBOL_LANGUAGE (sym), DMGL_PARAMS | DMGL_ANSI);
 > +	  annotate_arg_name_end ();
 > +	  fputs_filtered ("=", stream);
 >  #endif
 >  
 > -      /* Avoid value_print because it will deref ref parameters.  We just
 > -         want to print their addresses.  Print ??? for args whose address
 > -         we do not know.  We pass 2 as "recurse" to val_print because our
 > -         standard indentation here is 4 spaces, and val_print indents
 > -         2 for each recurse.  */
 > -      val = read_var_value (sym, fi);
 > +	  /* Avoid value_print because it will deref ref parameters.  We just
 > +	     want to print their addresses.  Print ??? for args whose address
 > +	     we do not know.  We pass 2 as "recurse" to val_print because our
 > +	     standard indentation here is 4 spaces, and val_print indents
 > +	     2 for each recurse.  */
 > +	  val = read_var_value (sym, fi);
 >  
 > -      annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
 > +	  annotate_arg_value (val == NULL ? NULL : VALUE_TYPE (val));
 >  
 > -      if (val)
 > -	{
 > +	  if (val)
 > +	    {
 >  #ifdef UI_OUT
 > -	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > -		     VALUE_ADDRESS (val),
 > -		     stb->stream, 0, 0, 2, Val_no_prettyprint);
 > -	  ui_out_field_stream (uiout, "value", stb);
 > -	}
 > -      else
 > -	ui_out_text (uiout, "???");
 > +	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > +			 VALUE_ADDRESS (val),
 > +			 stb->stream, 0, 0, 2, Val_no_prettyprint);
 > +	      ui_out_field_stream (uiout, "value", stb);
 > +	    }
 > +	  else
 > +	    ui_out_text (uiout, "???");
 >  
 > -      /* Invoke ui_out_tuple_end.  */
 > -      do_cleanups (list_chain);
 > +	  /* Invoke ui_out_tuple_end.  */
 > +	  do_cleanups (list_chain);
 >  #else
 > -	  val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > -		     VALUE_ADDRESS (val),
 > -		     stream, 0, 0, 2, Val_no_prettyprint);
 > -	}
 > -      else
 > -	fputs_filtered ("???", stream);
 > +	      val_print (VALUE_TYPE (val), VALUE_CONTENTS (val), 0,
 > +			 VALUE_ADDRESS (val),
 > +			 stream, 0, 0, 2, Val_no_prettyprint);
 > +	    }
 > +	  else
 > +	    fputs_filtered ("???", stream);
 >  #endif
 >  
 > -      annotate_arg_end ();
 > +	  annotate_arg_end ();
 >  
 > -      first = 0;
 > +	  first = 0;
 > +	}
 >      }
 >  
 >    /* Don't print nameless args in situations where we don't know

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

On Fri, Oct 12, 2001 at 12:16:06PM -0400, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
>  > On Thu, Oct 11, 2001 at 07:54:50PM -0400, Daniel Jacobowitz wrote:
>  > > On Thu, Oct 11, 2001 at 07:46:33PM -0400, Elena Zannoni wrote:
>  > > > 
>  > > > Daniel,
>  > > > Thanks so much for doing this. It makes it so much easier.
>  > > > 
>  > > > Yes, I looked ths over and it seems to work, except that I would really
>  > > > prefer the change to printcmd.c split in two. The first bit to
>  > > > rationalize that "if (func)..."  code. This would have with it all
>  > > > the indentation changes as well. The code as it is now doesn't really
>  > > > make much sense. So, that looks a good change to me. But it has nothing
>  > > > to do with the new macro.  After that change is in, you can introduce
>  > > > the macro in printcmd.c w/o having all the indent changes.
>  > > > It also makes it easier to distinguish a no-op change (the macro) from
>  > > > the other one.
>  > > 
>  > > OK.  Would you prefer I resubmit this patch broken up further, then? 
>  > > I could do that.
>  > 
>  > Having reduced it to truly obvious, I'm going to commit the attached
>  > patch, unless someone objects strenuously in the near future.  A diff
>  > ignoring whitespace shows that I am only moving a brace from before a
>  > for loop to after it; the for loop will never be executed unless the if
>  > is taken, anyway.  This'll shrink the other patch quite a bit.
>  > 
> 
> Yes, thanks!!

So let it be written, etc. etc.  Committed.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Thu, Oct 11, 2001 at 07:46:33PM -0400, Elena Zannoni wrote:
 > > 
 > > Daniel,
 > > Thanks so much for doing this. It makes it so much easier.
 > > 
 > > Yes, I looked ths over and it seems to work, except that I would really
 > > prefer the change to printcmd.c split in two. The first bit to
 > > rationalize that "if (func)..."  code. This would have with it all
 > > the indentation changes as well. The code as it is now doesn't really
 > > make much sense. So, that looks a good change to me. But it has nothing
 > > to do with the new macro.  After that change is in, you can introduce
 > > the macro in printcmd.c w/o having all the indent changes.
 > > It also makes it easier to distinguish a no-op change (the macro) from
 > > the other one.
 > 
 > OK.  Would you prefer I resubmit this patch broken up further, then? 
 > I could do that.

No need to resubmit. Just check it in as 2 patches instead of 1. So
that I can do cvs diffs between versions of the file later, if there
is an issue.

 > 
 > There's a double-edged sword here; every patch in this sequence except
 > for the hashing change is predicated on the previous patches.  So while
 > I understand that breaking them up does make reviewing much easier,
 > with the current length of the patch review cycle, every time I
 > decompose this further I add two or three more days to its eventual
 > (hopeful) approval.  I'm sure you can understand that it's a little
 > frustrating.
 > 

In theory, the case at hand shouldn't even occur. I myself have done
on the order of half a dozen commits to the same file in the same day,
just to break my changes into minimal pieces. I personally tend to
strip down patches to their bare bones. I agree though that it makes it
harder to keep the various copies of the files in synch.

But as I said, I don't care about you resubmitting this patch.


 > > The cahnge is printcmd.c needs to delete also the 
 > > 	  sym = BLOCK_SYM (b, i);
 > > line.
 > 
 > ... that's what I get for moving too fast between trees.  Thanks for noticing.
 > 
 > > 
 > > [Note that I don't maintain printcmd.c, so, I should just shut up :-)]
 > > 
 > > I applied your patch as is to my sources, and did a grep for BLOCK_SYM,
 > > and found a few more for loops that could be converted:
 > > 
 > > 
 > > This one in buildsym.c:
 > > line ~280:
 > > 
 > > 	  struct symbol *sym;
 > > 	  for (i = 0; i < BLOCK_NSYMS (block); i++)
 > > 	    {
 > > 	      sym = BLOCK_SYM (block, i);
 > 
 > I could change this one too.  It's from a case where the symbols will
 > never be hashable (i.e. order-sensitive), so I didn't touch it the
 > first time through.  Consistency is good, though.

Yes, I would prefer to change this too.

 > 
 > > And this one in symtab.c:
 > > line ~1500:
 > > 
 > > 	top = BLOCK_NSYMS (block);
 > > 	for (bot = 0; bot < top; bot++)
 > > 	  {
 > > 	    sym = BLOCK_SYM (block, bot);
 > > 
 > 
 > And this one's #if 0'd out.  I could fix it (just the ALL_BLOCK_SYMBOLS
 > change, I mean),  or delete it per the comment above it.
 > 

Ah, I didn't notice this. Just leave it then. 


How about this other one? in symtab.c ~line 1254

      top = BLOCK_NSYMS (block);
      while (bot < top)
	{
	  sym = BLOCK_SYM (block, bot);
	  if (SYMBOL_NAMESPACE (sym) == namespace &&
	      SYMBOL_MATCHES_NAME (sym, name))
	    {
	      return sym;
	    }
	  bot++;
	}


 > > Another one is in mi/mi-stack-cmd.c.
 > > There are some also in gdbtk/generic/gdbtk-cmds.c and gdbtk-stack.c.
 > 
 > These I missed; I didn't think to check further down than the top
 > level.  I'll get them when I repost this, which it looks like I'll need
 > to do.
 > 

No, just post the new changes only.


 > > Some tricky ones are left, for a second pass. In mdebugread.c: it is
 > > actually a while loop, in mylookup_symbol, similarly in coffread.c:
 > > patch_opaque_types(), the for loop is reversed.
 > > (Were these in your original patch? I don't remember).
 > 
 > The one in coffread worries me; I can not tell from the comments if it
 > is order-sensitive or not.  The one in mdebugread does not, because
 > mdebugread does dastardly things to blocks.  I deliberately left those
 > unhashed.

Hmmm, maybe we should leave the coffread one alone. 
But I think we should use the macro in mdebugread.c.

 > 
 > > I cannot spot any others, can you?
 > 
 > Nope.
 > 

Ok, good.

Elena


 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

On Fri, Oct 12, 2001 at 11:56:55AM -0400, Elena Zannoni wrote:
> No need to resubmit. Just check it in as 2 patches instead of 1. So
> that I can do cvs diffs between versions of the file later, if there
> is an issue.

Gotcha.

> Yes, I would prefer to change this too.

Included now.

> How about this other one? in symtab.c ~line 1254
> 
>       top = BLOCK_NSYMS (block);
>       while (bot < top)
> 	{
> 	  sym = BLOCK_SYM (block, bot);
> 	  if (SYMBOL_NAMESPACE (sym) == namespace &&
> 	      SYMBOL_MATCHES_NAME (sym, name))
> 	    {
> 	      return sym;
> 	    }
> 	  bot++;
> 	}

Can't.  Bot is not 0; this doesn't search all symbols.  Also,
ALL_BLOCK_SYMBOLS logically doesn't imply an ordered search, which this
demands.  This'll be clarified in the hashing patch.

>  > > Another one is in mi/mi-stack-cmd.c.
>  > > There are some also in gdbtk/generic/gdbtk-cmds.c and gdbtk-stack.c.
>  > 
>  > These I missed; I didn't think to check further down than the top
>  > level.  I'll get them when I repost this, which it looks like I'll need
>  > to do.
>  > 
> 
> No, just post the new changes only.

Got this bunch also.  Do I need someone else's approval to check in
changes to gdbtk/generic/?  I don't think so, since these are obvious.

I'm actually just attaching the whole patch; it's relatively short now,
and I've tweaked it in a couple of places.

>  > > Some tricky ones are left, for a second pass. In mdebugread.c: it is
>  > > actually a while loop, in mylookup_symbol, similarly in coffread.c:
>  > > patch_opaque_types(), the for loop is reversed.
>  > > (Were these in your original patch? I don't remember).
>  > 
>  > The one in coffread worries me; I can not tell from the comments if it
>  > is order-sensitive or not.  The one in mdebugread does not, because
>  > mdebugread does dastardly things to blocks.  I deliberately left those
>  > unhashed.
> 
> Hmmm, maybe we should leave the coffread one alone. 

I'd actually appreciate it if you, or someone else with a little
experience with coffread (any takers?), would look at this.  It
absolutely has to be changed eventually; I didn't include it in this
patch only because it wasn't obvious.

> But I think we should use the macro in mdebugread.c.

Sure.

Here's what I've got now.  I'll commit it later today unless someone
sees a problem with it.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-10-01  Daniel Jacobowitz  <drow@mvista.com>

	* symtab.h (struct block): (ALL_BLOCK_SYMBOLS): New macro.

	* symtab.c (find_pc_sect_symtab): Use ALL_BLOCK_SYMBOLS.
	(make_symbol_completion_list): Likewise.
	(make_symbol_overload_list): Likewise.
	* buildsym.c (finish_block): Likewise.
	* breakpoint.c (get_catch_sals):  Likewise.
	* mdebugread.c (mylookup_symbol): Likewise.
	* objfiles.c (objfile_relocate): Likewise.
	* printcmd.c (print_frame_args): Likewise.
	* stack.c (print_block_frame_locals): Likewise.
	(print_block_frame_labels): Likewise.
	(print_frame_arg_vars): Likewise.
	* symmisc.c (dump_symtab): Likewise.
	* tracepoint.c (add_local_symbols): Likewise.
	(scope_info): Likewise.

2001-10-12  Daniel Jacobowitz  <drow@mvista.com>

	* mi-cmd-stack.c (list_args_or_locals): Use ALL_BLOCK_SYMBOLS.

2001-10-12  Daniel Jacobowitz  <drow@mvista.com>

	* generic/gdbtk-cmds.c (gdb_listfuncs): Use ALL_BLOCK_SYMBOLS.
	* generic/gdbtk-stack.c (gdb_block_vars): Likewise.
	(gdb_get_blocks): Likewise.
	(gdb_get_vars_command): Likewise.

Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.53
diff -u -p -r1.53 breakpoint.c
--- gdb/breakpoint.c	2001/09/18 05:00:48	1.53
+++ gdb/breakpoint.c	2001/10/12 18:34:49
@@ -5871,15 +5871,11 @@ get_catch_sals (int this_level_only)
 	  if (blocks_searched[index] == 0)
 	    {
 	      struct block *b = BLOCKVECTOR_BLOCK (bl, index);
-	      int nsyms;
 	      register int i;
 	      register struct symbol *sym;
 
-	      nsyms = BLOCK_NSYMS (b);
-
-	      for (i = 0; i < nsyms; i++)
+	      ALL_BLOCK_SYMBOLS (b, i, sym)
 		{
-		  sym = BLOCK_SYM (b, i);
 		  if (STREQ (SYMBOL_NAME (sym), "default"))
 		    {
 		      if (have_default)
Index: gdb/buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.11
diff -u -p -r1.11 buildsym.c
--- gdb/buildsym.c	2001/04/30 10:30:27	1.11
+++ gdb/buildsym.c	2001/10/12 18:34:49
@@ -275,9 +275,8 @@ finish_block (struct symbol *symbol, str
 	     parameter symbols. */
 	  int nparams = 0, iparams;
 	  struct symbol *sym;
-	  for (i = 0; i < BLOCK_NSYMS (block); i++)
+	  ALL_BLOCK_SYMBOLS (block, i, sym)
 	    {
-	      sym = BLOCK_SYM (block, i);
 	      switch (SYMBOL_CLASS (sym))
 		{
 		case LOC_ARG:
Index: gdb/mdebugread.c
===================================================================
RCS file: /cvs/src/src/gdb/mdebugread.c,v
retrieving revision 1.15
diff -u -p -r1.15 mdebugread.c
--- gdb/mdebugread.c	2001/09/05 02:54:15	1.15
+++ gdb/mdebugread.c	2001/10/12 18:34:50
@@ -3853,22 +3853,19 @@ static struct symbol *
 mylookup_symbol (char *name, register struct block *block,
 		 namespace_enum namespace, enum address_class class)
 {
-  register int bot, top, inc;
-  register struct symbol *sym;
+  int i, inc;
+  struct symbol *sym;
 
-  bot = 0;
-  top = BLOCK_NSYMS (block);
   inc = name[0];
-  while (bot < top)
+  ALL_BLOCK_SYMBOLS (block, i, sym)
     {
-      sym = BLOCK_SYM (block, bot);
       if (SYMBOL_NAME (sym)[0] == inc
 	  && SYMBOL_NAMESPACE (sym) == namespace
 	  && SYMBOL_CLASS (sym) == class
 	  && strcmp (SYMBOL_NAME (sym), name) == 0)
 	return sym;
-      bot++;
     }
+
   block = BLOCK_SUPERBLOCK (block);
   if (block)
     return mylookup_symbol (name, block, namespace, class);
Index: gdb/objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.16
diff -u -p -r1.16 objfiles.c
--- gdb/objfiles.c	2001/07/05 21:32:39	1.16
+++ gdb/objfiles.c	2001/10/12 18:34:50
@@ -557,16 +557,15 @@ objfile_relocate (struct objfile *objfil
       for (i = 0; i < BLOCKVECTOR_NBLOCKS (bv); ++i)
 	{
 	  struct block *b;
+	  struct symbol *sym;
 	  int j;
 
 	  b = BLOCKVECTOR_BLOCK (bv, i);
 	  BLOCK_START (b) += ANOFFSET (delta, s->block_line_section);
 	  BLOCK_END (b) += ANOFFSET (delta, s->block_line_section);
 
-	  for (j = 0; j < BLOCK_NSYMS (b); ++j)
+	  ALL_BLOCK_SYMBOLS (b, j, sym)
 	    {
-	      struct symbol *sym = BLOCK_SYM (b, j);
-
 	      fixup_symbol_section (sym, objfile);
 
 	      /* The RS6000 code from which this was taken skipped
Index: gdb/printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.28
diff -u -p -r1.28 printcmd.c
--- gdb/printcmd.c	2001/10/12 17:15:34	1.28
+++ gdb/printcmd.c	2001/10/12 18:34:50
@@ -1783,7 +1783,6 @@ print_frame_args (struct symbol *func, s
 		  struct ui_file *stream)
 {
   struct block *b = NULL;
-  int nsyms = 0;
   int first = 1;
   register int i;
   register struct symbol *sym;
@@ -1806,12 +1805,9 @@ print_frame_args (struct symbol *func, s
   if (func)
     {
       b = SYMBOL_BLOCK_VALUE (func);
-      nsyms = BLOCK_NSYMS (b);
-
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (b, i, sym)
         {
 	  QUIT;
-	  sym = BLOCK_SYM (b, i);
 
 	  /* Keep track of the highest stack argument offset seen, and
 	     skip over any kinds of symbols we don't care about.  */
Index: gdb/stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.22
diff -u -p -r1.22 stack.c
--- gdb/stack.c	2001/07/14 18:59:07	1.22
+++ gdb/stack.c	2001/10/12 18:34:50
@@ -1207,16 +1207,12 @@ static int
 print_block_frame_locals (struct block *b, register struct frame_info *fi,
 			  int num_tabs, register struct ui_file *stream)
 {
-  int nsyms;
   register int i, j;
   register struct symbol *sym;
   register int values_printed = 0;
 
-  nsyms = BLOCK_NSYMS (b);
-
-  for (i = 0; i < nsyms; i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       switch (SYMBOL_CLASS (sym))
 	{
 	case LOC_LOCAL:
@@ -1246,16 +1242,12 @@ static int
 print_block_frame_labels (struct block *b, int *have_default,
 			  register struct ui_file *stream)
 {
-  int nsyms;
   register int i;
   register struct symbol *sym;
   register int values_printed = 0;
-
-  nsyms = BLOCK_NSYMS (b);
 
-  for (i = 0; i < nsyms; i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       if (STREQ (SYMBOL_NAME (sym), "default"))
 	{
 	  if (*have_default)
@@ -1432,7 +1424,6 @@ print_frame_arg_vars (register struct fr
 {
   struct symbol *func = get_frame_function (fi);
   register struct block *b;
-  int nsyms;
   register int i;
   register struct symbol *sym, *sym2;
   register int values_printed = 0;
@@ -1444,11 +1435,8 @@ print_frame_arg_vars (register struct fr
     }
 
   b = SYMBOL_BLOCK_VALUE (func);
-  nsyms = BLOCK_NSYMS (b);
-
-  for (i = 0; i < nsyms; i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       switch (SYMBOL_CLASS (sym))
 	{
 	case LOC_ARG:
@@ -1483,7 +1471,6 @@ print_frame_arg_vars (register struct fr
 	  break;
 	}
     }
-
   if (!values_printed)
     {
       fprintf_filtered (stream, "No arguments.\n");
Index: gdb/symmisc.c
===================================================================
RCS file: /cvs/src/src/gdb/symmisc.c,v
retrieving revision 1.5
diff -u -p -r1.5 symmisc.c
--- gdb/symmisc.c	2001/03/06 08:21:17	1.5
+++ gdb/symmisc.c	2001/10/12 18:34:50
@@ -410,6 +410,7 @@ dump_symtab (struct objfile *objfile, st
   int len, blen;
   register struct linetable *l;
   struct blockvector *bv;
+  struct symbol *sym;
   register struct block *b;
   int depth;
 
@@ -471,11 +472,12 @@ dump_symtab (struct objfile *objfile, st
 	  if (BLOCK_GCC_COMPILED (b))
 	    fprintf_filtered (outfile, ", compiled with gcc%d", BLOCK_GCC_COMPILED (b));
 	  fprintf_filtered (outfile, "\n");
-	  /* Now print each symbol in this block */
-	  for (j = 0; j < blen; j++)
+	  /* Now print each symbol in this block.  */
+	  /* FIXMED: Sort?  */
+	  ALL_BLOCK_SYMBOLS (b, j, sym)
 	    {
 	      struct print_symbol_args s;
-	      s.symbol = BLOCK_SYM (b, j);
+	      s.symbol = sym;
 	      s.depth = depth + 1;
 	      s.outfile = outfile;
 	      catch_errors (print_symbol, &s, "Error printing symbol:\n",
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.43
diff -u -p -r1.43 symtab.c
--- gdb/symtab.c	2001/10/12 03:38:12	1.43
+++ gdb/symtab.c	2001/10/12 18:34:50
@@ -2992,9 +2992,8 @@ make_symbol_completion_list (char *text,
       /* Also catch fields of types defined in this places which match our
          text string.  Only complete on types visible from current context. */
 
-      for (i = 0; i < BLOCK_NSYMS (b); i++)
+      ALL_BLOCK_SYMBOLS (b, i, sym)
 	{
-	  sym = BLOCK_SYM (b, i);
 	  COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 	  if (SYMBOL_CLASS (sym) == LOC_TYPEDEF)
 	    {
@@ -3023,9 +3022,8 @@ make_symbol_completion_list (char *text,
   {
     QUIT;
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
       }
   }
@@ -3037,9 +3035,8 @@ make_symbol_completion_list (char *text,
     /* Don't do this block twice.  */
     if (b == surrounding_static_block)
       continue;
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
       }
   }
@@ -3143,16 +3140,14 @@ make_file_symbol_completion_list (char *
      symbols which match.  */
 
   b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-  for (i = 0; i < BLOCK_NSYMS (b); i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
     }
 
   b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
-  for (i = 0; i < BLOCK_NSYMS (b); i++)
+  ALL_BLOCK_SYMBOLS (b, i, sym)
     {
-      sym = BLOCK_SYM (b, i);
       COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
     }
 
@@ -3530,9 +3525,8 @@ make_symbol_overload_list (struct symbol
       /* Also catch fields of types defined in this places which match our
          text string.  Only complete on types visible from current context. */
 
-      for (i = 0; i < BLOCK_NSYMS (b); i++)
+      ALL_BLOCK_SYMBOLS (b, i, sym)
 	{
-	  sym = BLOCK_SYM (b, i);
 	  overload_list_add_symbol (sym, oload_name);
 	}
     }
@@ -3544,9 +3538,8 @@ make_symbol_overload_list (struct symbol
   {
     QUIT;
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	overload_list_add_symbol (sym, oload_name);
       }
   }
@@ -3558,9 +3551,8 @@ make_symbol_overload_list (struct symbol
     /* Don't do this block twice.  */
     if (b == surrounding_static_block)
       continue;
-    for (i = 0; i < BLOCK_NSYMS (b); i++)
+    ALL_BLOCK_SYMBOLS (b, i, sym)
       {
-	sym = BLOCK_SYM (b, i);
 	overload_list_add_symbol (sym, oload_name);
       }
   }
Index: gdb/symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.24
diff -u -p -r1.24 symtab.h
--- gdb/symtab.h	2001/07/07 17:19:50	1.24
+++ gdb/symtab.h	2001/10/12 18:34:50
@@ -467,6 +467,14 @@ struct block
 #define BLOCK_SUPERBLOCK(bl)	(bl)->superblock
 #define BLOCK_GCC_COMPILED(bl)	(bl)->gcc_compile_flag
 
+/* Macro to loop through all symbols in a block BL.
+   i counts which symbol we are looking at, and sym points to the current
+   symbol.  */
+#define ALL_BLOCK_SYMBOLS(bl, i, sym)			\
+	for ((i) = 0, (sym) = BLOCK_SYM ((bl), (i));	\
+	     (i) < BLOCK_NSYMS ((bl));			\
+	     ++(i), (sym) = BLOCK_SYM ((bl), (i)))
+
 /* Nonzero if symbols of block BL should be sorted alphabetically.
    Don't sort a block which corresponds to a function.  If we did the
    sorting would have to preserve the order of the symbols for the
Index: gdb/tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.26
diff -u -p -r1.26 tracepoint.c
--- gdb/tracepoint.c	2001/08/23 20:31:36	1.26
+++ gdb/tracepoint.c	2001/10/12 18:34:50
@@ -1296,16 +1296,14 @@ add_local_symbols (struct collection_lis
 {
   struct symbol *sym;
   struct block *block;
-  int i, nsyms, count = 0;
+  int i, count = 0;
 
   block = block_for_pc (pc);
   while (block != 0)
     {
       QUIT;			/* allow user to bail out with ^C */
-      nsyms = BLOCK_NSYMS (block);
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (block, i, sym)
 	{
-	  sym = BLOCK_SYM (block, i);
 	  switch (SYMBOL_CLASS (sym))
 	    {
 	    default:
@@ -2335,7 +2333,7 @@ scope_info (char *args, int from_tty)
   struct minimal_symbol *msym;
   struct block *block;
   char **canonical, *symname, *save_args = args;
-  int i, j, nsyms, count = 0;
+  int i, j, count = 0;
 
   if (args == 0 || *args == 0)
     error ("requires an argument (function, line or *addr) to define a scope");
@@ -2351,14 +2349,13 @@ scope_info (char *args, int from_tty)
   while (block != 0)
     {
       QUIT;			/* allow user to bail out with ^C */
-      nsyms = BLOCK_NSYMS (block);
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (block, i, sym)
 	{
 	  QUIT;			/* allow user to bail out with ^C */
 	  if (count == 0)
 	    printf_filtered ("Scope for %s:\n", save_args);
 	  count++;
-	  sym = BLOCK_SYM (block, i);
+
 	  symname = SYMBOL_NAME (sym);
 	  if (symname == NULL || *symname == '\0')
 	    continue;		/* probably botched, certainly useless */
Index: gdb/gdbtk/generic/gdbtk-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-cmds.c,v
retrieving revision 1.39
diff -u -p -r1.39 gdbtk-cmds.c
--- gdb/gdbtk/generic/gdbtk-cmds.c	2001/08/28 22:22:56	1.39
+++ gdb/gdbtk/generic/gdbtk-cmds.c	2001/10/12 18:34:50
@@ -1498,9 +1498,8 @@ gdb_listfuncs (clientData, interp, objc,
       /* Skip the sort if this block is always sorted.  */
       if (!BLOCK_SHOULD_SORT (b))
 	sort_block_syms (b);
-      for (j = 0; j < BLOCK_NSYMS (b); j++)
+      ALL_BLOCK_SYMBOLS (b, j, sym)
 	{
-	  sym = BLOCK_SYM (b, j);
 	  if (SYMBOL_CLASS (sym) == LOC_BLOCK)
 	    {
 
Index: gdb/gdbtk/generic/gdbtk-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-stack.c,v
retrieving revision 1.2
diff -u -p -r1.2 gdbtk-stack.c
--- gdb/gdbtk/generic/gdbtk-stack.c	2001/06/11 20:07:46	1.2
+++ gdb/gdbtk/generic/gdbtk-stack.c	2001/10/12 18:34:50
@@ -19,6 +19,7 @@
    Boston, MA 02111-1307, USA.  */
 
 #include "defs.h"
+#include "symtab.h"
 #include "frame.h"
 #include "value.h"
 #include "target.h"
@@ -93,7 +94,7 @@ gdb_block_vars (clientData, interp, objc
      Tcl_Obj *CONST objv[];
 {
   struct block *block;
-  int nsyms, i;
+  int i;
   struct symbol *sym;
   CORE_ADDR start, end;
 
@@ -117,10 +118,8 @@ gdb_block_vars (clientData, interp, objc
     {
       if (BLOCK_START (block) == start && BLOCK_END (block) == end)
 	{
-	  nsyms = BLOCK_NSYMS (block);
-	  for (i = 0; i < nsyms; i++)
+	  ALL_BLOCK_SYMBOLS (block, i, sym)
 	    {
-	      sym = BLOCK_SYM (block, i);
 	      switch (SYMBOL_CLASS (sym))
 		{
 		case LOC_ARG:		  /* argument              */
@@ -172,7 +171,7 @@ gdb_get_blocks (clientData, interp, objc
      Tcl_Obj *CONST objv[];
 {
   struct block *block;
-  int nsyms, i, junk;
+  int i, junk;
   struct symbol *sym;
   CORE_ADDR pc;
 
@@ -184,11 +183,9 @@ gdb_get_blocks (clientData, interp, objc
       pc = get_frame_pc (selected_frame);
       while (block != 0)
 	{
-	  nsyms = BLOCK_NSYMS (block);
 	  junk = 0;
-	  for (i = 0; i < nsyms; i++)
+	  ALL_BLOCK_SYMBOLS (block, i, sym)
 	    {
-	      sym = BLOCK_SYM (block, i);
 	      switch (SYMBOL_CLASS (sym))
 		{
 		default:
@@ -301,7 +298,7 @@ gdb_get_vars_command (clientData, interp
   struct symbol *sym;
   struct block *block;
   char **canonical, *args;
-  int i, nsyms, arguments;
+  int i, arguments;
 
   if (objc > 2)
     {
@@ -344,10 +341,8 @@ gdb_get_vars_command (clientData, interp
 
   while (block != 0)
     {
-      nsyms = BLOCK_NSYMS (block);
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (block, i, sym)
 	{
-	  sym = BLOCK_SYM (block, i);
 	  switch (SYMBOL_CLASS (sym))
 	    {
 	    default:
Index: gdb/mi/mi-cmd-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-stack.c,v
retrieving revision 1.7
diff -u -p -r1.7 mi-cmd-stack.c
--- gdb/mi/mi-cmd-stack.c	2001/06/26 18:50:51	1.7
+++ gdb/mi/mi-cmd-stack.c	2001/10/12 18:34:50
@@ -25,6 +25,7 @@
 #include "value.h"
 #include "mi-cmds.h"
 #include "ui-out.h"
+#include "symtab.h"
 
 #ifdef UI_OUT
 /* FIXME: these should go in some .h file but stack.c doesn't have a
@@ -226,10 +227,8 @@ list_args_or_locals (int locals, int val
 
   while (block != 0)
     {
-      nsyms = BLOCK_NSYMS (block);
-      for (i = 0; i < nsyms; i++)
+      ALL_BLOCK_SYMBOLS (block, i, sym)
 	{
-	  sym = BLOCK_SYM (block, i);
 	  switch (SYMBOL_CLASS (sym))
 	    {
 	    default:

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Andrew Cagney]

> 
> Got this bunch also.  Do I need someone else's approval to check in
> changes to gdbtk/generic/?  I don't think so, since these are obvious.

I'd just CC insight@sources.redhat.com so that insight people are 
guarenteed to know the change has come through.  I'm sure insight 
developers will appreciate you thinking of them.

Andrew

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Fri, Oct 12, 2001 at 11:56:55AM -0400, Elena Zannoni wrote:
 > > No need to resubmit. Just check it in as 2 patches instead of 1. So
 > > that I can do cvs diffs between versions of the file later, if there
 > > is an issue.
 > 
 > Gotcha.
 > 
 > > Yes, I would prefer to change this too.
 > 
 > Included now.
 > 
 > > How about this other one? in symtab.c ~line 1254
 > > 
 > >       top = BLOCK_NSYMS (block);
 > >       while (bot < top)
 > > 	{
 > > 	  sym = BLOCK_SYM (block, bot);
 > > 	  if (SYMBOL_NAMESPACE (sym) == namespace &&
 > > 	      SYMBOL_MATCHES_NAME (sym, name))
 > > 	    {
 > > 	      return sym;
 > > 	    }
 > > 	  bot++;
 > > 	}
 > 
 > Can't.  Bot is not 0; this doesn't search all symbols.  Also,
 > ALL_BLOCK_SYMBOLS logically doesn't imply an ordered search, which this
 > demands.  This'll be clarified in the hashing patch.
 > 

Ah yes, didn't notice.

 > >  > > Another one is in mi/mi-stack-cmd.c.
 > >  > > There are some also in gdbtk/generic/gdbtk-cmds.c and gdbtk-stack.c.
 > >  > 
 > >  > These I missed; I didn't think to check further down than the top
 > >  > level.  I'll get them when I repost this, which it looks like I'll need
 > >  > to do.
 > >  > 
 > > 
 > > No, just post the new changes only.
 > 
 > Got this bunch also.  Do I need someone else's approval to check in
 > changes to gdbtk/generic/?  I don't think so, since these are obvious.
 > 
 > I'm actually just attaching the whole patch; it's relatively short now,
 > and I've tweaked it in a couple of places.
 > 

Looks ok.

 > >  > > Some tricky ones are left, for a second pass. In mdebugread.c: it is
 > >  > > actually a while loop, in mylookup_symbol, similarly in coffread.c:
 > >  > > patch_opaque_types(), the for loop is reversed.
 > >  > > (Were these in your original patch? I don't remember).
 > >  > 
 > >  > The one in coffread worries me; I can not tell from the comments if it
 > >  > is order-sensitive or not.  The one in mdebugread does not, because
 > >  > mdebugread does dastardly things to blocks.  I deliberately left those
 > >  > unhashed.
 > > 
 > > Hmmm, maybe we should leave the coffread one alone. 
 > 
 > I'd actually appreciate it if you, or someone else with a little
 > experience with coffread (any takers?), would look at this.  It
 > absolutely has to be changed eventually; I didn't include it in this
 > patch only because it wasn't obvious.
 > 

Ok, I'll look it over.

 > > But I think we should use the macro in mdebugread.c.
 > 
 > Sure.
 > 
 > Here's what I've got now.  I'll commit it later today unless someone
 > sees a problem with it.
 > 

Just send a copy of this to the insight mailing list to see if they
are ok with the gdbtk changes. I don't see why they wouldn't.

Elena

 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 
 > 2001-10-01  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* symtab.h (struct block): (ALL_BLOCK_SYMBOLS): New macro.
 > 
 > 	* symtab.c (find_pc_sect_symtab): Use ALL_BLOCK_SYMBOLS.
 > 	(make_symbol_completion_list): Likewise.
 > 	(make_symbol_overload_list): Likewise.
 > 	* buildsym.c (finish_block): Likewise.
 > 	* breakpoint.c (get_catch_sals):  Likewise.
 > 	* mdebugread.c (mylookup_symbol): Likewise.
 > 	* objfiles.c (objfile_relocate): Likewise.
 > 	* printcmd.c (print_frame_args): Likewise.
 > 	* stack.c (print_block_frame_locals): Likewise.
 > 	(print_block_frame_labels): Likewise.
 > 	(print_frame_arg_vars): Likewise.
 > 	* symmisc.c (dump_symtab): Likewise.
 > 	* tracepoint.c (add_local_symbols): Likewise.
 > 	(scope_info): Likewise.
 > 
 > 2001-10-12  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* mi-cmd-stack.c (list_args_or_locals): Use ALL_BLOCK_SYMBOLS.
 > 
 > 2001-10-12  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* generic/gdbtk-cmds.c (gdb_listfuncs): Use ALL_BLOCK_SYMBOLS.
 > 	* generic/gdbtk-stack.c (gdb_block_vars): Likewise.
 > 	(gdb_get_blocks): Likewise.
 > 	(gdb_get_vars_command): Likewise.
 > 
 > Index: gdb/breakpoint.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/breakpoint.c,v
 > retrieving revision 1.53
 > diff -u -p -r1.53 breakpoint.c
 > --- gdb/breakpoint.c	2001/09/18 05:00:48	1.53
 > +++ gdb/breakpoint.c	2001/10/12 18:34:49
 > @@ -5871,15 +5871,11 @@ get_catch_sals (int this_level_only)
 >  	  if (blocks_searched[index] == 0)
 >  	    {
 >  	      struct block *b = BLOCKVECTOR_BLOCK (bl, index);
 > -	      int nsyms;
 >  	      register int i;
 >  	      register struct symbol *sym;
 >  
 > -	      nsyms = BLOCK_NSYMS (b);
 > -
 > -	      for (i = 0; i < nsyms; i++)
 > +	      ALL_BLOCK_SYMBOLS (b, i, sym)
 >  		{
 > -		  sym = BLOCK_SYM (b, i);
 >  		  if (STREQ (SYMBOL_NAME (sym), "default"))
 >  		    {
 >  		      if (have_default)
 > Index: gdb/buildsym.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/buildsym.c,v
 > retrieving revision 1.11
 > diff -u -p -r1.11 buildsym.c
 > --- gdb/buildsym.c	2001/04/30 10:30:27	1.11
 > +++ gdb/buildsym.c	2001/10/12 18:34:49
 > @@ -275,9 +275,8 @@ finish_block (struct symbol *symbol, str
 >  	     parameter symbols. */
 >  	  int nparams = 0, iparams;
 >  	  struct symbol *sym;
 > -	  for (i = 0; i < BLOCK_NSYMS (block); i++)
 > +	  ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	    {
 > -	      sym = BLOCK_SYM (block, i);
 >  	      switch (SYMBOL_CLASS (sym))
 >  		{
 >  		case LOC_ARG:
 > Index: gdb/mdebugread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mdebugread.c,v
 > retrieving revision 1.15
 > diff -u -p -r1.15 mdebugread.c
 > --- gdb/mdebugread.c	2001/09/05 02:54:15	1.15
 > +++ gdb/mdebugread.c	2001/10/12 18:34:50
 > @@ -3853,22 +3853,19 @@ static struct symbol *
 >  mylookup_symbol (char *name, register struct block *block,
 >  		 namespace_enum namespace, enum address_class class)
 >  {
 > -  register int bot, top, inc;
 > -  register struct symbol *sym;
 > +  int i, inc;
 > +  struct symbol *sym;
 >  
 > -  bot = 0;
 > -  top = BLOCK_NSYMS (block);
 >    inc = name[0];
 > -  while (bot < top)
 > +  ALL_BLOCK_SYMBOLS (block, i, sym)
 >      {
 > -      sym = BLOCK_SYM (block, bot);
 >        if (SYMBOL_NAME (sym)[0] == inc
 >  	  && SYMBOL_NAMESPACE (sym) == namespace
 >  	  && SYMBOL_CLASS (sym) == class
 >  	  && strcmp (SYMBOL_NAME (sym), name) == 0)
 >  	return sym;
 > -      bot++;
 >      }
 > +
 >    block = BLOCK_SUPERBLOCK (block);
 >    if (block)
 >      return mylookup_symbol (name, block, namespace, class);
 > Index: gdb/objfiles.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/objfiles.c,v
 > retrieving revision 1.16
 > diff -u -p -r1.16 objfiles.c
 > --- gdb/objfiles.c	2001/07/05 21:32:39	1.16
 > +++ gdb/objfiles.c	2001/10/12 18:34:50
 > @@ -557,16 +557,15 @@ objfile_relocate (struct objfile *objfil
 >        for (i = 0; i < BLOCKVECTOR_NBLOCKS (bv); ++i)
 >  	{
 >  	  struct block *b;
 > +	  struct symbol *sym;
 >  	  int j;
 >  
 >  	  b = BLOCKVECTOR_BLOCK (bv, i);
 >  	  BLOCK_START (b) += ANOFFSET (delta, s->block_line_section);
 >  	  BLOCK_END (b) += ANOFFSET (delta, s->block_line_section);
 >  
 > -	  for (j = 0; j < BLOCK_NSYMS (b); ++j)
 > +	  ALL_BLOCK_SYMBOLS (b, j, sym)
 >  	    {
 > -	      struct symbol *sym = BLOCK_SYM (b, j);
 > -
 >  	      fixup_symbol_section (sym, objfile);
 >  
 >  	      /* The RS6000 code from which this was taken skipped
 > Index: gdb/printcmd.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/printcmd.c,v
 > retrieving revision 1.28
 > diff -u -p -r1.28 printcmd.c
 > --- gdb/printcmd.c	2001/10/12 17:15:34	1.28
 > +++ gdb/printcmd.c	2001/10/12 18:34:50
 > @@ -1783,7 +1783,6 @@ print_frame_args (struct symbol *func, s
 >  		  struct ui_file *stream)
 >  {
 >    struct block *b = NULL;
 > -  int nsyms = 0;
 >    int first = 1;
 >    register int i;
 >    register struct symbol *sym;
 > @@ -1806,12 +1805,9 @@ print_frame_args (struct symbol *func, s
 >    if (func)
 >      {
 >        b = SYMBOL_BLOCK_VALUE (func);
 > -      nsyms = BLOCK_NSYMS (b);
 > -
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (b, i, sym)
 >          {
 >  	  QUIT;
 > -	  sym = BLOCK_SYM (b, i);
 >  
 >  	  /* Keep track of the highest stack argument offset seen, and
 >  	     skip over any kinds of symbols we don't care about.  */
 > Index: gdb/stack.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/stack.c,v
 > retrieving revision 1.22
 > diff -u -p -r1.22 stack.c
 > --- gdb/stack.c	2001/07/14 18:59:07	1.22
 > +++ gdb/stack.c	2001/10/12 18:34:50
 > @@ -1207,16 +1207,12 @@ static int
 >  print_block_frame_locals (struct block *b, register struct frame_info *fi,
 >  			  int num_tabs, register struct ui_file *stream)
 >  {
 > -  int nsyms;
 >    register int i, j;
 >    register struct symbol *sym;
 >    register int values_printed = 0;
 >  
 > -  nsyms = BLOCK_NSYMS (b);
 > -
 > -  for (i = 0; i < nsyms; i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        switch (SYMBOL_CLASS (sym))
 >  	{
 >  	case LOC_LOCAL:
 > @@ -1246,16 +1242,12 @@ static int
 >  print_block_frame_labels (struct block *b, int *have_default,
 >  			  register struct ui_file *stream)
 >  {
 > -  int nsyms;
 >    register int i;
 >    register struct symbol *sym;
 >    register int values_printed = 0;
 > -
 > -  nsyms = BLOCK_NSYMS (b);
 >  
 > -  for (i = 0; i < nsyms; i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        if (STREQ (SYMBOL_NAME (sym), "default"))
 >  	{
 >  	  if (*have_default)
 > @@ -1432,7 +1424,6 @@ print_frame_arg_vars (register struct fr
 >  {
 >    struct symbol *func = get_frame_function (fi);
 >    register struct block *b;
 > -  int nsyms;
 >    register int i;
 >    register struct symbol *sym, *sym2;
 >    register int values_printed = 0;
 > @@ -1444,11 +1435,8 @@ print_frame_arg_vars (register struct fr
 >      }
 >  
 >    b = SYMBOL_BLOCK_VALUE (func);
 > -  nsyms = BLOCK_NSYMS (b);
 > -
 > -  for (i = 0; i < nsyms; i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        switch (SYMBOL_CLASS (sym))
 >  	{
 >  	case LOC_ARG:
 > @@ -1483,7 +1471,6 @@ print_frame_arg_vars (register struct fr
 >  	  break;
 >  	}
 >      }
 > -
 >    if (!values_printed)
 >      {
 >        fprintf_filtered (stream, "No arguments.\n");
 > Index: gdb/symmisc.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symmisc.c,v
 > retrieving revision 1.5
 > diff -u -p -r1.5 symmisc.c
 > --- gdb/symmisc.c	2001/03/06 08:21:17	1.5
 > +++ gdb/symmisc.c	2001/10/12 18:34:50
 > @@ -410,6 +410,7 @@ dump_symtab (struct objfile *objfile, st
 >    int len, blen;
 >    register struct linetable *l;
 >    struct blockvector *bv;
 > +  struct symbol *sym;
 >    register struct block *b;
 >    int depth;
 >  
 > @@ -471,11 +472,12 @@ dump_symtab (struct objfile *objfile, st
 >  	  if (BLOCK_GCC_COMPILED (b))
 >  	    fprintf_filtered (outfile, ", compiled with gcc%d", BLOCK_GCC_COMPILED (b));
 >  	  fprintf_filtered (outfile, "\n");
 > -	  /* Now print each symbol in this block */
 > -	  for (j = 0; j < blen; j++)
 > +	  /* Now print each symbol in this block.  */
 > +	  /* FIXMED: Sort?  */
 > +	  ALL_BLOCK_SYMBOLS (b, j, sym)
 >  	    {
 >  	      struct print_symbol_args s;
 > -	      s.symbol = BLOCK_SYM (b, j);
 > +	      s.symbol = sym;
 >  	      s.depth = depth + 1;
 >  	      s.outfile = outfile;
 >  	      catch_errors (print_symbol, &s, "Error printing symbol:\n",
 > Index: gdb/symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.43
 > diff -u -p -r1.43 symtab.c
 > --- gdb/symtab.c	2001/10/12 03:38:12	1.43
 > +++ gdb/symtab.c	2001/10/12 18:34:50
 > @@ -2992,9 +2992,8 @@ make_symbol_completion_list (char *text,
 >        /* Also catch fields of types defined in this places which match our
 >           text string.  Only complete on types visible from current context. */
 >  
 > -      for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +      ALL_BLOCK_SYMBOLS (b, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (b, i);
 >  	  COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >  	  if (SYMBOL_CLASS (sym) == LOC_TYPEDEF)
 >  	    {
 > @@ -3023,9 +3022,8 @@ make_symbol_completion_list (char *text,
 >    {
 >      QUIT;
 >      b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >        }
 >    }
 > @@ -3037,9 +3035,8 @@ make_symbol_completion_list (char *text,
 >      /* Don't do this block twice.  */
 >      if (b == surrounding_static_block)
 >        continue;
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >        }
 >    }
 > @@ -3143,16 +3140,14 @@ make_file_symbol_completion_list (char *
 >       symbols which match.  */
 >  
 >    b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > -  for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >      }
 >  
 >    b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
 > -  for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +  ALL_BLOCK_SYMBOLS (b, i, sym)
 >      {
 > -      sym = BLOCK_SYM (b, i);
 >        COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word);
 >      }
 >  
 > @@ -3530,9 +3525,8 @@ make_symbol_overload_list (struct symbol
 >        /* Also catch fields of types defined in this places which match our
 >           text string.  Only complete on types visible from current context. */
 >  
 > -      for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +      ALL_BLOCK_SYMBOLS (b, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (b, i);
 >  	  overload_list_add_symbol (sym, oload_name);
 >  	}
 >      }
 > @@ -3544,9 +3538,8 @@ make_symbol_overload_list (struct symbol
 >    {
 >      QUIT;
 >      b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	overload_list_add_symbol (sym, oload_name);
 >        }
 >    }
 > @@ -3558,9 +3551,8 @@ make_symbol_overload_list (struct symbol
 >      /* Don't do this block twice.  */
 >      if (b == surrounding_static_block)
 >        continue;
 > -    for (i = 0; i < BLOCK_NSYMS (b); i++)
 > +    ALL_BLOCK_SYMBOLS (b, i, sym)
 >        {
 > -	sym = BLOCK_SYM (b, i);
 >  	overload_list_add_symbol (sym, oload_name);
 >        }
 >    }
 > Index: gdb/symtab.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.h,v
 > retrieving revision 1.24
 > diff -u -p -r1.24 symtab.h
 > --- gdb/symtab.h	2001/07/07 17:19:50	1.24
 > +++ gdb/symtab.h	2001/10/12 18:34:50
 > @@ -467,6 +467,14 @@ struct block
 >  #define BLOCK_SUPERBLOCK(bl)	(bl)->superblock
 >  #define BLOCK_GCC_COMPILED(bl)	(bl)->gcc_compile_flag
 >  
 > +/* Macro to loop through all symbols in a block BL.
 > +   i counts which symbol we are looking at, and sym points to the current
 > +   symbol.  */
 > +#define ALL_BLOCK_SYMBOLS(bl, i, sym)			\
 > +	for ((i) = 0, (sym) = BLOCK_SYM ((bl), (i));	\
 > +	     (i) < BLOCK_NSYMS ((bl));			\
 > +	     ++(i), (sym) = BLOCK_SYM ((bl), (i)))
 > +
 >  /* Nonzero if symbols of block BL should be sorted alphabetically.
 >     Don't sort a block which corresponds to a function.  If we did the
 >     sorting would have to preserve the order of the symbols for the
 > Index: gdb/tracepoint.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/tracepoint.c,v
 > retrieving revision 1.26
 > diff -u -p -r1.26 tracepoint.c
 > --- gdb/tracepoint.c	2001/08/23 20:31:36	1.26
 > +++ gdb/tracepoint.c	2001/10/12 18:34:50
 > @@ -1296,16 +1296,14 @@ add_local_symbols (struct collection_lis
 >  {
 >    struct symbol *sym;
 >    struct block *block;
 > -  int i, nsyms, count = 0;
 > +  int i, count = 0;
 >  
 >    block = block_for_pc (pc);
 >    while (block != 0)
 >      {
 >        QUIT;			/* allow user to bail out with ^C */
 > -      nsyms = BLOCK_NSYMS (block);
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (block, i);
 >  	  switch (SYMBOL_CLASS (sym))
 >  	    {
 >  	    default:
 > @@ -2335,7 +2333,7 @@ scope_info (char *args, int from_tty)
 >    struct minimal_symbol *msym;
 >    struct block *block;
 >    char **canonical, *symname, *save_args = args;
 > -  int i, j, nsyms, count = 0;
 > +  int i, j, count = 0;
 >  
 >    if (args == 0 || *args == 0)
 >      error ("requires an argument (function, line or *addr) to define a scope");
 > @@ -2351,14 +2349,13 @@ scope_info (char *args, int from_tty)
 >    while (block != 0)
 >      {
 >        QUIT;			/* allow user to bail out with ^C */
 > -      nsyms = BLOCK_NSYMS (block);
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	{
 >  	  QUIT;			/* allow user to bail out with ^C */
 >  	  if (count == 0)
 >  	    printf_filtered ("Scope for %s:\n", save_args);
 >  	  count++;
 > -	  sym = BLOCK_SYM (block, i);
 > +
 >  	  symname = SYMBOL_NAME (sym);
 >  	  if (symname == NULL || *symname == '\0')
 >  	    continue;		/* probably botched, certainly useless */
 > Index: gdb/gdbtk/generic/gdbtk-cmds.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-cmds.c,v
 > retrieving revision 1.39
 > diff -u -p -r1.39 gdbtk-cmds.c
 > --- gdb/gdbtk/generic/gdbtk-cmds.c	2001/08/28 22:22:56	1.39
 > +++ gdb/gdbtk/generic/gdbtk-cmds.c	2001/10/12 18:34:50
 > @@ -1498,9 +1498,8 @@ gdb_listfuncs (clientData, interp, objc,
 >        /* Skip the sort if this block is always sorted.  */
 >        if (!BLOCK_SHOULD_SORT (b))
 >  	sort_block_syms (b);
 > -      for (j = 0; j < BLOCK_NSYMS (b); j++)
 > +      ALL_BLOCK_SYMBOLS (b, j, sym)
 >  	{
 > -	  sym = BLOCK_SYM (b, j);
 >  	  if (SYMBOL_CLASS (sym) == LOC_BLOCK)
 >  	    {
 >  
 > Index: gdb/gdbtk/generic/gdbtk-stack.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-stack.c,v
 > retrieving revision 1.2
 > diff -u -p -r1.2 gdbtk-stack.c
 > --- gdb/gdbtk/generic/gdbtk-stack.c	2001/06/11 20:07:46	1.2
 > +++ gdb/gdbtk/generic/gdbtk-stack.c	2001/10/12 18:34:50
 > @@ -19,6 +19,7 @@
 >     Boston, MA 02111-1307, USA.  */
 >  
 >  #include "defs.h"
 > +#include "symtab.h"
 >  #include "frame.h"
 >  #include "value.h"
 >  #include "target.h"
 > @@ -93,7 +94,7 @@ gdb_block_vars (clientData, interp, objc
 >       Tcl_Obj *CONST objv[];
 >  {
 >    struct block *block;
 > -  int nsyms, i;
 > +  int i;
 >    struct symbol *sym;
 >    CORE_ADDR start, end;
 >  
 > @@ -117,10 +118,8 @@ gdb_block_vars (clientData, interp, objc
 >      {
 >        if (BLOCK_START (block) == start && BLOCK_END (block) == end)
 >  	{
 > -	  nsyms = BLOCK_NSYMS (block);
 > -	  for (i = 0; i < nsyms; i++)
 > +	  ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	    {
 > -	      sym = BLOCK_SYM (block, i);
 >  	      switch (SYMBOL_CLASS (sym))
 >  		{
 >  		case LOC_ARG:		  /* argument              */
 > @@ -172,7 +171,7 @@ gdb_get_blocks (clientData, interp, objc
 >       Tcl_Obj *CONST objv[];
 >  {
 >    struct block *block;
 > -  int nsyms, i, junk;
 > +  int i, junk;
 >    struct symbol *sym;
 >    CORE_ADDR pc;
 >  
 > @@ -184,11 +183,9 @@ gdb_get_blocks (clientData, interp, objc
 >        pc = get_frame_pc (selected_frame);
 >        while (block != 0)
 >  	{
 > -	  nsyms = BLOCK_NSYMS (block);
 >  	  junk = 0;
 > -	  for (i = 0; i < nsyms; i++)
 > +	  ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	    {
 > -	      sym = BLOCK_SYM (block, i);
 >  	      switch (SYMBOL_CLASS (sym))
 >  		{
 >  		default:
 > @@ -301,7 +298,7 @@ gdb_get_vars_command (clientData, interp
 >    struct symbol *sym;
 >    struct block *block;
 >    char **canonical, *args;
 > -  int i, nsyms, arguments;
 > +  int i, arguments;
 >  
 >    if (objc > 2)
 >      {
 > @@ -344,10 +341,8 @@ gdb_get_vars_command (clientData, interp
 >  
 >    while (block != 0)
 >      {
 > -      nsyms = BLOCK_NSYMS (block);
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (block, i);
 >  	  switch (SYMBOL_CLASS (sym))
 >  	    {
 >  	    default:
 > Index: gdb/mi/mi-cmd-stack.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mi/mi-cmd-stack.c,v
 > retrieving revision 1.7
 > diff -u -p -r1.7 mi-cmd-stack.c
 > --- gdb/mi/mi-cmd-stack.c	2001/06/26 18:50:51	1.7
 > +++ gdb/mi/mi-cmd-stack.c	2001/10/12 18:34:50
 > @@ -25,6 +25,7 @@
 >  #include "value.h"
 >  #include "mi-cmds.h"
 >  #include "ui-out.h"
 > +#include "symtab.h"
 >  
 >  #ifdef UI_OUT
 >  /* FIXME: these should go in some .h file but stack.c doesn't have a
 > @@ -226,10 +227,8 @@ list_args_or_locals (int locals, int val
 >  
 >    while (block != 0)
 >      {
 > -      nsyms = BLOCK_NSYMS (block);
 > -      for (i = 0; i < nsyms; i++)
 > +      ALL_BLOCK_SYMBOLS (block, i, sym)
 >  	{
 > -	  sym = BLOCK_SYM (block, i);
 >  	  switch (SYMBOL_CLASS (sym))
 >  	    {
 >  	    default:

Re: [rfa] symbol hashing, part 2/n - ALL_BLOCK_SYMBOLS

[Daniel Jacobowitz]

On Fri, Oct 12, 2001 at 02:58:34PM -0400, Daniel Jacobowitz wrote:
> Here's what I've got now.  I'll commit it later today unless someone
> sees a problem with it.

No one did, so it's committed.

The next one might even be interesting!

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer