RFA: Correct field names for class methods

[Daniel Jacobowitz]

On Thu, Aug 29, 2002 at 10:48:05PM -0500, Jim Blandy wrote:
> 
> I understand the method code very little, so I have only superficial
> comments.  I'll try to do a more thorough reading next week, if Elena
> doesn't beat me to it.

Thanks.

> 
> Daniel Jacobowitz <drow@mvista.com> writes:
> > @@ -129,15 +128,11 @@ cp_print_class_method (char *valaddr,
> >  	  f = TYPE_FN_FIELDLIST1 (domain, i);
> >  	  len2 = TYPE_FN_FIELDLIST_LENGTH (domain, i);
> >  
> > +	  check_stub_method_group (f, j);
> >  	  for (j = 0; j < len2; j++)
> >  	    {
> > -	      QUIT;
> > -	      if (TYPE_FN_FIELD_STUB (f, j))
> > -		check_stub_method (domain, i, j);
> >  	      if (STREQ (SYMBOL_NAME (sym), TYPE_FN_FIELD_PHYSNAME (f, j)))
> > -		{
> > -		  goto common;
> > -		}
> > +		goto common;
> >  	    }
> >  	}
> >      }
> 
> Not that it matters much, but why are you removing the QUIT from this
> loop?

Exactly... because I moved the check_stub_method outside of the loop,
there is no longer any possible justification to QUIT; there.  It's a
loop of strcmp calls!  If you want, I could add the QUIT in
check_stub_method_group, but I don't think it's necessary.

> > +/* Find the last component of the demangled C++ name NAME.
> > +
> > +   This function return a pointer to the first colon before the
> > +   last component, or NULL if the name had only one component.  */
> 
> Hmm, not every demangled name contains a closing paren.  Shouldn't this
> comment clarify that it only works on demangled method names?

Yes, you're right.  It used to not require the closing paren, and then
I realized unpleasant things about the ways some local inner classes
are mangled.

> > @@ -3377,6 +3378,127 @@ read_member_functions (struct field_info
> >  	}
> >        else
> >  	{
> > +	  int has_stub = 0;
> > +	  int has_nondestructor = 0, has_destructor = 0;
> > +	  int is_v3 = 0;
> > +	  struct next_fnfield *tmp_sublist;
> > +
> > +	  /* Various versions of GCC emit various mostly-useless
> > +	     strings in the name field for special member functions.
> > +	     For some methods, we have a complete physname, so we
> > +	     could fix this up now.  For stub methods we will do this
> > +	     later, in check_stub_method_group.  For non-stub methods,
> > +	     we have no clear way to know whether or not the physname
> > +	     is correct; g++ 2.95.x only reliably emits full physnames
> > +	     for operators which do not start with the method name
> > +	     (constructors, destructors fall in this category; there
> > +	     may be others?).  But for other methods it may or may not
> > +	     emit a full physname depending on the platform (if
> > +	     CPLUS_MARKER can be `$' or `.', it will use minimal debug
> > +	     information, but not otherwise).
> > +
> > +	     Rather than dealing with this, we take a different approach.
> > +	     For v3 mangled names, we can use the full physname; for v2,
> > +	     we use cplus_demangle_opname, because the only interesting
> > +	     names are all operators.  Skip if any method in the group
> > +	     is a stub, to prevent our fouling up the workings of
> > +	     gdb_mangle_name.
> > +
> > +	     Another thing that we need to clean up here: GCC 2.95.x
> > +	     puts constructors and destructors in the same group.  We
> > +	     need to split this into two groups.  */
> > +
> > +	  tmp_sublist = sublist;
> > +	  while (tmp_sublist != NULL)
> > +	    {
> > +	      if (tmp_sublist->fn_field.is_stub)
> > +		has_stub = 1;
> > +	      if (tmp_sublist->fn_field.physname[0] == '_'
> > +		  && tmp_sublist->fn_field.physname[1] == 'Z')
> > +		is_v3 = 1;
> > +
> > +	      if (is_destructor_name (tmp_sublist->fn_field.physname))
> > +		has_destructor++;
> > +	      else
> > +		has_nondestructor++;
> > +
> > +	      tmp_sublist = tmp_sublist->next;
> > +	    }
> > +
> > +	  if (has_destructor && has_nondestructor)
> > +	    {
> > +	      struct next_fnfieldlist *destr_fnlist;
> > +	      struct next_fnfield *last_sublist;
> > +
> > +	      /* Create a new fn_fieldlist for the destructors.  */;
> > +	      destr_fnlist = (struct next_fnfieldlist *)
> > +		xmalloc (sizeof (struct next_fnfieldlist));
> > +	      make_cleanup (xfree, destr_fnlist);
> > +	      memset (destr_fnlist, 0, sizeof (struct next_fnfieldlist));
> > +	      destr_fnlist->fn_fieldlist.name
> > +		= obconcat (&objfile->type_obstack, "", "~",
> > +			    new_fnlist->fn_fieldlist.name);
> > +
> > +	      destr_fnlist->fn_fieldlist.fn_fields = (struct fn_field *)
> > +		obstack_alloc (&objfile->type_obstack,
> > +			       sizeof (struct fn_field) * has_destructor);
> > +	      memset (destr_fnlist->fn_fieldlist.fn_fields, 0,
> > +		  sizeof (struct fn_field) * has_destructor);
> > +	      tmp_sublist = sublist;
> > +	      last_sublist = NULL;
> > +	      i = 0;
> > +	      while (tmp_sublist != NULL)
> > +		{
> > +		  if (!is_destructor_name (tmp_sublist->fn_field.physname))
> > +		    {
> > +		      tmp_sublist = tmp_sublist->next;
> > +		      continue;
> > +		    }
> > +		  
> > +		  destr_fnlist->fn_fieldlist.fn_fields[i]
> > +		    = tmp_sublist->fn_field;
> > +		  if (last_sublist)
> > +		    last_sublist->next = tmp_sublist->next;
> > +		  else
> > +		    sublist = tmp_sublist->next;
> > +		  last_sublist = tmp_sublist;
> > +		  tmp_sublist = tmp_sublist->next;
> > +		}
> > +
> > +	      destr_fnlist->fn_fieldlist.length = has_destructor;
> > +	      destr_fnlist->next = fip->fnlist;
> > +	      fip->fnlist = destr_fnlist;
> > +	      nfn_fields++;
> > +	      total_length += has_destructor;
> > +	      length -= has_destructor;
> > +	    }
> > +	  else if (is_v3)
> > +	    {
> > +	      /* v3 mangling prevents the use of abbreviated physnames,
> > +		 so we can do this here.  There are stubbed methods in v3
> > +		 only:
> > +		 - in -gstabs instead of -gstabs+
> > +		 - or for static methods, who are output as a function type
> > +		   instead of a method type.  */
> > +
> > +	      update_method_name_from_physname (&new_fnlist->fn_fieldlist.name,
> > +						sublist->fn_field.physname);
> > +	    }
> > +	  else if (!has_stub)
> > +	    {
> > +	      char dem_opname[256];
> > +	      int ret;
> > +	      ret = cplus_demangle_opname (new_fnlist->fn_fieldlist.name,
> > +					      dem_opname, DMGL_ANSI);
> > +	      if (!ret)
> > +		ret = cplus_demangle_opname (new_fnlist->fn_fieldlist.name,
> > +					     dem_opname, 0);
> > +	      if (ret)
> > +		new_fnlist->fn_fieldlist.name
> > +		  = obsavestring (dem_opname, strlen (dem_opname),
> > +				  &objfile->type_obstack);
> > +	    }
> > +
> 
> Is there any way we could pull this out into a separate function or
> functions?  read_member_functions is bad enough already.

Hmm... I'm not sure.  In particular, when splitting constructors from
destructors it tramples on the function state a bit: fip, nfn_fields,
total_length, length, and new_fnlist.  My usual criterion for breaking
out a function is whether its arguments would make any sense on their
own (outside of the calling function's control flow), and these wouldn't.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer