RE: [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: TYPE_SPECIFIC_SELF_TYPE

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

  Hi all,


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Doug Evans
> Envoyé : lundi 26 janvier 2015 01:07
> À : gdb-patches@sourceware.org; gaius@glam.ac.uk
> Objet : [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}:
> TYPE_SPECIFIC_SELF_TYPE
> 
> Hi.
> 
> This patch moves TYPE_SELF_TYPE into new field type_specific.self_type
> for MEMBERPTR,METHODPTR types, and into type_specific.func_stuff
> for METHODs, and then updates everything to use that.
> TYPE_CODE_METHOD could share some things with TYPE_CODE_FUNC
> (e.g. TYPE_NO_RETURN) and it seemed simplest to keep them together.
> 
> Moving TYPE_SELF_TYPE into type_specific.func_stuff for
> TYPE_CODE_METHOD
> is also nice because when we allocate space for function types we
> assume
> they're TYPE_CODE_FUNCs. If TYPE_CODE_METHODs don't need or use that
> space then that space would be wasted, and cleaning that up would
> involve
> more invasive changes.
> 
> In order to catch errant uses I've added accessor functions
> that do some checking.
> 
> One can no longer assign to TYPE_SELF_TYPE like this:
> 
>   TYPE_SELF_TYPE (foo) = bar;
> 
> One instead has to do:
> 
>   set_type_self_type (foo, bar);
> 
> But I've left reading of the type to the macro:
> 
>   bar = TYPE_SELF_TYPE (foo);
> 
> I could add SET_TYPE_SELF_TYPE as a wrapper on set_type_self_type
> if you prefer that.
> 
> In order to discourage bypassing the TYPE_SELF_TYPE macro
> I've named the underlying function that implements it
....
> 	* stabsread.c (read_member_functions): Mark methods with
> 	TYPE_CODE_METHOD, not TYPE_CODE_FUNC.  Update setting of
> 	TYPE_SELF_TYPE.
.....
> diff --git a/gdb/stabsread.c b/gdb/stabsread.c
> index 1f46f75..423c442 100644
> --- a/gdb/stabsread.c
> +++ b/gdb/stabsread.c
> @@ -2376,14 +2376,21 @@ read_member_functions (struct field_info *fip,
> char **pp, struct type *type,
>  	      p++;
>  	    }
> 
> -	  /* If this is just a stub, then we don't have the real name
> here.  */
> +	  /* These are methods, not functions.  */
> +	  if (TYPE_CODE (new_sublist->fn_field.type) == TYPE_CODE_FUNC)
> +	    TYPE_CODE (new_sublist->fn_field.type) = TYPE_CODE_METHOD;
> +	  else
> +	    gdb_assert (TYPE_CODE (new_sublist->fn_field.type)
> +			== TYPE_CODE_METHOD);
> 
> +	  /* If this is just a stub, then we don't have the real name
> here.  */
>  	  if (TYPE_STUB (new_sublist->fn_field.type))
>  	    {
>  	      if (!TYPE_SELF_TYPE (new_sublist->fn_field.type))
  I suspect this is the part that generates the failure 
I saw when trying to test my pascal patch that used stabs debugging
information.
   
     internal_type_self_type  generates an internal error   
it does not simply return NULL...

> -		TYPE_SELF_TYPE (new_sublist->fn_field.type) = type;
> +		set_type_self_type (new_sublist->fn_field.type, type);
>  	      new_sublist->fn_field.is_stub = 1;
>  	    }
> +
>  	  new_sublist->fn_field.physname = savestring (*pp, p - *pp);
>  	  *pp = p + 1;

  The patch below removes the internal error,
but I am not sure it is the correct fix...
Maybe set_type_self_type should be called unconditionally.

  Likewise, the line:
valops.c:2547:    gdb_assert (TYPE_SELF_TYPE (fns_ptr[0].type) != NULL);
is not compatible with your new internal_type_self_type as this
new function never returns NULL....


Pierre Muller

$ git diff
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 2a160c5..392fdb2 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -2386,7 +2386,7 @@ read_member_functions (struct field_info *fip, char
**pp, struct type *type,
          /* If this is just a stub, then we don't have the real name here.
*/
          if (TYPE_STUB (new_sublist->fn_field.type))
            {
-             if (!TYPE_SELF_TYPE (new_sublist->fn_field.type))
+              if (TYPE_SPECIFIC_FIELD (new_sublist->fn_field.type) ==
TYPE_SPECIFIC_NONE)
                set_type_self_type (new_sublist->fn_field.type, type);
              new_sublist->fn_field.is_stub = 1;
            }