This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on'
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: ppluzhnikov at google dot com (Paul Pluzhnikov)
- Cc: tromey at redhat dot com (Tom Tromey), gdb-patches at sourceware dot org
- Date: Thu, 23 Oct 2008 00:31:37 +0200 (CEST)
- Subject: Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on'
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