[rfa] linespec.c, part 5

[rfa] linespec.c, part 5

[David Carlton]

[ Just what is it that causes my fingers to confuse C-cs and C-cC-s at
the start of typing a message, even though my fingers never confuse
them at the end of typing a message?  Sigh... ]

The next part: decode_compound, to handle C++ and Java compound data
structures.  Some notes:

* The function that this creates, decode_compound, is quite long.
  Later patches in this series will break it down into smaller chunks;
  they will come after I've got decode_line_1 itself down to a nice
  size.

* To get this to compile without warnings, I changed the declaration
  of cplusplus_error to note that it doesn't return.

* While preparing this patch, I noticed that the variable
  gdb_completer_quote_characters is declared but not used; I got rid
  of it.

* Since decode_line_1 returns the value of decode_compound, there's no
  need to check whether or not decode_compound modifies the arguments
  that it's been passed: later code in decode_line_1 can never be
  effected by such modifications.

David Carlton
carlton@math.stanford.edu

2002-11-15  David Carlton  <carlton@math.stanford.edu>

	* linespec.c (decode_compound): New function.
	(decode_line_1): Move code into decode_compound.

--- linespec.c-4	Tue Nov 12 14:07:59 2002
+++ linespec.c		Tue Nov 12 15:40:10 2002
@@ -48,7 +48,15 @@ static struct symtabs_and_lines decode_i
 
 static char *locate_first_half (char **argptr, int *is_quote_enclosed);
 
-static void cplusplus_error (const char *name, const char *fmt, ...) ATTR_FORMAT (printf, 2, 3);
+static struct symtabs_and_lines decode_compound (char **argptr,
+						 int funfirstline,
+						 char ***canonical,
+						 char *saved_arg,
+						 char *p);
+
+static NORETURN void cplusplus_error (const char *name,
+				      const char *fmt, ...)
+     ATTR_NORETURN ATTR_FORMAT (printf, 2, 3);
 
 static int total_number_of_methods (struct type *type);
 
