Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on'

["Ulrich Weigand" <uweigand@de.ibm.com>]

Paul Pluzhnikov wrote:

> gdb/ChangeLog
> 
> 2008-08-19  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	Changes to treat 'p.x' and 'p->x' the same.
> 	* eval.c (value_static_or_dynamic_member): New.
> 	(evaluate_subexp_standard): Call it.
> 	
> gdb/testsuite/ChangeLog
> 
> 2008-08-19  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* gdb.cp/class3.exp: New test case.
> 	* gdb.cp/class3.cc: New source file.


Sorry for the late review.  Tom just pointed me to this
patch in the context of a different discussion ...

In general, I think the approach looks right to me.

However, the current patch does seem to have a couple
of issues:

> +static struct value *
> +value_static_or_dynamic_member(struct value *arg, char *string,
> +			       char *name, enum noside noside)
> +{
> +  struct type *type = value_type (arg);
> +  struct value *temp;
> +
> +  if (noside == EVAL_AVOID_SIDE_EFFECTS)
> +    return value_zero (lookup_struct_elt_type (type, string, 0),
> +		       lval_memory);
> +  temp = arg;
> +  {
> +    volatile struct gdb_exception except;
> +    struct value *v = NULL;
> +    TRY_CATCH (except, RETURN_MASK_ERROR)
> +      {
> +	v = value_struct_elt (&temp, NULL, string, NULL, name);
> +      }
> +    if (v)
> +      return v;
> +  }
> +
> +  /* If we got here, value_struct_elt() above must have thrown,
> +     and there is no field 'name' in 'type'.
> +     Try dynamic type of 'arg' instead. */
> +
> +  if (TYPE_TARGET_TYPE(type)
> +      && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS))
> +    {
> +      struct type *real_type;
> +      int full, top, using_enc;
> +      real_type = value_rtti_target_type (arg, &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);
> +
> +	  temp = arg = value_cast (real_type, arg);
> +	}
> +    }
> +  return value_struct_elt (&temp, NULL, string, NULL, name);
> +}

You're treating looking up for type (EVAL_AVOID_SIDE_EFFECTS) 
differently from looking up for value; in the first case, you
*never* look at the dynamic type -- that doesn't seem right
to me (and in fact differs from what the original code does).
Maybe you should add some "ptype p->x" tests to exercise
this path as well.

Also, you're calling value_static_or_dynamic_member both for
class types *and* for pointer types -- but the whole dynamic
type lookup as written only works for pointer types (starting
with the if (TYPE_TARGET_TYPE)).

If you really want to treat p.x and p->x equivalent in every
case, why don't you call value_static_or_dynamic_member *always*
with the class type (using a value_ind in the STRUCTOP_PTR case)?
Then you'd use value_rtti_type instead of value_rtti_target_type.


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com