Re: RTTI type improvement for (was: "Re: set print object on should affect MI varobjs (PR mi/13393)")

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

Hello Anton,

On Fri, 06 Jan 2012 16:47:08 +0100, xgsa wrote:
> > That change to value_rtti_indirect_type would be easier to read if it was
> > a separate patch.
> 
> This is the first part of my previous patch that contains the change
> to value_rtti_indirect_type. The second part requires this one to be
> applied, so I'll provide it after commit.

Unfortunately we cannot yet check it in while waiting for the copyright
paperwork.  But sure you can base the other patch on top of this one.


> Here is an example of what this patch fixes:
[...]
> The "whatis ri" command fails with message:
>   "Attempt to take contents of a non-pointer value.".
> The "whatis pi" command does not respect cv-qualifiers:
>   type = /* real type = Derived * */
>   const Base *

I do not see any change with this patch in the results of your testcase
gdb.mi/mi-var-rtti.exp .  Regressions in the future are better evaluated when
one sees the testcase and associated code which it breaks.  A separate
testcase for this patch would be nice but I assume the full changes require
this functionality so it gets tested anyway.


> gdb/ChangeLog:
> 
> 2012-01-06  Anton Gorenkov <xgsa@yandex.ru>
> 
>         * c-valprint.c (c_value_print): Use value_rtti_indirect_type
>         instead of value_rtti_target_type.
>         * eval.c (evaluate_subexp_standard): Use value_rtti_indirect_type
>         instead of value_rtti_target_type.
>         * typeprint.c (whatis_exp): Use value_rtti_indirect_type instead of
>         value_rtti_target_type.
>         * valops.c (value_ind): Extract function readjust_indirect_value_type.
>         (value_rtti_indirect_type): The replacement for value_rtti_target_type.

Here really should be value_rtti_target_type in parens, like:
	  (value_rtti_target_type): Rename to ...
	  (value_rtti_indirect_type): ... here and make it indirect.  Update
	  function comment.


>         * value.c (readjust_indirect_value_type): New function.
>         (coerce_ref): Support for enclosing type setting for references
>         with readjust_indirect_value_type.


>         * value.h (readjust_value_type): The replacement for
>         value_rtti_target_type.

Use:
	  * value.h (readjust_value_type): New declaration.
	  (value_rtti_target_type): Rename to ...
	  (value_rtti_indirect_type): ... here.



> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index 3461b08..e2e2792 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -698,22 +698,13 @@ c_value_print (struct value *val, struct ui_file *stream,
>  
>  	  if (value_entirely_available (val))
>   	    {
> -	      real_type = value_rtti_target_type (val, &full, &top, &using_enc);
> +	      real_type = value_rtti_indirect_type (val, &full, &top,
> +						    &using_enc);
>  	      if (real_type)
>  		{
>  		  /* RTTI entry found.  */
> -		  if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -		    {
> -		      /* Create a pointer type pointing to the real
> -			 type.  */
> -		      type = lookup_pointer_type (real_type);
> -		    }
> -		  else
> -		    {
> -		      /* Create a reference type referencing the real
> -			 type.  */
> -		      type = lookup_reference_type (real_type);
> -		    }
> +		  type = real_type;
> +
>  		  /* Need to adjust pointer value.  */
>  		  val = value_from_pointer (type, value_as_address (val) - top);
>  
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 5d758d1..10606a8 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -1990,16 +1990,10 @@ evaluate_subexp_standard (struct type *expect_type,
>          if (opts.objectprint && TYPE_TARGET_TYPE(type)
>              && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS))
>            {
> -            real_type = value_rtti_target_type (arg1, &full, &top, &using_enc);
> +            real_type = value_rtti_indirect_type (arg1, &full, &top,

This line should indent by tabs, not spaces.  Sure a nitpick, no problem.
Do not care if the old code was wrongly using spaces.


> +						  &using_enc);
>              if (real_type)
> -              {
> -                if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -                  real_type = lookup_pointer_type (real_type);
> -                else
> -                  real_type = lookup_reference_type (real_type);
> -
>                  arg1 = value_cast (real_type, arg1);
> -              }
>            }
>        }
>  
> diff --git a/gdb/typeprint.c b/gdb/typeprint.c
> index cf4158d..6546c98 100644
> --- a/gdb/typeprint.c
> +++ b/gdb/typeprint.c
> @@ -140,16 +140,7 @@ whatis_exp (char *exp, int show)
>        if (((TYPE_CODE (type) == TYPE_CODE_PTR)
>  	   || (TYPE_CODE (type) == TYPE_CODE_REF))
>  	  && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS))
> -        {
> -          real_type = value_rtti_target_type (val, &full, &top, &using_enc);
> -          if (real_type)
> -            {
> -              if (TYPE_CODE (type) == TYPE_CODE_PTR)
> -                real_type = lookup_pointer_type (real_type);
> -              else
> -                real_type = lookup_reference_type (real_type);
> -            }
> -        }
> +        real_type = value_rtti_indirect_type (val, &full, &top, &using_enc);
>        else if (TYPE_CODE (type) == TYPE_CODE_CLASS)
>  	real_type = value_rtti_type (val, &full, &top, &using_enc);
>      }
> diff --git a/gdb/valops.c b/gdb/valops.c
> index cb39677..2bc02f3 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -1774,15 +1774,7 @@ value_ind (struct value *arg1)
>  			      (value_as_address (arg1)
>  			       - value_pointed_to_offset (arg1)));
>  
> -      /* Re-adjust type.  */
> -      deprecated_set_value_type (arg2, TYPE_TARGET_TYPE (base_type));
> -      /* Add embedding info.  */
> -      set_value_enclosing_type (arg2, enc_type);
> -      set_value_embedded_offset (arg2, value_pointed_to_offset (arg1));
> -
> -      /* We may be pointing to an object of some derived type.  */
> -      arg2 = value_full_object (arg2, NULL, 0, 0, 0);
> -      return arg2;
> +      return readjust_indirect_value_type (arg2, enc_type, base_type, arg1);
>      }
>  
>    error (_("Attempt to take contents of a non-pointer value."));
> @@ -3528,21 +3520,48 @@ value_maybe_namespace_elt (const struct type *curtype,
>    return result;
>  }
>  
> -/* Given a pointer value V, find the real (RTTI) type of the object it
> -   points to.
> +/* Given a pointer or a reference value V, find its real (RTTI) type.
>  
>     Other parameters FULL, TOP, USING_ENC as with value_rtti_type()
>     and refer to the values computed for the object pointed to.  */
>  
>  struct type *
> -value_rtti_target_type (struct value *v, int *full, 
> -			int *top, int *using_enc)
> +value_rtti_indirect_type (struct value *v, int *full, 
> +			  int *top, int *using_enc)
>  {
>    struct value *target;
> +  struct type *type, *real_type, *target_type;
> +
> +  type = value_type (v);
> +  type = check_typedef (type);
> +  if (TYPE_CODE (type) == TYPE_CODE_REF)
> +    target = coerce_ref (v);
> +  else if (TYPE_CODE (type) == TYPE_CODE_PTR)
> +    target = value_ind (v);
> +  else
> +    return NULL;
>  
> -  target = value_ind (v);
> +  real_type = value_rtti_type (target, full, top, using_enc);
> +
> +  if (real_type)
> +    {
> +      /* Copy qualifiers to the referenced object.  */
> +      target_type = value_type (target);
> +      real_type = make_cv_type (TYPE_CONST (target_type),
> +				TYPE_VOLATILE (target_type), real_type, NULL);
> +      if (TYPE_CODE (type) == TYPE_CODE_REF)
> +        real_type = lookup_reference_type (real_type);
> +      else if (TYPE_CODE (type) == TYPE_CODE_PTR)
> +        real_type = lookup_pointer_type (real_type);
> +      else
> +        internal_error (__FILE__, __LINE__, _("Unexpected value type."));

Here should be tabs for indentation, 3 times.

> +
> +      /* Copy qualifiers to the pointer/reference.  */
> +      real_type = make_cv_type (TYPE_CONST (type), TYPE_VOLATILE (type),
> +				real_type, NULL);
> +    }
>  
> -  return value_rtti_type (target, full, top, using_enc);
> +  return real_type;
>  }
>  
>  /* Given a value pointed to by ARGP, check its real run-time type, and
> diff --git a/gdb/value.c b/gdb/value.c
> index d263d0c..c78e489 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -3109,11 +3109,30 @@ coerce_ref_if_computed (const struct value *arg)
>    return funcs->coerce_ref (arg);
>  }
>  
> +/* Look at value.h for description.  */
> +
> +struct value *
> +readjust_indirect_value_type (struct value *value, struct type *enc_type,
> +			      struct type *original_type,
> +			      struct value *original_value)
> +{
> +  /* Re-adjust type.  */
> +  deprecated_set_value_type (value, TYPE_TARGET_TYPE (original_type));
> +
> +  /* Add embedding info.  */
> +  set_value_enclosing_type (value, enc_type);
> +  set_value_embedded_offset (value, value_pointed_to_offset (original_value));
> +
> +  /* We may be pointing to an object of some derived type.  */
> +  return value_full_object (value, NULL, 0, 0, 0);
> +}
> +
>  struct value *
>  coerce_ref (struct value *arg)
>  {
>    struct type *value_type_arg_tmp = check_typedef (value_type (arg));
>    struct value *retval;
> +  struct type *enc_type;
>  
>    retval = coerce_ref_if_computed (arg);
>    if (retval)
> @@ -3122,9 +3141,13 @@ coerce_ref (struct value *arg)
>    if (TYPE_CODE (value_type_arg_tmp) != TYPE_CODE_REF)
>      return arg;
>  
> -  return value_at_lazy (TYPE_TARGET_TYPE (value_type_arg_tmp),
> -			unpack_pointer (value_type (arg),
> -					value_contents (arg)));
> +  enc_type = check_typedef (value_enclosing_type (arg));
> +  enc_type = TYPE_TARGET_TYPE (enc_type);
> +
> +  retval = value_at_lazy (enc_type,
> +                          unpack_pointer (value_type (arg),
> +                                          value_contents (arg)));
> +  return readjust_indirect_value_type (retval, enc_type, value_type_arg_tmp, arg);

Line has more than 80 characters.


>  }
>  
>  struct value *
> diff --git a/gdb/value.h b/gdb/value.h
> index d2c58ec..679192f 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -353,6 +353,19 @@ extern short *deprecated_value_regnum_hack (struct value *);
>  
>  extern struct value *coerce_ref_if_computed (const struct value *arg);
>  
> +/* Setup a new value type and enclosing value type for dereferenced value VALUE.
> +   ENC_TYPE is the new enclosing type that should be set.  ORIGINAL_TYPE and
> +   ORIGINAL_VALUE are the type and value of the original reference or pointer.
> +
> +   Note, that VALUE is modified by this function.
> +
> +   It is a common implementation for coerce_ref and value_ind.  */
> +
> +extern struct value *
> +readjust_indirect_value_type (struct value *value, struct type *enc_type,
> +			      struct type *original_type,
> +			      struct value *original_value);
> +
>  /* Convert a REF to the object referenced.  */
>  
>  extern struct value *coerce_ref (struct value *value);
> @@ -617,8 +630,8 @@ extern struct value *value_primitive_field (struct value *arg1, int offset,
>  					    struct type *arg_type);
>  
>  
> -extern struct type *value_rtti_target_type (struct value *, int *, int *,
> -					    int *);
> +extern struct type *value_rtti_indirect_type (struct value *, int *, int *,
> +					      int *);
>  
>  extern struct value *value_full_object (struct value *, struct type *, int,
>  					int, int);


In general OK, prepared for a check-in.


Thanks,
Jan