This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Simpify varobj children handling for C++
- From: Vladimir Prus <ghost at cs dot msu dot su>
- To: gdb-patches at sources dot redhat dot com
- Date: Mon, 25 Dec 2006 11:16:15 +0300
- Subject: Re: Simpify varobj children handling for C++
- References: <200612240050.30172.ghost@cs.msu.su>
Vladimir Prus wrote:
>
> This patch merges the code for getting name, value and type of
> varobj children for C++ into one function, just like the previously
> posted patch for C.
>
> This was manually tested from KDevelop. We really need tests
> for MI C++ functionality, and I plan to write them, but only
> after my patch that makes it possible to write MI tests using
> just C files is approved.
>
> Is this patch OK provided that tests will be writted before committing?
Here's a revised version of this. I've noticed that the new
adjust_value_for_children_access function subsumes get_type_deref,
and therefore this patch removes get_type_deref.
- Volodya
Refactor getting children name, value and type access
for varobjs in C++.
* value.c (value_as_address): Use coerce_array_proper
instead of coerce_array so that not fail for references.
(coerce_array_proper): New function.
(coerce_array): Use the above.
* value.h (coerce_array_proper): Declare.
* valops.c (value_ind): Handle references.
* varobj.c (get_type_deref): Remove.
(adjust_value_for_children_access): New.
(c_number_of_children): Use the above.
(c_describe_child): Likewise.
(enum accessibility): New.
(match_accessibility): New function.
(cplus_describe_child): New function.
(cplus_name_of_child, cplus_value_of_child)
(cplus_type_of_child): Reimplement in terms
of cplus_describe_child.
(cplus_number_of_children): Use
adjust_value_for_children_access.
--- gdb/value.c (/patches/gdb/path_3_unify/gdb_mainline) (revision 2937)
+++ gdb/value.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2937)
@@ -1006,7 +1006,7 @@ value_as_address (struct value *val)
|| TYPE_CODE (value_type (val)) == TYPE_CODE_METHOD)
return VALUE_ADDRESS (val);
- val = coerce_array (val);
+ val = coerce_array_proper (val);
/* Some architectures (e.g. Harvard), map instruction and data
addresses onto a single large unified address space. For
@@ -1621,9 +1621,8 @@ coerce_ref (struct value *arg)
}
struct value *
-coerce_array (struct value *arg)
+coerce_array_proper (struct value *arg)
{
- arg = coerce_ref (arg);
if (current_language->c_style_arrays
&& TYPE_CODE (value_type (arg)) == TYPE_CODE_ARRAY)
arg = value_coerce_array (arg);
@@ -1632,6 +1631,13 @@ coerce_array (struct value *arg)
return arg;
}
+
+struct value *
+coerce_array (struct value *arg)
+{
+ return coerce_array_proper (coerce_ref (arg));
+}
+
struct value *
coerce_number (struct value *arg)
{
--- gdb/value.h (/patches/gdb/path_3_unify/gdb_mainline) (revision 2937)
+++ gdb/value.h (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2937)
@@ -224,6 +224,8 @@ extern short *deprecated_value_regnum_ha
extern struct value *coerce_ref (struct value *value);
+extern struct value *coerce_array_proper (struct value *arg);
+
/* If ARG is an array, convert it to a pointer.
If ARG is an enum, convert it to an integer.
If ARG is a function, convert it to a function pointer.
--- gdb/testsuite/gdb.mi/mi-var-cp.cc (/patches/gdb/path_3_unify/gdb_mainline) (revision 2937)
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2937)
@@ -39,7 +39,6 @@ void base_in_reference_test_main ()
base_in_reference_test (s);
}
-
int main ()
{
reference_update_tests ();
--- gdb/valops.c (/patches/gdb/path_3_unify/gdb_mainline) (revision 2937)
+++ gdb/valops.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2937)
@@ -950,7 +950,8 @@ value_ind (struct value *arg1)
if (TYPE_CODE (base_type) == TYPE_CODE_INT)
return value_at_lazy (builtin_type_int,
(CORE_ADDR) value_as_long (arg1));
- else if (TYPE_CODE (base_type) == TYPE_CODE_PTR)
+ else if (TYPE_CODE (base_type) == TYPE_CODE_PTR
+ || TYPE_CODE (base_type) == TYPE_CODE_PTR)
{
struct type *enc_type;
/* We may be pointing to something embedded in a larger object */
--- gdb/varobj.c (/patches/gdb/path_3_unify/gdb_mainline) (revision 2937)
+++ gdb/varobj.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2937)
@@ -175,8 +175,6 @@ static struct cleanup *make_cleanup_free
static struct type *get_type (struct varobj *var);
-static struct type *get_type_deref (struct varobj *var);
-
static struct type *get_target_type (struct type *);
static enum varobj_display_formats variable_default_display (struct varobj *);
@@ -1435,21 +1433,6 @@ get_type (struct varobj *var)
return type;
}
-/* This returns the type of the variable, dereferencing pointers, too. */
-static struct type *
-get_type_deref (struct varobj *var)
-{
- struct type *type;
-
- type = get_type (var);
-
- if (type != NULL && (TYPE_CODE (type) == TYPE_CODE_PTR
- || TYPE_CODE (type) == TYPE_CODE_REF))
- type = get_target_type (type);
-
- return type;
-}
-
/* This returns the target type (or NULL) of TYPE, also skipping
past typedefs, just like get_type ().
@@ -1690,17 +1673,68 @@ varobj_value_is_changeable_p (struct var
return r;
}
+/* Given a value and a type of a variable object,
+ adjust those value and type to those necessary
+ for getting childrens of the variable object.
+ This includes dereferencing top-level reference
+ to all types and dereferencing pointers to
+ structures.
+
+ Both TYPE and *TYPE should be non-null. VALUE
+ can be null if we want to only translate type.
+ *VALUE can be null as well -- if the parent
+ value is not known. */
+static void
+adjust_value_for_children_access (struct value **value,
+ struct type **type)
+{
+ gdb_assert (type && *type);
+
+ *type = check_typedef (*type);
+
+ /* If the parent is reference, we always strip the
+ reference when getting children, since in C++,
+ reference is basically undistinguishable in
+ usage from a plain variable. */
+
+ if (TYPE_CODE (*type) == TYPE_CODE_REF)
+ {
+ struct type *target_type = get_target_type (*type);
+ if (value && *value)
+ gdb_value_ind (*value, value);
+ *type = get_target_type (*type);
+ }
+
+ /* The 'get_target_type' function call check_typedef on
+ result, so we can immediately check type code. No
+ need to call check_typedef here. */
+
+ /* Pointers to structures are treated just like
+ structures when accessing children. Don't
+ dererences pointers to other types. */
+ if (TYPE_CODE (*type) == TYPE_CODE_PTR)
+ {
+ struct type *target_type = get_target_type (*type);
+ if (TYPE_CODE (target_type) == TYPE_CODE_STRUCT
+ || TYPE_CODE (target_type) == TYPE_CODE_UNION)
+ {
+ if (value && *value)
+ gdb_value_ind (*value, value);
+ *type = target_type;
+ }
+ }
+}
+
/* C */
static int
c_number_of_children (struct varobj *var)
{
- struct type *type;
+ struct type *type = var->type;
+ int children = 0;
struct type *target;
- int children;
- type = get_type (var);
+ adjust_value_for_children_access (NULL, &type);
target = get_target_type (type);
- children = 0;
switch (TYPE_CODE (type))
{
@@ -1718,30 +1752,18 @@ c_number_of_children (struct varobj *var
break;
case TYPE_CODE_PTR:
- /* This is where things get compilcated. All pointers have one child.
- Except, of course, for struct and union ptr, which we automagically
- dereference for the user and function ptrs, which have no children.
- We also don't dereference void* as we don't know what to show.
+ /* The type here is a pointer to non-struct. Typically, pointers
+ have one child, except for function ptrs, which have no children,
+ and except for void*, as we don't know what to show.
We can show char* so we allow it to be dereferenced. If you decide
to test for it, please mind that a little magic is necessary to
properly identify it: char* has TYPE_CODE == TYPE_CODE_INT and
TYPE_NAME == "char" */
-
- switch (TYPE_CODE (target))
- {
- case TYPE_CODE_STRUCT:
- case TYPE_CODE_UNION:
- children = TYPE_NFIELDS (target);
- break;
-
- case TYPE_CODE_FUNC:
- case TYPE_CODE_VOID:
- children = 0;
- break;
-
- default:
- children = 1;
- }
+ if (TYPE_CODE (target) == TYPE_CODE_FUNC
+ || TYPE_CODE (target) == TYPE_CODE_VOID)
+ children = 0;
+ else
+ children = 1;
break;
default:
@@ -1816,19 +1838,7 @@ c_describe_child (struct varobj *parent,
if (ctype)
*ctype = NULL;
- /* Pointers to structures are treated just like
- structures when accessing children. */
- if (TYPE_CODE (type) == TYPE_CODE_PTR)
- {
- struct type *target_type = get_target_type (type);
- if (TYPE_CODE (target_type) == TYPE_CODE_STRUCT
- || TYPE_CODE (target_type) == TYPE_CODE_UNION)
- {
- if (value)
- gdb_value_ind (value, &value);
- type = target_type;
- }
- }
+ adjust_value_for_children_access (&value, &type);
switch (TYPE_CODE (type))
{
@@ -1876,6 +1886,10 @@ c_describe_child (struct varobj *parent,
if (cvalue && value)
gdb_value_ind (value, cvalue);
+ /* The get_target_type function calls check_typedef
+ on the result. I'm not sure if showing check_typedefed
+ type for the child as opposed to the declared type is
+ right. */
if (ctype)
*ctype = get_target_type (type);
@@ -2051,7 +2065,8 @@ cplus_number_of_children (struct varobj
if (!CPLUS_FAKE_CHILD (var))
{
- type = get_type_deref (var);
+ type = var->type;
+ adjust_value_for_children_access (NULL, &type);
if (((TYPE_CODE (type)) == TYPE_CODE_STRUCT) ||
((TYPE_CODE (type)) == TYPE_CODE_UNION))
@@ -2077,7 +2092,8 @@ cplus_number_of_children (struct varobj
{
int kids[3];
- type = get_type_deref (var->parent);
+ type = var->parent->type;
+ adjust_value_for_children_access (NULL, &type);
cplus_class_num_children (type, kids);
if (strcmp (var->name, "public") == 0)
@@ -2128,25 +2144,56 @@ cplus_name_of_variable (struct varobj *p
return c_name_of_variable (parent);
}
-static char *
-cplus_name_of_child (struct varobj *parent, int index)
+enum accessibility { private_field, protected_field, public_field };
+
+/* Check if field INDEX of TYPE has the specified accessibility.
+ Return 0 if so and 1 otherwise. */
+static int
+match_accessibility (struct type *type, int index, enum accessibility acc)
+{
+ if (acc == private_field && TYPE_FIELD_PRIVATE (type, index))
+ return 1;
+ else if (acc == protected_field && TYPE_FIELD_PROTECTED (type, index))
+ return 1;
+ else if (acc == public_field && !TYPE_FIELD_PRIVATE (type, index)
+ && !TYPE_FIELD_PROTECTED (type, index))
+ return 1;
+ else
+ return 0;
+}
+
+static void
+cplus_describe_child (struct varobj *parent, int index,
+ char **cname, struct value **cvalue, struct type **ctype)
{
- char *name;
+ char *name = 0;
+ struct value *value;
struct type *type;
+ if (cname)
+ *cname = NULL;
+ if (cvalue)
+ *cvalue = NULL;
+ if (ctype)
+ *ctype = NULL;
+
+
if (CPLUS_FAKE_CHILD (parent))
{
- /* Looking for children of public, private, or protected. */
- type = get_type_deref (parent->parent);
+ value = parent->parent->value;
+ type = get_type (parent->parent);
}
else
- type = get_type_deref (parent);
+ {
+ value = parent->value;
+ type = get_type (parent);
+ }
- name = NULL;
- switch (TYPE_CODE (type))
+ adjust_value_for_children_access (&value, &type);
+
+ if (TYPE_CODE (type) == TYPE_CODE_STRUCT
+ || TYPE_CODE (type) == TYPE_CODE_STRUCT)
{
- case TYPE_CODE_STRUCT:
- case TYPE_CODE_UNION:
if (CPLUS_FAKE_CHILD (parent))
{
/* The fields of the class type are ordered as they
@@ -2156,56 +2203,54 @@ cplus_name_of_child (struct varobj *pare
have the access control we are looking for to properly
find the indexed field. */
int type_index = TYPE_N_BASECLASSES (type);
+ enum accessibility acc = public_field;
if (strcmp (parent->name, "private") == 0)
- {
- while (index >= 0)
- {
- if (TYPE_VPTR_BASETYPE (type) == type
- && type_index == TYPE_VPTR_FIELDNO (type))
- ; /* ignore vptr */
- else if (TYPE_FIELD_PRIVATE (type, type_index))
- --index;
- ++type_index;
- }
- --type_index;
- }
+ acc = private_field;
else if (strcmp (parent->name, "protected") == 0)
+ acc = protected_field;
+
+ while (index >= 0)
{
- while (index >= 0)
- {
- if (TYPE_VPTR_BASETYPE (type) == type
- && type_index == TYPE_VPTR_FIELDNO (type))
- ; /* ignore vptr */
- else if (TYPE_FIELD_PROTECTED (type, type_index))
+ if (TYPE_VPTR_BASETYPE (type) == type
+ && type_index == TYPE_VPTR_FIELDNO (type))
+ ; /* ignore vptr */
+ else if (match_accessibility (type, type_index, acc))
--index;
++type_index;
- }
- --type_index;
}
- else
+ --type_index;
+
+ if (cname)
+ *cname = TYPE_FIELD_NAME (type, type_index);
+
+ if (cvalue && value)
+ *cvalue = value_struct_element_index (value, type_index);
+
+ if (ctype)
+ *ctype = TYPE_FIELD_TYPE (type, type_index);
+ }
+ else if (index < TYPE_N_BASECLASSES (type))
+ {
+ /* This is baseclass. */
+ if (cname)
+ *cname = TYPE_FIELD_NAME (type, index);
+
+ if (cvalue && value)
{
- while (index >= 0)
- {
- if (TYPE_VPTR_BASETYPE (type) == type
- && type_index == TYPE_VPTR_FIELDNO (type))
- ; /* ignore vptr */
- else if (!TYPE_FIELD_PRIVATE (type, type_index) &&
- !TYPE_FIELD_PROTECTED (type, type_index))
- --index;
- ++type_index;
- }
- --type_index;
+ *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value);
+ release_value (*cvalue);
}
- name = TYPE_FIELD_NAME (type, type_index);
+ if (ctype)
+ {
+ *ctype = TYPE_FIELD_TYPE (type, index);
+ }
}
- else if (index < TYPE_N_BASECLASSES (type))
- /* We are looking up the name of a base class */
- name = TYPE_FIELD_NAME (type, index);
else
{
+ char *access = 0;
int children[3];
- cplus_class_num_children(type, children);
+ cplus_class_num_children (type, children);
/* Everything beyond the baseclasses can
only be "public", "private", or "protected"
@@ -2217,46 +2262,49 @@ cplus_name_of_child (struct varobj *pare
{
case 0:
if (children[v_public] > 0)
- name = "public";
+ access = "public";
else if (children[v_private] > 0)
- name = "private";
+ access = "private";
else
- name = "protected";
+ access = "protected";
break;
case 1:
if (children[v_public] > 0)
{
if (children[v_private] > 0)
- name = "private";
+ access = "private";
else
- name = "protected";
+ access = "protected";
}
else if (children[v_private] > 0)
- name = "protected";
+ access = "protected";
break;
case 2:
/* Must be protected */
- name = "protected";
+ access = "protected";
break;
default:
/* error! */
break;
}
- }
- break;
+
+ if (cname)
+ *cname = access;
- default:
- break;
+ /* Value and type are null here. */
+ }
}
-
- if (name == NULL)
- return c_name_of_child (parent, index);
else
{
- if (name != NULL)
- name = savestring (name, strlen (name));
- }
+ c_describe_child (parent, index, cname, cvalue, ctype);
+ }
+}
+static char *
+cplus_name_of_child (struct varobj *parent, int index)
+{
+ char *name = NULL;
+ cplus_describe_child (parent, index, &name, NULL, NULL);
return name;
}
@@ -2269,118 +2317,16 @@ cplus_value_of_root (struct varobj **var
static struct value *
cplus_value_of_child (struct varobj *parent, int index)
{
- struct type *type;
- struct value *value;
-
- if (CPLUS_FAKE_CHILD (parent))
- type = get_type_deref (parent->parent);
- else
- type = get_type_deref (parent);
-
- value = NULL;
-
- if (((TYPE_CODE (type)) == TYPE_CODE_STRUCT) ||
- ((TYPE_CODE (type)) == TYPE_CODE_UNION))
- {
- if (CPLUS_FAKE_CHILD (parent))
- {
- char *name;
- struct value *temp = parent->parent->value;
-
- if (temp == NULL)
- return NULL;
-
- name = name_of_child (parent, index);
- gdb_value_struct_elt (NULL, &value, &temp, NULL, name, NULL,
- "cplus_structure");
- if (value != NULL)
- release_value (value);
-
- xfree (name);
- }
- else if (index >= TYPE_N_BASECLASSES (type))
- {
- /* public, private, or protected */
- return NULL;
- }
- else
- {
- /* Baseclass */
- if (parent->value != NULL)
- {
- struct value *temp = NULL;
-
- /* No special processing for references is needed --
- value_cast below handles references. */
- if (TYPE_CODE (value_type (parent->value)) == TYPE_CODE_PTR)
- {
- if (!gdb_value_ind (parent->value, &temp))
- return NULL;
- }
- else
- temp = parent->value;
-
- if (temp != NULL)
- {
- value = value_cast (TYPE_FIELD_TYPE (type, index), temp);
- release_value (value);
- }
- else
- {
- /* We failed to evaluate the parent's value, so don't even
- bother trying to evaluate this child. */
- return NULL;
- }
- }
- }
- }
-
- if (value == NULL)
- return c_value_of_child (parent, index);
-
+ struct value *value = NULL;
+ cplus_describe_child (parent, index, NULL, &value, NULL);
return value;
}
static struct type *
cplus_type_of_child (struct varobj *parent, int index)
{
- struct type *type, *t;
-
- if (CPLUS_FAKE_CHILD (parent))
- {
- /* Looking for the type of a child of public, private, or protected. */
- t = get_type_deref (parent->parent);
- }
- else
- t = get_type_deref (parent);
-
- type = NULL;
- switch (TYPE_CODE (t))
- {
- case TYPE_CODE_STRUCT:
- case TYPE_CODE_UNION:
- if (CPLUS_FAKE_CHILD (parent))
- {
- char *name = cplus_name_of_child (parent, index);
- type = lookup_struct_elt_type (t, name, 0);
- xfree (name);
- }
- else if (index < TYPE_N_BASECLASSES (t))
- type = TYPE_FIELD_TYPE (t, index);
- else
- {
- /* special */
- return NULL;
- }
- break;
-
- default:
- break;
- }
-
- if (type == NULL)
- return c_type_of_child (parent, index);
-
+ struct type *type = NULL;
+ cplus_describe_child (parent, index, NULL, NULL, &type);
return type;
}
Property changes on:
___________________________________________________________________
Name: csl:base
-/all/patches/gdb/path_2_children/gdb_mainline
+/all/patches/gdb/path_3_unify/gdb_mainline
Name: svk:merge
d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_1/gdb_mainline:2869
+d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_3_unify/gdb_mainline:2930
d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_expression/gdb_mainline:2562
e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:157978