@@ -80,7 +88,7 @@ symtabs_and_lines minsym_found (int funf
    single quoted demangled C++ symbols as part of the completion
    error.  */
 
-static void
+static NORETURN void
 cplusplus_error (const char *name, const char *fmt, ...)
 {
   struct ui_file *tmp_stream;
@@ -532,10 +540,7 @@ decode_line_1 (char **argptr, int funfir
   struct symtabs_and_lines values;
   struct symtab_and_line val;
   register char *p, *p1;
-  char *q, *p2;
-#if 0
-  char *q1;
-#endif
+  char *q;
   register struct symtab *s = NULL;
 
   register struct symbol *sym;
@@ -544,8 +549,6 @@ decode_line_1 (char **argptr, int funfir
 
   register struct minimal_symbol *msymbol;
   char *copy;
-  struct symbol *sym_class;
-  int i1;
   /* This is NULL if there are no parens in *ARGPTR, or a pointer to
      the closing parenthesis if there are parens.  */
   char *paren_pointer;
@@ -554,10 +557,7 @@ decode_line_1 (char **argptr, int funfir
   int is_quoted;
   /* Is part of *ARGPTR is enclosed in double quotes?  */
   int is_quote_enclosed;
-  struct symbol **sym_arr;
-  struct type *t;
   char *saved_arg = *argptr;
-  extern char *gdb_completer_quote_characters;
 
   init_sal (&val);		/* initialize to zeroes */
 
@@ -589,237 +589,13 @@ decode_line_1 (char **argptr, int funfir
 
   if ((p[0] == ':' || p[0] == '.') && paren_pointer == NULL)
     {
-      /*  C++ */
-      /*  ... or Java */
       if (is_quoted)
 	*argptr = *argptr + 1;
       if (p[0] == '.' || p[1] == ':')
-	{
-	  char *saved_arg2 = *argptr;
-	  char *temp_end;
-	  /* First check for "global" namespace specification,
-	     of the form "::foo". If found, skip over the colons
-	     and jump to normal symbol processing */
-	  if (p[0] == ':' 
-	      && ((*argptr == p) || (p[-1] == ' ') || (p[-1] == '\t')))
-	    saved_arg2 += 2;
-
-	  /* We have what looks like a class or namespace
-	     scope specification (A::B), possibly with many
-	     levels of namespaces or classes (A::B::C::D).
-
-	     Some versions of the HP ANSI C++ compiler (as also possibly
-	     other compilers) generate class/function/member names with
-	     embedded double-colons if they are inside namespaces. To
-	     handle this, we loop a few times, considering larger and
-	     larger prefixes of the string as though they were single
-	     symbols.  So, if the initially supplied string is
-	     A::B::C::D::foo, we have to look up "A", then "A::B",
-	     then "A::B::C", then "A::B::C::D", and finally
-	     "A::B::C::D::foo" as single, monolithic symbols, because
-	     A, B, C or D may be namespaces.
-
-	     Note that namespaces can nest only inside other
-	     namespaces, and not inside classes.  So we need only
-	     consider *prefixes* of the string; there is no need to look up
-	     "B::C" separately as a symbol in the previous example. */
-
-	  p2 = p;		/* save for restart */
-	  while (1)
-	    {
-	      /* Extract the class name.  */
-	      p1 = p;
-	      while (p != *argptr && p[-1] == ' ')
-		--p;
-	      copy = (char *) alloca (p - *argptr + 1);
-	      memcpy (copy, *argptr, p - *argptr);
-	      copy[p - *argptr] = 0;
-
-	      /* Discard the class name from the arg.  */
-	      p = p1 + (p1[0] == ':' ? 2 : 1);
-	      while (*p == ' ' || *p == '\t')
-		p++;
-	      *argptr = p;
-
-	      sym_class = lookup_symbol (copy, 0, STRUCT_NAMESPACE, 0,
-					 (struct symtab **) NULL);
-
-	      if (sym_class &&
-		  (t = check_typedef (SYMBOL_TYPE (sym_class)),
-		   (TYPE_CODE (t) == TYPE_CODE_STRUCT
-		    || TYPE_CODE (t) == TYPE_CODE_UNION)))
-		{
-		  /* Arg token is not digits => try it as a function name
-		     Find the next token(everything up to end or next blank). */
-		  if (**argptr
-		      && strchr (get_gdb_completer_quote_characters (),
-				 **argptr) != NULL)
-		    {
-		      p = skip_quoted (*argptr);
-		      *argptr = *argptr + 1;
-		    }
-		  else
-		    {
-		      p = *argptr;
-		      while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':')
-			p++;
-		    }
-/*
-   q = operator_chars (*argptr, &q1);
-   if (q1 - q)
-   {
-   char *opname;
-   char *tmp = alloca (q1 - q + 1);
-   memcpy (tmp, q, q1 - q);
-   tmp[q1 - q] = '\0';
-   opname = cplus_mangle_opname (tmp, DMGL_ANSI);
-   if (opname == NULL)
-   {
-   cplusplus_error (saved_arg, "no mangling for \"%s\"\n", tmp);
-   }
-   copy = (char*) alloca (3 + strlen(opname));
-   sprintf (copy, "__%s", opname);
-   p = q1;
-   }
-   else
- */
-		  {
-		    copy = (char *) alloca (p - *argptr + 1);
-		    memcpy (copy, *argptr, p - *argptr);
-		    copy[p - *argptr] = '\0';
-		    if (p != *argptr
-			&& copy[p - *argptr - 1]
-			&& strchr (get_gdb_completer_quote_characters (),
-				   copy[p - *argptr - 1]) != NULL)
-		      copy[p - *argptr - 1] = '\0';
-		  }
-
-		  /* no line number may be specified */
-		  while (*p == ' ' || *p == '\t')
-		    p++;
-		  *argptr = p;
-
-		  sym = 0;
-		  i1 = 0;	/*  counter for the symbol array */
-		  sym_arr = (struct symbol **) alloca (total_number_of_methods (t)
-						* sizeof (struct symbol *));
-
-		  if (destructor_name_p (copy, t))
-		    {
-		      /* Destructors are a special case.  */
-		      int m_index, f_index;
-
-		      if (get_destructor_fn_field (t, &m_index, &f_index))
-			{
-			  struct fn_field *f = TYPE_FN_FIELDLIST1 (t, m_index);
-
-			  sym_arr[i1] =
-			    lookup_symbol (TYPE_FN_FIELD_PHYSNAME (f, f_index),
-					   NULL, VAR_NAMESPACE, (int *) NULL,
-					   (struct symtab **) NULL);
-			  if (sym_arr[i1])
-			    i1++;
-			}
-		    }
-		  else
-		    i1 = find_methods (t, copy, sym_arr);
-		  if (i1 == 1)
-		    {
-		      /* There is exactly one field with that name.  */
-		      sym = sym_arr[0];
-
-		      if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
-			{
-			  values.sals = (struct symtab_and_line *)
-			    xmalloc (sizeof (struct symtab_and_line));
-			  values.nelts = 1;
-			  values.sals[0] = find_function_start_sal (sym,
-							      funfirstline);
-			}
-		      else
-			{
-			  values.nelts = 0;
-			}
-		      return values;
-		    }
-		  if (i1 > 0)
-		    {
-		      /* There is more than one field with that name
-		         (overloaded).  Ask the user which one to use.  */
-		      return decode_line_2 (sym_arr, i1, funfirstline, canonical);
-		    }
-		  else
-		    {
-		      char *tmp;
-
-		      if (is_operator_name (copy))
-			{
-			  tmp = (char *) alloca (strlen (copy + 3) + 9);
-			  strcpy (tmp, "operator ");
-			  strcat (tmp, copy + 3);
-			}
-		      else
-			tmp = copy;
-		      if (tmp[0] == '~')
-			cplusplus_error (saved_arg,
-					 "the class `%s' does not have destructor defined\n",
-					 SYMBOL_SOURCE_NAME (sym_class));
-		      else
-			cplusplus_error (saved_arg,
-					 "the class %s does not have any method named %s\n",
-					 SYMBOL_SOURCE_NAME (sym_class), tmp);
-		    }
-		}
-
-	      /* Move pointer up to next possible class/namespace token */
-	      p = p2 + 1;	/* restart with old value +1 */
-	      /* Move pointer ahead to next double-colon */
-	      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\''))
-		{
-		  if (p[0] == '<')
-		    {
-		      temp_end = find_template_name_end (p);
-		      if (!temp_end)
-			error ("malformed template specification in command");
-		      p = temp_end;
-		    }
-		  else if ((p[0] == ':') && (p[1] == ':'))
-		    break;	/* found double-colon */
-		  else
-		    p++;
-		}
-
-	      if (*p != ':')
-		break;		/* out of the while (1) */
-
-	      p2 = p;		/* save restart for next time around */
-	      *argptr = saved_arg2;	/* restore argptr */
-	    }			/* while (1) */
-
-	  /* Last chance attempt -- check entire name as a symbol */
-	  /* Use "copy" in preparation for jumping out of this block,
-	     to be consistent with usage following the jump target */
-	  copy = (char *) alloca (p - saved_arg2 + 1);
-	  memcpy (copy, saved_arg2, p - saved_arg2);
-	  /* Note: if is_quoted should be true, we snuff out quote here anyway */
-	  copy[p - saved_arg2] = '\000';
-	  /* Set argptr to skip over the name */
-	  *argptr = (*p == '\'') ? p + 1 : p;
-	  /* Look up entire name */
-	  sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
-	  s = (struct symtab *) 0;
-	  if (sym)
-	    return symbol_found (funfirstline, canonical, copy, sym,
-				 NULL, sym_symtab);
-
-	  /* Couldn't find any interpretation as classes/namespaces, so give up */
-	  /* The quotes are important if copy is empty.  */
-	  cplusplus_error (saved_arg,
-			   "Can't find member of namespace, class, struct, or union named \"%s\"\n",
-			   copy);
-	}
-      /*  end of C++  */
-
+	/*  C++ */
+	/*  ... or Java */
+	return decode_compound (argptr, funfirstline, canonical,
+				saved_arg, p);
 
       /* Extract the file name.  */
       p1 = p;
@@ -1281,6 +1057,252 @@ locate_first_half (char **argptr, int *i
 }
 
 \f
+
+/* This handles C++ and Java compound data structures.  P should point
+   at the first component separator, i.e. double-colon or period.  */
+
+static struct symtabs_and_lines
+decode_compound (char **argptr, int funfirstline, char ***canonical,
+		 char *saved_arg, char *p)
+{
+  struct symtabs_and_lines values;
+  char *p1, *p2;
+#if 0
+  char *q, *q1;
+#endif
+  char *saved_arg2 = *argptr;
+  char *temp_end;
+  struct symbol *sym;
+  /* The symtab that SYM was found in.  */
+  struct symtab *sym_symtab;
+  char *copy;
+  struct symbol *sym_class;
+  int i1;
+  struct symbol **sym_arr;
+  struct type *t;
+
+  /* First check for "global" namespace specification,
+     of the form "::foo". If found, skip over the colons
+     and jump to normal symbol processing */
+  if (p[0] == ':' 
+      && ((*argptr == p) || (p[-1] == ' ') || (p[-1] == '\t')))
+    saved_arg2 += 2;
+
+  /* We have what looks like a class or namespace
+     scope specification (A::B), possibly with many
+     levels of namespaces or classes (A::B::C::D).
+
+     Some versions of the HP ANSI C++ compiler (as also possibly
+     other compilers) generate class/function/member names with
+     embedded double-colons if they are inside namespaces. To
+     handle this, we loop a few times, considering larger and
+     larger prefixes of the string as though they were single
+     symbols.  So, if the initially supplied string is
+     A::B::C::D::foo, we have to look up "A", then "A::B",
+     then "A::B::C", then "A::B::C::D", and finally
+     "A::B::C::D::foo" as single, monolithic symbols, because
+     A, B, C or D may be namespaces.
+
+     Note that namespaces can nest only inside other
+     namespaces, and not inside classes.  So we need only
+     consider *prefixes* of the string; there is no need to look up
+     "B::C" separately as a symbol in the previous example. */
+
+  p2 = p;		/* save for restart */
+  while (1)
+    {
+      /* Extract the class name.  */
+      p1 = p;
+      while (p != *argptr && p[-1] == ' ')
+	--p;
+      copy = (char *) alloca (p - *argptr + 1);
+      memcpy (copy, *argptr, p - *argptr);
+      copy[p - *argptr] = 0;
+
+      /* Discard the class name from the arg.  */
+      p = p1 + (p1[0] == ':' ? 2 : 1);
+      while (*p == ' ' || *p == '\t')
+	p++;
+      *argptr = p;
+
+      sym_class = lookup_symbol (copy, 0, STRUCT_NAMESPACE, 0,
+				 (struct symtab **) NULL);
+
+      if (sym_class &&
+	  (t = check_typedef (SYMBOL_TYPE (sym_class)),
+	   (TYPE_CODE (t) == TYPE_CODE_STRUCT
+	    || TYPE_CODE (t) == TYPE_CODE_UNION)))
+	{
+	  /* Arg token is not digits => try it as a function name
+	     Find the next token(everything up to end or next blank). */
+	  if (**argptr
+	      && strchr (get_gdb_completer_quote_characters (),
+			 **argptr) != NULL)
+	    {
+	      p = skip_quoted (*argptr);
+	      *argptr = *argptr + 1;
+	    }
+	  else
+	    {
+	      p = *argptr;
+	      while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':')
+		p++;
+	    }
+/*
+   q = operator_chars (*argptr, &q1);
+   if (q1 - q)
+   {
+   char *opname;
+   char *tmp = alloca (q1 - q + 1);
+   memcpy (tmp, q, q1 - q);
+   tmp[q1 - q] = '\0';
+   opname = cplus_mangle_opname (tmp, DMGL_ANSI);
+   if (opname == NULL)
+   {
+   cplusplus_error (saved_arg, "no mangling for \"%s\"\n", tmp);
+   }
+   copy = (char*) alloca (3 + strlen(opname));
+   sprintf (copy, "__%s", opname);
+   p = q1;
+   }
+   else
+ */
+	  {
+	    copy = (char *) alloca (p - *argptr + 1);
+	    memcpy (copy, *argptr, p - *argptr);
+	    copy[p - *argptr] = '\0';
+	    if (p != *argptr
+		&& copy[p - *argptr - 1]
+		&& strchr (get_gdb_completer_quote_characters (),
+			   copy[p - *argptr - 1]) != NULL)
+	      copy[p - *argptr - 1] = '\0';
+	  }
+
+	  /* no line number may be specified */
+	  while (*p == ' ' || *p == '\t')
+	    p++;
+	  *argptr = p;
+
+	  sym = 0;
+	  i1 = 0;	/*  counter for the symbol array */
+	  sym_arr = (struct symbol **) alloca (total_number_of_methods (t)
+					       * sizeof (struct symbol *));
+
+	  if (destructor_name_p (copy, t))
+	    {
+	      /* Destructors are a special case.  */
+	      int m_index, f_index;
+
+	      if (get_destructor_fn_field (t, &m_index, &f_index))
+		{
+		  struct fn_field *f = TYPE_FN_FIELDLIST1 (t, m_index);
+
+		  sym_arr[i1] =
+		    lookup_symbol (TYPE_FN_FIELD_PHYSNAME (f, f_index),
+				   NULL, VAR_NAMESPACE, (int *) NULL,
+				   (struct symtab **) NULL);
+		  if (sym_arr[i1])
+		    i1++;
+		}
+	    }
+	  else
+	    i1 = find_methods (t, copy, sym_arr);
+	  if (i1 == 1)
+	    {
+	      /* There is exactly one field with that name.  */
+	      sym = sym_arr[0];
+
+	      if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
+		{
+		  values.sals = (struct symtab_and_line *)
+		    xmalloc (sizeof (struct symtab_and_line));
+		  values.nelts = 1;
+		  values.sals[0] = find_function_start_sal (sym,
+							    funfirstline);
+		}
+	      else
+		{
+		  values.nelts = 0;
+		}
+	      return values;
+	    }
+	  if (i1 > 0)
+	    {
+	      /* There is more than one field with that name
+		 (overloaded).  Ask the user which one to use.  */
+	      return decode_line_2 (sym_arr, i1, funfirstline, canonical);
+	    }
+	  else
+	    {
+	      char *tmp;
+
+	      if (is_operator_name (copy))
+		{
+		  tmp = (char *) alloca (strlen (copy + 3) + 9);
+		  strcpy (tmp, "operator ");
+		  strcat (tmp, copy + 3);
+		}
+	      else
+		tmp = copy;
+	      if (tmp[0] == '~')
+		cplusplus_error (saved_arg,
+				 "the class `%s' does not have destructor defined\n",
+				 SYMBOL_SOURCE_NAME (sym_class));
+	      else
+		cplusplus_error (saved_arg,
+				 "the class %s does not have any method named %s\n",
+				 SYMBOL_SOURCE_NAME (sym_class), tmp);
+	    }
+	}
+
+      /* Move pointer up to next possible class/namespace token */
+      p = p2 + 1;	/* restart with old value +1 */
+      /* Move pointer ahead to next double-colon */
+      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\''))
+	{
+	  if (p[0] == '<')
+	    {
+	      temp_end = find_template_name_end (p);
+	      if (!temp_end)
+		error ("malformed template specification in command");
+	      p = temp_end;
+	    }
+	  else if ((p[0] == ':') && (p[1] == ':'))
+	    break;	/* found double-colon */
+	  else
+	    p++;
+	}
+
+      if (*p != ':')
+	break;		/* out of the while (1) */
+
+      p2 = p;		/* save restart for next time around */
+      *argptr = saved_arg2;	/* restore argptr */
+    }			/* while (1) */
+
+  /* Last chance attempt -- check entire name as a symbol */
+  /* Use "copy" in preparation for jumping out of this block,
+     to be consistent with usage following the jump target */
+  copy = (char *) alloca (p - saved_arg2 + 1);
+  memcpy (copy, saved_arg2, p - saved_arg2);
+  /* Note: if is_quoted should be true, we snuff out quote here anyway */
+  copy[p - saved_arg2] = '\000';
+  /* Set argptr to skip over the name */
+  *argptr = (*p == '\'') ? p + 1 : p;
+  /* Look up entire name */
+  sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
+  if (sym)
+    return symbol_found (funfirstline, canonical, copy, sym,
+			 NULL, sym_symtab);
+
+  /* Couldn't find any interpretation as classes/namespaces, so give up */
+  /* The quotes are important if copy is empty.  */
+  cplusplus_error (saved_arg,
+		   "Can't find member of namespace, class, struct, or union named \"%s\"\n",
+		   copy);
+}
+
+\f
 
 /* Now come some functions that are called from multiple places within
    decode_line_1.  */

Re: [rfa] linespec.c, part 5

[Elena Zannoni]

David Carlton writes:
 > [ Just what is it that causes my fingers to confuse C-cs and C-cC-s at
 > the start of typing a message, even though my fingers never confuse
 > them at the end of typing a message?  Sigh... ]
 > 
 > The next part: decode_compound, to handle C++ and Java compound data
 > structures.  Some notes:
 > 
 > * The function that this creates, decode_compound, is quite long.
 >   Later patches in this series will break it down into smaller chunks;
 >   they will come after I've got decode_line_1 itself down to a nice
 >   size.
 > 

OK but, could you add something to the comment at the top of the
function that describes what the function returns and what the params
are?

 > * To get this to compile without warnings, I changed the declaration
 >   of cplusplus_error to note that it doesn't return.
 > 

I think this is OK.

 > * While preparing this patch, I noticed that the variable
 >   gdb_completer_quote_characters is declared but not used; I got rid
 >   of it.
 > 

good.

 > * Since decode_line_1 returns the value of decode_compound, there's no
 >   need to check whether or not decode_compound modifies the arguments
 >   that it's been passed: later code in decode_line_1 can never be
 >   effected by such modifications.
 > 

Hmmm. maybe I am not understanding what you say. Does decode_compound
modify its args? seems like it changes argptr and canonical. What if
somebody changes decode_line_1 in the future so that there is more
code executed after the call, it could probably cause some head
scratching to see that the call didn't leave things as expected. I
haven't looked too closely though.

thanks
Elena


 > David Carlton
 > carlton@math.stanford.edu
 > 
 > 2002-11-15  David Carlton  <carlton@math.stanford.edu>
 > 
 > 	* linespec.c (decode_compound): New function.
 > 	(decode_line_1): Move code into decode_compound.
 > 
 > --- linespec.c-4	Tue Nov 12 14:07:59 2002
 > +++ linespec.c		Tue Nov 12 15:40:10 2002
 > @@ -48,7 +48,15 @@ static struct symtabs_and_lines decode_i
 >  
 >  static char *locate_first_half (char **argptr, int *is_quote_enclosed);
 >  
 > -static void cplusplus_error (const char *name, const char *fmt, ...) ATTR_FORMAT (printf, 2, 3);
 > +static struct symtabs_and_lines decode_compound (char **argptr,
 > +						 int funfirstline,
 > +						 char ***canonical,
 > +						 char *saved_arg,
 > +						 char *p);
 > +
 > +static NORETURN void cplusplus_error (const char *name,
 > +				      const char *fmt, ...)
 > +     ATTR_NORETURN ATTR_FORMAT (printf, 2, 3);
 >  
 >  static int total_number_of_methods (struct type *type);
 >  
 > @@ -80,7 +88,7 @@ symtabs_and_lines minsym_found (int funf
 >     single quoted demangled C++ symbols as part of the completion
 >     error.  */
 >  
 > -static void
 > +static NORETURN void
 >  cplusplus_error (const char *name, const char *fmt, ...)
 >  {
 >    struct ui_file *tmp_stream;
 > @@ -532,10 +540,7 @@ decode_line_1 (char **argptr, int funfir
 >    struct symtabs_and_lines values;
 >    struct symtab_and_line val;
 >    register char *p, *p1;
 > -  char *q, *p2;
 > -#if 0
 > -  char *q1;
 > -#endif
 > +  char *q;
 >    register struct symtab *s = NULL;
 >  
 >    register struct symbol *sym;
 > @@ -544,8 +549,6 @@ decode_line_1 (char **argptr, int funfir
 >  
 >    register struct minimal_symbol *msymbol;
 >    char *copy;
 > -  struct symbol *sym_class;
 > -  int i1;
 >    /* This is NULL if there are no parens in *ARGPTR, or a pointer to
 >       the closing parenthesis if there are parens.  */
 >    char *paren_pointer;
 > @@ -554,10 +557,7 @@ decode_line_1 (char **argptr, int funfir
 >    int is_quoted;
 >    /* Is part of *ARGPTR is enclosed in double quotes?  */
 >    int is_quote_enclosed;
 > -  struct symbol **sym_arr;
 > -  struct type *t;
 >    char *saved_arg = *argptr;
 > -  extern char *gdb_completer_quote_characters;
 >  
 >    init_sal (&val);		/* initialize to zeroes */
 >  
 > @@ -589,237 +589,13 @@ decode_line_1 (char **argptr, int funfir
 >  
 >    if ((p[0] == ':' || p[0] == '.') && paren_pointer == NULL)
 >      {
 > -      /*  C++ */
 > -      /*  ... or Java */
 >        if (is_quoted)
 >  	*argptr = *argptr + 1;
 >        if (p[0] == '.' || p[1] == ':')
 > -	{
 > -	  char *saved_arg2 = *argptr;
 > -	  char *temp_end;
 > -	  /* First check for "global" namespace specification,
 > -	     of the form "::foo". If found, skip over the colons
 > -	     and jump to normal symbol processing */
 > -	  if (p[0] == ':' 
 > -	      && ((*argptr == p) || (p[-1] == ' ') || (p[-1] == '\t')))
 > -	    saved_arg2 += 2;
 > -
 > -	  /* We have what looks like a class or namespace
 > -	     scope specification (A::B), possibly with many
 > -	     levels of namespaces or classes (A::B::C::D).
 > -
 > -	     Some versions of the HP ANSI C++ compiler (as also possibly
 > -	     other compilers) generate class/function/member names with
 > -	     embedded double-colons if they are inside namespaces. To
 > -	     handle this, we loop a few times, considering larger and
 > -	     larger prefixes of the string as though they were single
 > -	     symbols.  So, if the initially supplied string is
 > -	     A::B::C::D::foo, we have to look up "A", then "A::B",
 > -	     then "A::B::C", then "A::B::C::D", and finally
 > -	     "A::B::C::D::foo" as single, monolithic symbols, because
 > -	     A, B, C or D may be namespaces.
 > -
 > -	     Note that namespaces can nest only inside other
 > -	     namespaces, and not inside classes.  So we need only
 > -	     consider *prefixes* of the string; there is no need to look up
 > -	     "B::C" separately as a symbol in the previous example. */
 > -
 > -	  p2 = p;		/* save for restart */
 > -	  while (1)
 > -	    {
 > -	      /* Extract the class name.  */
 > -	      p1 = p;
 > -	      while (p != *argptr && p[-1] == ' ')
 > -		--p;
 > -	      copy = (char *) alloca (p - *argptr + 1);
 > -	      memcpy (copy, *argptr, p - *argptr);
 > -	      copy[p - *argptr] = 0;
 > -
 > -	      /* Discard the class name from the arg.  */
 > -	      p = p1 + (p1[0] == ':' ? 2 : 1);
 > -	      while (*p == ' ' || *p == '\t')
 > -		p++;
 > -	      *argptr = p;
 > -
 > -	      sym_class = lookup_symbol (copy, 0, STRUCT_NAMESPACE, 0,
 > -					 (struct symtab **) NULL);
 > -
 > -	      if (sym_class &&
 > -		  (t = check_typedef (SYMBOL_TYPE (sym_class)),
 > -		   (TYPE_CODE (t) == TYPE_CODE_STRUCT
 > -		    || TYPE_CODE (t) == TYPE_CODE_UNION)))
 > -		{
 > -		  /* Arg token is not digits => try it as a function name
 > -		     Find the next token(everything up to end or next blank). */
 > -		  if (**argptr
 > -		      && strchr (get_gdb_completer_quote_characters (),
 > -				 **argptr) != NULL)
 > -		    {
 > -		      p = skip_quoted (*argptr);
 > -		      *argptr = *argptr + 1;
 > -		    }
 > -		  else
 > -		    {
 > -		      p = *argptr;
 > -		      while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':')
 > -			p++;
 > -		    }
 > -/*
 > -   q = operator_chars (*argptr, &q1);
 > -   if (q1 - q)
 > -   {
 > -   char *opname;
 > -   char *tmp = alloca (q1 - q + 1);
 > -   memcpy (tmp, q, q1 - q);
 > -   tmp[q1 - q] = '\0';
 > -   opname = cplus_mangle_opname (tmp, DMGL_ANSI);
 > -   if (opname == NULL)
 > -   {
 > -   cplusplus_error (saved_arg, "no mangling for \"%s\"\n", tmp);
 > -   }
 > -   copy = (char*) alloca (3 + strlen(opname));
 > -   sprintf (copy, "__%s", opname);
 > -   p = q1;
 > -   }
 > -   else
 > - */
 > -		  {
 > -		    copy = (char *) alloca (p - *argptr + 1);
 > -		    memcpy (copy, *argptr, p - *argptr);
 > -		    copy[p - *argptr] = '\0';
 > -		    if (p != *argptr
 > -			&& copy[p - *argptr - 1]
 > -			&& strchr (get_gdb_completer_quote_characters (),
 > -				   copy[p - *argptr - 1]) != NULL)
 > -		      copy[p - *argptr - 1] = '\0';
 > -		  }
 > -
 > -		  /* no line number may be specified */
 > -		  while (*p == ' ' || *p == '\t')
 > -		    p++;
 > -		  *argptr = p;
 > -
 > -		  sym = 0;
 > -		  i1 = 0;	/*  counter for the symbol array */
 > -		  sym_arr = (struct symbol **) alloca (total_number_of_methods (t)
 > -						* sizeof (struct symbol *));
 > -
 > -		  if (destructor_name_p (copy, t))
 > -		    {
 > -		      /* Destructors are a special case.  */
 > -		      int m_index, f_index;
 > -
 > -		      if (get_destructor_fn_field (t, &m_index, &f_index))
 > -			{
 > -			  struct fn_field *f = TYPE_FN_FIELDLIST1 (t, m_index);
 > -
 > -			  sym_arr[i1] =
 > -			    lookup_symbol (TYPE_FN_FIELD_PHYSNAME (f, f_index),
 > -					   NULL, VAR_NAMESPACE, (int *) NULL,
 > -					   (struct symtab **) NULL);
 > -			  if (sym_arr[i1])
 > -			    i1++;
 > -			}
 > -		    }
 > -		  else
 > -		    i1 = find_methods (t, copy, sym_arr);
 > -		  if (i1 == 1)
 > -		    {
 > -		      /* There is exactly one field with that name.  */
 > -		      sym = sym_arr[0];
 > -
 > -		      if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
 > -			{
 > -			  values.sals = (struct symtab_and_line *)
 > -			    xmalloc (sizeof (struct symtab_and_line));
 > -			  values.nelts = 1;
 > -			  values.sals[0] = find_function_start_sal (sym,
 > -							      funfirstline);
 > -			}
 > -		      else
 > -			{
 > -			  values.nelts = 0;
 > -			}
 > -		      return values;
 > -		    }
 > -		  if (i1 > 0)
 > -		    {
 > -		      /* There is more than one field with that name
 > -		         (overloaded).  Ask the user which one to use.  */
 > -		      return decode_line_2 (sym_arr, i1, funfirstline, canonical);
 > -		    }
 > -		  else
 > -		    {
 > -		      char *tmp;
 > -
 > -		      if (is_operator_name (copy))
 > -			{
 > -			  tmp = (char *) alloca (strlen (copy + 3) + 9);
 > -			  strcpy (tmp, "operator ");
 > -			  strcat (tmp, copy + 3);
 > -			}
 > -		      else
 > -			tmp = copy;
 > -		      if (tmp[0] == '~')
 > -			cplusplus_error (saved_arg,
 > -					 "the class `%s' does not have destructor defined\n",
 > -					 SYMBOL_SOURCE_NAME (sym_class));
 > -		      else
 > -			cplusplus_error (saved_arg,
 > -					 "the class %s does not have any method named %s\n",
 > -					 SYMBOL_SOURCE_NAME (sym_class), tmp);
 > -		    }
 > -		}
 > -
 > -	      /* Move pointer up to next possible class/namespace token */
 > -	      p = p2 + 1;	/* restart with old value +1 */
 > -	      /* Move pointer ahead to next double-colon */
 > -	      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\''))
 > -		{
 > -		  if (p[0] == '<')
 > -		    {
 > -		      temp_end = find_template_name_end (p);
 > -		      if (!temp_end)
 > -			error ("malformed template specification in command");
 > -		      p = temp_end;
 > -		    }
 > -		  else if ((p[0] == ':') && (p[1] == ':'))
 > -		    break;	/* found double-colon */
 > -		  else
 > -		    p++;
 > -		}
 > -
 > -	      if (*p != ':')
 > -		break;		/* out of the while (1) */
 > -
 > -	      p2 = p;		/* save restart for next time around */
 > -	      *argptr = saved_arg2;	/* restore argptr */
 > -	    }			/* while (1) */
 > -
 > -	  /* Last chance attempt -- check entire name as a symbol */
 > -	  /* Use "copy" in preparation for jumping out of this block,
 > -	     to be consistent with usage following the jump target */
 > -	  copy = (char *) alloca (p - saved_arg2 + 1);
 > -	  memcpy (copy, saved_arg2, p - saved_arg2);
 > -	  /* Note: if is_quoted should be true, we snuff out quote here anyway */
 > -	  copy[p - saved_arg2] = '\000';
 > -	  /* Set argptr to skip over the name */
 > -	  *argptr = (*p == '\'') ? p + 1 : p;
 > -	  /* Look up entire name */
 > -	  sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
 > -	  s = (struct symtab *) 0;
 > -	  if (sym)
 > -	    return symbol_found (funfirstline, canonical, copy, sym,
 > -				 NULL, sym_symtab);
 > -
 > -	  /* Couldn't find any interpretation as classes/namespaces, so give up */
 > -	  /* The quotes are important if copy is empty.  */
 > -	  cplusplus_error (saved_arg,
 > -			   "Can't find member of namespace, class, struct, or union named \"%s\"\n",
 > -			   copy);
 > -	}
 > -      /*  end of C++  */
 > -
 > +	/*  C++ */
 > +	/*  ... or Java */
 > +	return decode_compound (argptr, funfirstline, canonical,
 > +				saved_arg, p);
 >  
 >        /* Extract the file name.  */
 >        p1 = p;
 > @@ -1281,6 +1057,252 @@ locate_first_half (char **argptr, int *i
 >  }
 >  
 >  \f
 > +
 > +/* This handles C++ and Java compound data structures.  P should point
 > +   at the first component separator, i.e. double-colon or period.  */
 > +
 > +static struct symtabs_and_lines
 > +decode_compound (char **argptr, int funfirstline, char ***canonical,
 > +		 char *saved_arg, char *p)
 > +{
 > +  struct symtabs_and_lines values;
 > +  char *p1, *p2;
 > +#if 0
 > +  char *q, *q1;
 > +#endif
 > +  char *saved_arg2 = *argptr;
 > +  char *temp_end;
 > +  struct symbol *sym;
 > +  /* The symtab that SYM was found in.  */
 > +  struct symtab *sym_symtab;
 > +  char *copy;
 > +  struct symbol *sym_class;
 > +  int i1;
 > +  struct symbol **sym_arr;
 > +  struct type *t;
 > +
 > +  /* First check for "global" namespace specification,
 > +     of the form "::foo". If found, skip over the colons
 > +     and jump to normal symbol processing */
 > +  if (p[0] == ':' 
 > +      && ((*argptr == p) || (p[-1] == ' ') || (p[-1] == '\t')))
 > +    saved_arg2 += 2;
 > +
 > +  /* We have what looks like a class or namespace
 > +     scope specification (A::B), possibly with many
 > +     levels of namespaces or classes (A::B::C::D).
 > +
 > +     Some versions of the HP ANSI C++ compiler (as also possibly
 > +     other compilers) generate class/function/member names with
 > +     embedded double-colons if they are inside namespaces. To
 > +     handle this, we loop a few times, considering larger and
 > +     larger prefixes of the string as though they were single
 > +     symbols.  So, if the initially supplied string is
 > +     A::B::C::D::foo, we have to look up "A", then "A::B",
 > +     then "A::B::C", then "A::B::C::D", and finally
 > +     "A::B::C::D::foo" as single, monolithic symbols, because
 > +     A, B, C or D may be namespaces.
 > +
 > +     Note that namespaces can nest only inside other
 > +     namespaces, and not inside classes.  So we need only
 > +     consider *prefixes* of the string; there is no need to look up
 > +     "B::C" separately as a symbol in the previous example. */
 > +
 > +  p2 = p;		/* save for restart */
 > +  while (1)
 > +    {
 > +      /* Extract the class name.  */
 > +      p1 = p;
 > +      while (p != *argptr && p[-1] == ' ')
 > +	--p;
 > +      copy = (char *) alloca (p - *argptr + 1);
 > +      memcpy (copy, *argptr, p - *argptr);
 > +      copy[p - *argptr] = 0;
 > +
 > +      /* Discard the class name from the arg.  */
 > +      p = p1 + (p1[0] == ':' ? 2 : 1);
 > +      while (*p == ' ' || *p == '\t')
 > +	p++;
 > +      *argptr = p;
 > +
 > +      sym_class = lookup_symbol (copy, 0, STRUCT_NAMESPACE, 0,
 > +				 (struct symtab **) NULL);
 > +
 > +      if (sym_class &&
 > +	  (t = check_typedef (SYMBOL_TYPE (sym_class)),
 > +	   (TYPE_CODE (t) == TYPE_CODE_STRUCT
 > +	    || TYPE_CODE (t) == TYPE_CODE_UNION)))
 > +	{
 > +	  /* Arg token is not digits => try it as a function name
 > +	     Find the next token(everything up to end or next blank). */
 > +	  if (**argptr
 > +	      && strchr (get_gdb_completer_quote_characters (),
 > +			 **argptr) != NULL)
 > +	    {
 > +	      p = skip_quoted (*argptr);
 > +	      *argptr = *argptr + 1;
 > +	    }
 > +	  else
 > +	    {
 > +	      p = *argptr;
 > +	      while (*p && *p != ' ' && *p != '\t' && *p != ',' && *p != ':')
 > +		p++;
 > +	    }
 > +/*
 > +   q = operator_chars (*argptr, &q1);
 > +   if (q1 - q)
 > +   {
 > +   char *opname;
 > +   char *tmp = alloca (q1 - q + 1);
 > +   memcpy (tmp, q, q1 - q);
 > +   tmp[q1 - q] = '\0';
 > +   opname = cplus_mangle_opname (tmp, DMGL_ANSI);
 > +   if (opname == NULL)
 > +   {
 > +   cplusplus_error (saved_arg, "no mangling for \"%s\"\n", tmp);
 > +   }
 > +   copy = (char*) alloca (3 + strlen(opname));
 > +   sprintf (copy, "__%s", opname);
 > +   p = q1;
 > +   }
 > +   else
 > + */
 > +	  {
 > +	    copy = (char *) alloca (p - *argptr + 1);
 > +	    memcpy (copy, *argptr, p - *argptr);
 > +	    copy[p - *argptr] = '\0';
 > +	    if (p != *argptr
 > +		&& copy[p - *argptr - 1]
 > +		&& strchr (get_gdb_completer_quote_characters (),
 > +			   copy[p - *argptr - 1]) != NULL)
 > +	      copy[p - *argptr - 1] = '\0';
 > +	  }
 > +
 > +	  /* no line number may be specified */
 > +	  while (*p == ' ' || *p == '\t')
 > +	    p++;
 > +	  *argptr = p;
 > +
 > +	  sym = 0;
 > +	  i1 = 0;	/*  counter for the symbol array */
 > +	  sym_arr = (struct symbol **) alloca (total_number_of_methods (t)
 > +					       * sizeof (struct symbol *));
 > +
 > +	  if (destructor_name_p (copy, t))
 > +	    {
 > +	      /* Destructors are a special case.  */
 > +	      int m_index, f_index;
 > +
 > +	      if (get_destructor_fn_field (t, &m_index, &f_index))
 > +		{
 > +		  struct fn_field *f = TYPE_FN_FIELDLIST1 (t, m_index);
 > +
 > +		  sym_arr[i1] =
 > +		    lookup_symbol (TYPE_FN_FIELD_PHYSNAME (f, f_index),
 > +				   NULL, VAR_NAMESPACE, (int *) NULL,
 > +				   (struct symtab **) NULL);
 > +		  if (sym_arr[i1])
 > +		    i1++;
 > +		}
 > +	    }
 > +	  else
 > +	    i1 = find_methods (t, copy, sym_arr);
 > +	  if (i1 == 1)
 > +	    {
 > +	      /* There is exactly one field with that name.  */
 > +	      sym = sym_arr[0];
 > +
 > +	      if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
 > +		{
 > +		  values.sals = (struct symtab_and_line *)
 > +		    xmalloc (sizeof (struct symtab_and_line));
 > +		  values.nelts = 1;
 > +		  values.sals[0] = find_function_start_sal (sym,
 > +							    funfirstline);
 > +		}
 > +	      else
 > +		{
 > +		  values.nelts = 0;
 > +		}
 > +	      return values;
 > +	    }
 > +	  if (i1 > 0)
 > +	    {
 > +	      /* There is more than one field with that name
 > +		 (overloaded).  Ask the user which one to use.  */
 > +	      return decode_line_2 (sym_arr, i1, funfirstline, canonical);
 > +	    }
 > +	  else
 > +	    {
 > +	      char *tmp;
 > +
 > +	      if (is_operator_name (copy))
 > +		{
 > +		  tmp = (char *) alloca (strlen (copy + 3) + 9);
 > +		  strcpy (tmp, "operator ");
 > +		  strcat (tmp, copy + 3);
 > +		}
 > +	      else
 > +		tmp = copy;
 > +	      if (tmp[0] == '~')
 > +		cplusplus_error (saved_arg,
 > +				 "the class `%s' does not have destructor defined\n",
 > +				 SYMBOL_SOURCE_NAME (sym_class));
 > +	      else
 > +		cplusplus_error (saved_arg,
 > +				 "the class %s does not have any method named %s\n",
 > +				 SYMBOL_SOURCE_NAME (sym_class), tmp);
 > +	    }
 > +	}
 > +
 > +      /* Move pointer up to next possible class/namespace token */
 > +      p = p2 + 1;	/* restart with old value +1 */
 > +      /* Move pointer ahead to next double-colon */
 > +      while (*p && (p[0] != ' ') && (p[0] != '\t') && (p[0] != '\''))
 > +	{
 > +	  if (p[0] == '<')
 > +	    {
 > +	      temp_end = find_template_name_end (p);
 > +	      if (!temp_end)
 > +		error ("malformed template specification in command");
 > +	      p = temp_end;
 > +	    }
 > +	  else if ((p[0] == ':') && (p[1] == ':'))
 > +	    break;	/* found double-colon */
 > +	  else
 > +	    p++;
 > +	}
 > +
 > +      if (*p != ':')
 > +	break;		/* out of the while (1) */
 > +
 > +      p2 = p;		/* save restart for next time around */
 > +      *argptr = saved_arg2;	/* restore argptr */
 > +    }			/* while (1) */
 > +
 > +  /* Last chance attempt -- check entire name as a symbol */
 > +  /* Use "copy" in preparation for jumping out of this block,
 > +     to be consistent with usage following the jump target */
 > +  copy = (char *) alloca (p - saved_arg2 + 1);
 > +  memcpy (copy, saved_arg2, p - saved_arg2);
 > +  /* Note: if is_quoted should be true, we snuff out quote here anyway */
 > +  copy[p - saved_arg2] = '\000';
 > +  /* Set argptr to skip over the name */
 > +  *argptr = (*p == '\'') ? p + 1 : p;
 > +  /* Look up entire name */
 > +  sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
 > +  if (sym)
 > +    return symbol_found (funfirstline, canonical, copy, sym,
 > +			 NULL, sym_symtab);
 > +
 > +  /* Couldn't find any interpretation as classes/namespaces, so give up */
 > +  /* The quotes are important if copy is empty.  */
 > +  cplusplus_error (saved_arg,
 > +		   "Can't find member of namespace, class, struct, or union named \"%s\"\n",
 > +		   copy);
 > +}
 > +
 > +\f
 >  
 >  /* Now come some functions that are called from multiple places within
 >     decode_line_1.  */

Re: [rfa] linespec.c, part 5

[Eli Zaretskii]

> From: David Carlton <carlton@math.stanford.edu>
> Date: 15 Nov 2002 14:25:38 -0800
> 
> * While preparing this patch, I noticed that the variable
>   gdb_completer_quote_characters is declared but not used; I got rid
>   of it.

While making all these changes, you do make sure that completion on
the names of functions, variables, classes, etc. still works, right?
(That completion includes the correct handling of quoted names, btw.)

Re: [rfa] linespec.c, part 5

[David Carlton]

On Sat, 16 Nov 2002 11:45:53 +0300, "Eli Zaretskii" <eliz@is.elta.co.il> said:
>> From: David Carlton <carlton@math.stanford.edu>
>> Date: 15 Nov 2002 14:25:38 -0800

>> * While preparing this patch, I noticed that the variable
>> gdb_completer_quote_characters is declared but not used; I got rid
>> of it.

> While making all these changes, you do make sure that completion on
> the names of functions, variables, classes, etc. still works, right?
> (That completion includes the correct handling of quoted names,
> btw.)

I've been running it through the testsuite, but I have no particular
reason to believe that the testsuite comprehensively tests everything
that decode_line_1 does.  There are some completion tests in
gdb.base/completion.exp, so at least some of the completion has been
tested.

It would probably be a boon to GDB for somebody to ensure that the
testsuite comprehensively tests decode_line_1.

By the way, if you were worried about the removal of
gdb_completer_quote_characters, it was replaced by an accessor
function get_gdb_completer_quote_characters a couple of years ago.

David Carlton
carlton@math.stanford.edu

Re: [rfa] linespec.c, part 5

[David Carlton]

On Fri, 15 Nov 2002 20:08:27 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> * The function that this creates, decode_compound, is quite long.
>> Later patches in this series will break it down into smaller
>> chunks; they will come after I've got decode_line_1 itself down to
>> a nice size.

> OK but, could you add something to the comment at the top of the
> function that describes what the function returns and what the
> params are?

Sure, I can elaborate on that comment.  Though I don't think I want to
describe _all_ of the args there: many functions get passed the args
of decode_line_1, so I'd rather put a single comment describing that
somewhere rather than repeating that over and over again.

Maybe I'll replace the these comments right after the end of
decode_line_1:

/* Now, still more helper functions.  */

/* NOTE: carlton/2002-11-07: Some of these have non-obvious side
   effects.  In particular, if a function is passed ARGPTR as an
   argument, it modifies what ARGPTR points to.  (Typically, it
   advances *ARGPTR past whatever substring it has just looked
   at.)  */

with a comment saying:

/* Now, more helper functions for decode_line_1.  Some conventions
   that these functions follow:

   Decode_line_1 typically passes along some of its arguments or local
   variables to the subfunctions.  It passes the variables by
   reference if they are modified by the subfunction, and by value
   otherwise.

   Some of the functions have side effects that don't arise from
   variables that are passed by reference.  In particular, if a
   function is passed ARGPTR as an argument, it modifies what ARGPTR
   points to; typically, it advances *ARGPTR past whatever substring
   it has just looked at.  (If it doesn't modify *ARGPTR, then the
   function gets passed *ARGPTR instead, which is then called ARG: see
   set_flags, for example.)  Also, functions that return a struct
   symtabs_and_lines may modify CANONICAL, as in the description of
   decode_line_1.

   If a function returns a struct symtabs_and_lines, then that struct
   will immediately make its way up the call chain to be returned by
   decode_line_1.  In particular, all of the functions decode_XXX
   calculate the appropriate struct symtabs_and_lines, under the
   assumption that their argument is of the form XXX.  */

Is that clearer?

>> * Since decode_line_1 returns the value of decode_compound, there's no
>> need to check whether or not decode_compound modifies the arguments
>> that it's been passed: later code in decode_line_1 can never be
>> effected by such modifications.

> Hmmm. maybe I am not understanding what you say. Does
> decode_compound modify its args? seems like it changes argptr and
> canonical. What if somebody changes decode_line_1 in the future so
> that there is more code executed after the call, it could probably
> cause some head scratching to see that the call didn't leave things
> as expected. I haven't looked too closely though.

Does the above comment help?  What I meant was: every code path in the
code that I extracted leads to either a return from decode_line_1 or
an error.  So, for example, subsequent code in decode_line_1 won't
depend on, say, modifications to 'p' within decode_compound.

Decode_compound prepares ARGPTR and CANONICAL appropriately for
return, and calculates the struct symtabs_and_lines to be returned.
If somebody subsequently decides that, say, a bit more fiddling should
happen within decode_line_1, I think these changes should make that
fiddling a lot easier rather than a lot harder.

It will also help once I've renamed some variables, so 's' turns into
'file_symtab', 'p' turns into 'next_component', and so forth.  That
will make it much easier for future readers of the code to understand
the possible ramifications of changing the values of those variables.
(Whereas who knows what it means to change the value of a variable 'p'
that is referred to in a zillion places, with some of those uses
distinct and some of them not.)

David Carlton
carlton@math.stanford.edu

Re: [rfa] linespec.c, part 5

[Elena Zannoni]

David Carlton writes:
 > On Fri, 15 Nov 2002 20:08:27 -0500, Elena Zannoni <ezannoni@redhat.com> said:
 > > David Carlton writes:
 > 
 > >> * The function that this creates, decode_compound, is quite long.
 > >> Later patches in this series will break it down into smaller
 > >> chunks; they will come after I've got decode_line_1 itself down to
 > >> a nice size.
 > 
 > > OK but, could you add something to the comment at the top of the
 > > function that describes what the function returns and what the
 > > params are?
 > 
 > Sure, I can elaborate on that comment.  Though I don't think I want to
 > describe _all_ of the args there: many functions get passed the args
 > of decode_line_1, so I'd rather put a single comment describing that
 > somewhere rather than repeating that over and over again.
 > 
 > Maybe I'll replace the these comments right after the end of
 > decode_line_1:
 > 
 > /* Now, still more helper functions.  */
 > 
 > /* NOTE: carlton/2002-11-07: Some of these have non-obvious side
 >    effects.  In particular, if a function is passed ARGPTR as an
 >    argument, it modifies what ARGPTR points to.  (Typically, it
 >    advances *ARGPTR past whatever substring it has just looked
 >    at.)  */
 > 
 > with a comment saying:
 > 
 > /* Now, more helper functions for decode_line_1.  Some conventions
 >    that these functions follow:
 > 
 >    Decode_line_1 typically passes along some of its arguments or local
 >    variables to the subfunctions.  It passes the variables by
 >    reference if they are modified by the subfunction, and by value
 >    otherwise.
 > 
 >    Some of the functions have side effects that don't arise from
 >    variables that are passed by reference.  In particular, if a
 >    function is passed ARGPTR as an argument, it modifies what ARGPTR
 >    points to; typically, it advances *ARGPTR past whatever substring
 >    it has just looked at.  (If it doesn't modify *ARGPTR, then the
 >    function gets passed *ARGPTR instead, which is then called ARG: see
 >    set_flags, for example.)  Also, functions that return a struct
 >    symtabs_and_lines may modify CANONICAL, as in the description of
 >    decode_line_1.
 > 
 >    If a function returns a struct symtabs_and_lines, then that struct
 >    will immediately make its way up the call chain to be returned by
 >    decode_line_1.  In particular, all of the functions decode_XXX
 >    calculate the appropriate struct symtabs_and_lines, under the
 >    assumption that their argument is of the form XXX.  */
 > 
 > Is that clearer?
 > 

better, yes.

 > >> * Since decode_line_1 returns the value of decode_compound, there's no
 > >> need to check whether or not decode_compound modifies the arguments
 > >> that it's been passed: later code in decode_line_1 can never be
 > >> effected by such modifications.
 > 
 > > Hmmm. maybe I am not understanding what you say. Does
 > > decode_compound modify its args? seems like it changes argptr and
 > > canonical. What if somebody changes decode_line_1 in the future so
 > > that there is more code executed after the call, it could probably
 > > cause some head scratching to see that the call didn't leave things
 > > as expected. I haven't looked too closely though.
 > 
 > Does the above comment help?  What I meant was: every code path in the
 > code that I extracted leads to either a return from decode_line_1 or
 > an error.  So, for example, subsequent code in decode_line_1 won't
 > depend on, say, modifications to 'p' within decode_compound.
 > 
 > Decode_compound prepares ARGPTR and CANONICAL appropriately for
 > return, and calculates the struct symtabs_and_lines to be returned.
 > If somebody subsequently decides that, say, a bit more fiddling should
 > happen within decode_line_1, I think these changes should make that
 > fiddling a lot easier rather than a lot harder.
 > 

I guess we'll heve to wait for the rest of your patches. If that's
still an issue we can revisit later.

 > It will also help once I've renamed some variables, so 's' turns into
 > 'file_symtab', 'p' turns into 'next_component', and so forth.  That
 > will make it much easier for future readers of the code to understand
 > the possible ramifications of changing the values of those variables.
 > (Whereas who knows what it means to change the value of a variable 'p'
 > that is referred to in a zillion places, with some of those uses
 > distinct and some of them not.)
 > 

BTW, I did a diff -w of decode_compound and the code you removed, and
I noticed this (ignore the line numbers):

@@ -211,7 +231,6 @@
          *argptr = (*p == '\'') ? p + 1 : p;
          /* Look up entire name */
          sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
-         s = (struct symtab *) 0;
          if (sym)
            return symbol_found (funfirstline, canonical, copy, sym,
                                 NULL, sym_symtab);


I agree with you that such assignment didn't make much sense. Dig dig dig,
it came in with the HP merge.
 It was doing this:

 /* Look up entire name */
 sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
 s = (struct symtab *) 0;
 /* Prepare to jump: restore the " if (condition)" so outer layers see it */
 if (has_if)
  *ii = ' ';
 /* Symbol was found --> jump to normal symbol processing.
    Code following "symbol_found" expects "copy" to have the
    symbol name, "sym" to have the symbol pointer, "s" to be
    a specified file's symtab, and sym_symtab to be the symbol's
    symtab. */
  /* By jumping there we avoid falling through the FILE:LINE and
     FILE:FUNC processing stuff below */
  if (sym)
   goto symbol_found;

and
symbol_found: was (is) checking for s==0.

But now that would be the NULL parameter in the symbol_found call
right after.

So I think that's safe.

OK check it in.

Elena

Re: [rfa] linespec.c, part 5

[David Carlton]

On Thu, 5 Dec 2002 12:12:15 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:

>> Maybe I'll replace the these comments right after the end of
>> decode_line_1:
>> 
>> /* Now, still more helper functions.  */
>> 
>> /* NOTE: carlton/2002-11-07: Some of these have non-obvious side
>> effects.  In particular, if a function is passed ARGPTR as an
>> argument, it modifies what ARGPTR points to.  (Typically, it
>> advances *ARGPTR past whatever substring it has just looked
>> at.)  */
>> 
>> with a comment saying:
>> 
>> /* Now, more helper functions for decode_line_1.  Some conventions
>> that these functions follow:
>> 
>> Decode_line_1 typically passes along some of its arguments or local
>> variables to the subfunctions.  It passes the variables by
>> reference if they are modified by the subfunction, and by value
>> otherwise.
>> 
>> Some of the functions have side effects that don't arise from
>> variables that are passed by reference.  In particular, if a
>> function is passed ARGPTR as an argument, it modifies what ARGPTR
>> points to; typically, it advances *ARGPTR past whatever substring
>> it has just looked at.  (If it doesn't modify *ARGPTR, then the
>> function gets passed *ARGPTR instead, which is then called ARG: see
>> set_flags, for example.)  Also, functions that return a struct
>> symtabs_and_lines may modify CANONICAL, as in the description of
>> decode_line_1.
>> 
>> If a function returns a struct symtabs_and_lines, then that struct
>> will immediately make its way up the call chain to be returned by
>> decode_line_1.  In particular, all of the functions decode_XXX
>> calculate the appropriate struct symtabs_and_lines, under the
>> assumption that their argument is of the form XXX.  */
>> 
>> Is that clearer?

> better, yes.

> BTW, I did a diff -w of decode_compound and the code you removed, and
> I noticed this (ignore the line numbers):

> @@ -211,7 +231,6 @@
>           *argptr = (*p == '\'') ? p + 1 : p;
>           /* Look up entire name */
>           sym = lookup_symbol (copy, 0, VAR_NAMESPACE, 0, &sym_symtab);
> -         s = (struct symtab *) 0;
>           if (sym)
>             return symbol_found (funfirstline, canonical, copy, sym,
>                                  NULL, sym_symtab);

Oh, sorry, I should have explained that.  As you noticed, the code
once looked something like

  s = (struct symtab *) 0;
  if (sym)
    goto symbol_found;

(there was probably other stuff there).

Then my first patch replaced all of the "goto symbol_found"'s by
"return symbol_found (a bunch of arguments, including s)".  So that
would have turned the code into

  s = (struct symtab *) 0;
  if (sym)
    return symbol_found (funfirstline, canonical, copy, sym,
                         s, sym_symtab);

except that, since we know that s is 0 there, I replaced the s by
NULL, giving us

  s = (struct symtab *) 0;
  if (sym)
    return symbol_found (funfirstline, canonical, copy, sym,
                         NULL, sym_symtab);

But at the time of my first patch, I couldn't say for sure if another
flow of control might be affected by the "s = (struct symtab *) 0;"
line, so I left it in there.

This patch, however, allows us to answer that question: the only place
in the new decode_compound function where 's' is referred to is in
that line that is deleted, and since decode_line_1 immediately returns
the value of decode_compound, setting 's' doesn't have any effect.  So
I deleted it.

> OK check it in.

Thanks, will do (with the expanded comment mentioned above).

David Carlton
carlton@math.stanford.edu

[rfa] linespec.c, part 5

[David Carlton]

The next part: decode_compound, to handle C++ and Java compound data
structures.  Some notes: