Re: [RFA] Typedef'd method parameters [3/4]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Thu, 21 Apr 2011 23:15:37 +0200, Keith Seitz wrote:
[...]
> --- cp-support.c	26 Feb 2011 02:07:07 -0000	1.49
> +++ cp-support.c	21 Apr 2011 17:03:35 -0000
> @@ -117,6 +117,115 @@ cp_already_canonical (const char *string
>      return 0;
>  }
>  
> +/* Insepct the given RET_COMP for its type.  If it is a typedef,
> +   replace the name stored in the tree with the typedef's fully resolved
> +   type.  If no changes have been made to the tree, set CHANGED to 0;
> +   otherwise set it to 1.  FREE_LIST contains any temprorary storage

temporary

> +   that was used to revise the tree.  It should be freed by the caller. */

CHANGED/FREE_LIST I do not understand what they refer to at all.


> +
> +static void
> +inspect_type (struct demangle_component *ret_comp)
> +{
> +  char *name;
> +  struct symbol *sym;
> +
> +  name = (char *) alloca (ret_comp->u.s_name.len + 1);
> +  strncpy (name, ret_comp->u.s_name.s, ret_comp->u.s_name.len);
> +  name[ret_comp->u.s_name.len] = '\0';
> +  sym = lookup_symbol (name, 0, VAR_DOMAIN, 0);
> +  if (sym != NULL)
> +    {
> +      struct type *type = SYMBOL_TYPE (sym);
> +
> +      if (TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
> +	{
> +	  char *canon;
> +	  struct ui_file *buf = mem_fileopen ();
> +
> +	  CHECK_TYPEDEF (type);

BUF should be make_cleanup protected against exceptions.


> +	  type_print (type, "", buf, -1);
> +	  name = ui_file_xstrdup (buf, NULL);
> +	  ui_file_delete (buf);
> +	  canon = cp_canonicalize_string_no_typedefs (name);
> +	  if (canon != NULL)
> +	    {
> +	      xfree (name);
> +	      name = canon;
> +	    }
> +	  ret_comp->u.s_name.s = name;
> +	  ret_comp->u.s_name.len = strlen (name);
> +	}
> +    }
> +}
> +
> +/* Walk the parse tree given by RET_COMP, replacing any typedefs with
> +   their basic types.  CHANGED is set to indicate whether any changes
> +   to the tree were made, and FREE_LIST will contain any memory used
> +   for this purpose.  It must be freed by the caller.  */

CHANGED/FREE_LIST I do not understand what they refer to at all.


> +
> +static void
> +replace_typedefs (struct demangle_component *ret_comp)
> +{
> +  if (ret_comp)
> +    {
> +      switch (ret_comp->type)
> +	{
> +	case DEMANGLE_COMPONENT_ARGLIST:
> +	  replace_typedefs (d_left (ret_comp));
> +	  replace_typedefs (d_right (ret_comp));
> +	  break;
> +
> +	case DEMANGLE_COMPONENT_NAME:
> +	  inspect_type (ret_comp);
> +	  break;
> +
> +	case DEMANGLE_COMPONENT_FUNCTION_TYPE:
> +	case DEMANGLE_COMPONENT_LOCAL_NAME:
> +	case DEMANGLE_COMPONENT_TYPED_NAME:
> +	  replace_typedefs (d_right (ret_comp));
> +	  break;
> +
> +	case DEMANGLE_COMPONENT_QUAL_NAME:
> +	case DEMANGLE_COMPONENT_POINTER:
> +	  replace_typedefs (d_left (ret_comp));
> +	  break;
> +
> +	default:
> +	  break;
> +	}
> +    }
> +}

This table is incomplete, for example DEMANGLE_COMPONENT_ARRAY_TYPE is
missing.
	C++ source / GDB input:
	f(CORE_ADDR (*)[10])
	GDB output:
	f(CORE_ADDR (*) [10])
	GCC output:
	f(unsigned long (*) [10])
	_Z1fPA10_m


Also there should be an equivalent of the CLOSED-FIXED PR:
	Bug 12328 - Can't set breakpoint on method taking const, non-reference/pointer scalar parameter

which result in such a kind of regression-like behavior with this patchset.
It may be related to the FSF GDB HEAD bug which I have filed now:
	[Bug symtab/12708] New: CU expansion problem for parameters

in this case
	void f (const int x) {}
gcc -c -g
with this patchset:
gdb -readnow -q -nx ./2.o -ex "complete b 'f(" -ex q
b 'f(int const)
b 'f(int)
gdb -readnow -q -nx ./2.o -ex "b 'f(int const)'" -ex q
Function "f(int const)" not defined.

I believe the `f(int const)' alias should be shown but replace_typedefs should
also remove that `const' during the `break' command.


> +
> +/* Parse STRING and convert it to canonical form, resolving any typedefs. 
> +   If parsing fails, or if STRING is already canonical, return NULL. 
> +   Otherwise return the canonical form.  The return value is allocated via
> +   xmalloc.  */
> +
> +char *
> +cp_canonicalize_string_no_typedefs (const char *string)
> +{
> +  char *ret;
> +  unsigned int estimated_len;
> +  struct demangle_parse_info *info;
> +
> +  ret = NULL;
> +  estimated_len = strlen (string) * 2;
> +  info = cp_demangled_name_to_comp (string, NULL);
> +  if (info != NULL)
> +    {
> +      replace_typedefs (info->tree);
> +      ret = cp_comp_to_string (info->tree, estimated_len);
> +      cp_demangled_name_parse_free (info);
> +      if (strcmp (string, ret) == 0)

cp_comp_to_string can return NULL, OK, it may not obviously happen but still
some gdb_assert / internal_error would be appropriate IMO.


> +	{
> +	  xfree (ret);
> +	  return NULL;
> +	}
> +    }
> +
> +  return ret;
> +}
> +
>  /* Parse STRING and convert it to canonical form.  If parsing fails,
>     or if STRING is already canonical, return NULL.  Otherwise return
>     the canonical form.  The return value is allocated via xmalloc.  */
> Index: cp-support.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/cp-support.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 cp-support.h
> --- cp-support.h	4 Apr 2011 14:37:16 -0000	1.42
> +++ cp-support.h	21 Apr 2011 17:03:35 -0000
> @@ -36,6 +36,16 @@ struct objfile;
>  struct type;
>  struct demangle_component;
>  
> +/* The result of parsing a name.  */
> +struct demangle_parse_info
> +{
> +  /* The memory used during the parse.  */
> +  struct demangle_info *info;
> +
> +  /* The result of the parse.  */
> +  struct demangle_component *tree;
> +};

BTW already present in [patch 1/4].


> +
>  /* This struct is designed to store data from using directives.  It
>     says that names from namespace IMPORT_SRC should be visible within
>     namespace IMPORT_DEST.  These form a linked list; NEXT is the next
[...]


Thanks,
Jan