* Simpify varobj children handling for C++
@ 2006-12-23 21:51 Vladimir Prus
2006-12-25 8:16 ` Vladimir Prus
0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Prus @ 2006-12-23 21:51 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]
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?
- 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 (adjust_value_for_children_access): New.
(c_describe_child): Use the above.
(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.
[-- Attachment #2: path_4_unify_cpp__gdb_mainline.diff --]
[-- Type: text/x-diff, Size: 13830 bytes --]
--- gdb/value.c (/patches/gdb/path_3_unify/gdb_mainline) (revision 2936)
+++ gdb/value.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2936)
@@ -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 2936)
+++ gdb/value.h (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2936)
@@ -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 2936)
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2936)
@@ -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 2936)
+++ gdb/valops.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2936)
@@ -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 2936)
+++ gdb/varobj.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 2936)
@@ -1792,6 +1792,45 @@ value_struct_element_index (struct value
}
}
+/* 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. */
+static void
+adjust_value_for_children_access (struct value **value,
+ struct type **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)
+ gdb_value_ind (*value, value);
+ *type = get_target_type (*type);
+ }
+
+ /* 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)
+ gdb_value_ind (*value, value);
+ *type = target_type;
+ }
+ }
+}
+
/* Obtain the information about child INDEX of the variable
object PARENT.
If CNAME is not null, sets *CNAME to the name of the child relative
@@ -1816,19 +1855,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))
{
@@ -2128,25 +2155,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 +2214,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 +2273,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 +2328,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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Simpify varobj children handling for C++
2006-12-23 21:51 Simpify varobj children handling for C++ Vladimir Prus
@ 2006-12-25 8:16 ` Vladimir Prus
2007-01-03 22:32 ` Daniel Jacobowitz
0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Prus @ 2006-12-25 8:16 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: path_4_unify_cpp__gdb_mainline.diff --]
[-- Type: text/x-diff; name="path_4_unify_cpp__gdb_mainline.diff", Size: 17618 bytes --]
--- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Simpify varobj children handling for C++
2006-12-25 8:16 ` Vladimir Prus
@ 2007-01-03 22:32 ` Daniel Jacobowitz
2007-01-17 18:57 ` Vladimir Prus
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2007-01-03 22:32 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
On Mon, Dec 25, 2006 at 11:16:15AM +0300, Vladimir Prus wrote:
> * 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.
I definitely need more information on this. What fails beforehand,
and how did you get there?
> @@ -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 */
Something tells me you didn't actually need value_ind to handle
references... :-)
> +/* Given a value and a type of a variable object,
"the value and the type of a variable object", because
the value is the property of some specific variable object.
> + adjust those value and type to those necessary
"adjust the"
> + for getting childrens of the variable object.
"children"
> + This includes dereferencing top-level reference
> + to all types and dereferencing pointers to
> + structures.
"references" to match "all types"
> +static void
> +adjust_value_for_children_access (struct value **value,
> + struct type **type)
"adjust_value_for_child_access", also.
> +{
> + 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. */
"is a reference", "in C++, a reference".
> + /* The 'get_target_type' function call check_typedef on
"calls"
> @@ -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);
>
It'd be nice if we didn't do that. We go to some effort to show
typedefs in the CLI for ptype.
> + /* This is baseclass. */
"is a"
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Simpify varobj children handling for C++
2007-01-03 22:32 ` Daniel Jacobowitz
@ 2007-01-17 18:57 ` Vladimir Prus
2007-01-20 19:42 ` Daniel Jacobowitz
0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Prus @ 2007-01-17 18:57 UTC (permalink / raw)
To: Daniel Jacobowitz, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]
Daniel Jacobowitz wrote:
> On Mon, Dec 25, 2006 at 11:16:15AM +0300, Vladimir Prus wrote:
>> * 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.
>
> I definitely need more information on this. What fails beforehand,
> and how did you get there?
In truth, this might not be needed. Previously I found that
value_ind just errors out on references and tried to fix this
in this patch.
>> @@ -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 */
>
> Something tells me you didn't actually need value_ind to handle
> references... :-)
Why? Because that's a bad idea or because varobj code does not
need to dereference rereferences? The latter is true,
attached version of the patch builds on the
"fix 'editable' attribute for references" patch I've posted earlier
and now 'adjust_value_for_child_access' asserts that the type
is not reference.
>> @@ -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);
>>
>
> It'd be nice if we didn't do that. We go to some effort to show
> typedefs in the CLI for ptype.
Changed. Revised patch attached, OK?
- Volodya
Refactor getting children name, value and type access
for varobjs in C++.
* varobj.c (get_type_deref): Remove.
(adjust_value_for_child_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_child_access.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: delta.diff --]
[-- Type: text/x-diff; name="delta.diff", Size: 7593 bytes --]
--- gdb/valops.c (revision 3175)
+++ gdb/valops.c (local)
@@ -935,8 +935,7 @@ 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
- || TYPE_CODE (base_type) == TYPE_CODE_PTR)
+ else if (TYPE_CODE (base_type) == TYPE_CODE_PTR)
{
struct type *enc_type;
/* We may be pointing to something embedded in a larger object */
--- gdb/value.c (revision 3175)
+++ gdb/value.c (local)
@@ -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_proper (val);
+ val = coerce_array (val);
/* Some architectures (e.g. Harvard), map instruction and data
addresses onto a single large unified address space. For
@@ -1620,8 +1620,9 @@ coerce_ref (struct value *arg)
}
struct value *
-coerce_array_proper (struct value *arg)
+coerce_array (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);
@@ -1630,13 +1631,6 @@ coerce_array_proper (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 (revision 3175)
+++ gdb/value.h (local)
@@ -224,8 +224,6 @@ 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/varobj.c (revision 3175)
+++ gdb/varobj.c (local)
@@ -1749,10 +1749,10 @@ 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
+/* Given the value and the type of a variable object,
+ adjust the value and type to those necessary
+ for getting children of the variable object.
+ This includes dereferencing top-level references
to all types and dereferencing pointers to
structures.
@@ -1761,29 +1761,18 @@ varobj_value_is_changeable_p (struct var
*VALUE can be null as well -- if the parent
value is not known. */
static void
-adjust_value_for_children_access (struct value **value,
+adjust_value_for_child_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. */
+ /* The type of value stored in varobj, that is passed
+ to us, is already supposed to be
+ reference-stripped. */
- 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. */
+ gdb_assert (TYPE_CODE (*type) != TYPE_CODE_REF);
/* Pointers to structures are treated just like
structures when accessing children. Don't
@@ -1799,17 +1788,21 @@ adjust_value_for_children_access (struct
*type = target_type;
}
}
+
+ /* The 'get_target_type' function calls check_typedef on
+ result, so we can immediately check type code. No
+ need to call check_typedef here. */
}
/* C */
static int
c_number_of_children (struct varobj *var)
{
- struct type *type = var->type;
+ struct type *type = get_value_type (var);
int children = 0;
struct type *target;
- adjust_value_for_children_access (NULL, &type);
+ adjust_value_for_child_access (NULL, &type);
target = get_target_type (type);
switch (TYPE_CODE (type))
@@ -1908,7 +1901,7 @@ c_describe_child (struct varobj *parent,
char **cname, struct value **cvalue, struct type **ctype)
{
struct value *value = parent->value;
- struct type *type = get_type (parent);
+ struct type *type = get_value_type (parent);
if (cname)
*cname = NULL;
@@ -1917,7 +1910,7 @@ c_describe_child (struct varobj *parent,
if (ctype)
*ctype = NULL;
- adjust_value_for_children_access (&value, &type);
+ adjust_value_for_child_access (&value, &type);
switch (TYPE_CODE (type))
{
@@ -1965,12 +1958,11 @@ 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. */
+ /* Don't use get_target_type because it calls
+ check_typedef and here, we want to show the true
+ declared type of the variable. */
if (ctype)
- *ctype = get_target_type (type);
+ *ctype = TYPE_TARGET_TYPE (type);
break;
@@ -2137,8 +2129,8 @@ cplus_number_of_children (struct varobj
if (!CPLUS_FAKE_CHILD (var))
{
- type = var->type;
- adjust_value_for_children_access (NULL, &type);
+ type = get_value_type (var);
+ adjust_value_for_child_access (NULL, &type);
if (((TYPE_CODE (type)) == TYPE_CODE_STRUCT) ||
((TYPE_CODE (type)) == TYPE_CODE_UNION))
@@ -2164,8 +2156,8 @@ cplus_number_of_children (struct varobj
{
int kids[3];
- type = var->parent->type;
- adjust_value_for_children_access (NULL, &type);
+ type = get_value_type (var->parent);
+ adjust_value_for_child_access (NULL, &type);
cplus_class_num_children (type, kids);
if (strcmp (var->name, "public") == 0)
@@ -2253,15 +2245,15 @@ cplus_describe_child (struct varobj *par
if (CPLUS_FAKE_CHILD (parent))
{
value = parent->parent->value;
- type = get_type (parent->parent);
+ type = get_value_type (parent->parent);
}
else
{
value = parent->value;
- type = get_type (parent);
+ type = get_value_type (parent);
}
- adjust_value_for_children_access (&value, &type);
+ adjust_value_for_child_access (&value, &type);
if (TYPE_CODE (type) == TYPE_CODE_STRUCT
|| TYPE_CODE (type) == TYPE_CODE_STRUCT)
@@ -2293,7 +2285,7 @@ cplus_describe_child (struct varobj *par
--type_index;
if (cname)
- *cname = TYPE_FIELD_NAME (type, type_index);
+ *cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
if (cvalue && value)
*cvalue = value_struct_element_index (value, type_index);
@@ -2303,9 +2295,9 @@ cplus_describe_child (struct varobj *par
}
else if (index < TYPE_N_BASECLASSES (type))
{
- /* This is baseclass. */
+ /* This is a baseclass. */
if (cname)
- *cname = TYPE_FIELD_NAME (type, index);
+ *cname = xstrdup (TYPE_FIELD_NAME (type, index));
if (cvalue && value)
{
@@ -2361,7 +2353,7 @@ cplus_describe_child (struct varobj *par
}
if (cname)
- *cname = access;
+ *cname = xstrdup (access);
/* Value and type are null here. */
}
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: path_4_unify_cpp__gdb_mainline.diff --]
[-- Type: text/x-diff; name="path_4_unify_cpp__gdb_mainline.diff", Size: 19385 bytes --]
--- gdb/testsuite/gdb.mi/mi-var-cp.exp (/patches/gdb/path_3_unify/gdb_mainline) (revision 3176)
+++ gdb/testsuite/gdb.mi/mi-var-cp.exp (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 3176)
@@ -44,6 +44,7 @@ mi_prepare_inline_tests $srcfile
mi_run_inline_test reference_update
mi_run_inline_test base_in_reference
mi_run_inline_test reference_to_pointer
+mi_run_inline_test reference_to_struct
mi_gdb_exit
return 0
--- gdb/testsuite/gdb.mi/mi-var-cp.cc (/patches/gdb/path_3_unify/gdb_mainline) (revision 3176)
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 3176)
@@ -30,6 +30,7 @@ void reference_update_tests ()
:*/
x = 567;
/*: mi_varobj_update RX {} "update RX (3)"
+ mi_delete_varobj RX "delete RX"
:*/
/* Dummy assignment to keep 'x' in scope. */
x = 444;
@@ -59,7 +60,8 @@ int base_in_reference_test (S2& s2)
mi_check_varobj_value "S2.S.public.i" "67" "check S2.S.public.i"
mi_check_varobj_value "S2.S.public.j" "89" "check S2.S.public.j"
-
+ mi_delete_varobj S2 "delete S2"
+
:*/
/*: END: base_in_reference :*/
}
@@ -91,15 +93,39 @@ int reference_to_pointer ()
mi_check_varobj_value RPTR.public.i 67 "check i member"
mi_check_varobj_value RPTR.public.j 89 "check j member"
+ mi_delete_varobj RPTR "delete RPTR"
:*/
return 99;
/*: END: reference_to_pointer :*/
}
+int reference_to_struct ()
+{
+ /*: BEGIN: reference_to_struct :*/
+ S s = {7, 8};
+ S& r = s;
+ /*:
+ mi_create_varobj S s "create varobj for s"
+ mi_create_varobj R r "create varobj for s"
+ mi_gdb_test "-var-show-attributes S" \
+ "\\^done,attr=\"noneditable\"" \
+ "check attributes of S"
+ mi_gdb_test "-var-show-attributes R" \
+ "\\^done,attr=\"noneditable\"" \
+ "check attributes of R"
+ :*/
+ s.i = 56;
+ /*: mi_varobj_update * [] "-var-update should not list structure varobjs"
+ :*/
+ return 99;
+ /*: END: reference_to_struct :*/
+}
+
int main ()
{
reference_update_tests ();
base_in_reference_test_main ();
reference_to_pointer ();
+ reference_to_struct ();
return 0;
}
--- gdb/testsuite/lib/mi-support.exp (/patches/gdb/path_3_unify/gdb_mainline) (revision 3176)
+++ gdb/testsuite/lib/mi-support.exp (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 3176)
@@ -1023,6 +1023,13 @@ proc mi_create_varobj { name expression
$testname
}
+# Deletes the specified NAME.
+proc mi_delete_varobj { name testname } {
+ mi_gdb_test "-var-delete $name" \
+ "\\^done,ndeleted=.*" \
+ $testname
+}
+
# Updates varobj named NAME and checks that all varobjs in EXPECTED
# are reported as updated, and no other varobj is updated.
# Assumes that no varobj is out of scope and that no varobj changes
--- gdb/varobj.c (/patches/gdb/path_3_unify/gdb_mainline) (revision 3176)
+++ gdb/varobj.c (/patches/gdb/path_4_unify_cpp/gdb_mainline) (revision 3176)
@@ -176,7 +176,7 @@ 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_value_type (struct varobj *var);
static struct type *get_target_type (struct type *);
@@ -1459,22 +1459,31 @@ get_type (struct varobj *var)
return type;
}
-/* This returns the type of the variable, dereferencing references, pointers
- and references to pointers, too. */
+/* Return the type of the value that's stored in VAR,
+ or that would have being stored there if the
+ value were accessible.
+
+ This differs from VAR->type in that VAR->type is always
+ the true type of the expession in the source language.
+ The return value of this function is the type we're
+ actually storing in varobj, and using for displaying
+ the values and for comparing previous and new values.
+
+ For example, top-level references are always stripped. */
static struct type *
-get_type_deref (struct varobj *var)
+get_value_type (struct varobj *var)
{
struct type *type;
- type = get_type (var);
+ if (var->value)
+ type = value_type (var->value);
+ else
+ type = var->type;
- if (type)
- {
- if (TYPE_CODE (type) == TYPE_CODE_REF)
- type = get_target_type (type);
- if (TYPE_CODE (type) == TYPE_CODE_PTR)
- type = get_target_type (type);
- }
+ if (TYPE_CODE (type) == TYPE_CODE_REF)
+ type = get_target_type (type);
+
+ type = check_typedef (type);
return type;
}
@@ -1723,7 +1732,7 @@ varobj_value_is_changeable_p (struct var
if (CPLUS_FAKE_CHILD (var))
return 0;
- type = get_type (var);
+ type = get_value_type (var);
switch (TYPE_CODE (type))
{
@@ -1740,17 +1749,61 @@ varobj_value_is_changeable_p (struct var
return r;
}
+/* Given the value and the type of a variable object,
+ adjust the value and type to those necessary
+ for getting children of the variable object.
+ This includes dereferencing top-level references
+ 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_child_access (struct value **value,
+ struct type **type)
+{
+ gdb_assert (type && *type);
+
+ *type = check_typedef (*type);
+
+ /* The type of value stored in varobj, that is passed
+ to us, is already supposed to be
+ reference-stripped. */
+
+ gdb_assert (TYPE_CODE (*type) != TYPE_CODE_REF);
+
+ /* 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;
+ }
+ }
+
+ /* The 'get_target_type' function calls check_typedef on
+ result, so we can immediately check type code. No
+ need to call check_typedef here. */
+}
+
/* C */
static int
c_number_of_children (struct varobj *var)
{
- struct type *type;
+ struct type *type = get_value_type (var);
+ int children = 0;
struct type *target;
- int children;
- type = get_type (var);
+ adjust_value_for_child_access (NULL, &type);
target = get_target_type (type);
- children = 0;
switch (TYPE_CODE (type))
{
@@ -1770,30 +1823,19 @@ c_number_of_children (struct varobj *var
break;
case TYPE_CODE_PTR:
- /* This is where things get complicated. 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:
@@ -1859,7 +1901,7 @@ c_describe_child (struct varobj *parent,
char **cname, struct value **cvalue, struct type **ctype)
{
struct value *value = parent->value;
- struct type *type = get_type (parent);
+ struct type *type = get_value_type (parent);
if (cname)
*cname = NULL;
@@ -1868,19 +1910,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_child_access (&value, &type);
switch (TYPE_CODE (type))
{
@@ -1928,8 +1958,11 @@ c_describe_child (struct varobj *parent,
if (cvalue && value)
gdb_value_ind (value, cvalue);
+ /* Don't use get_target_type because it calls
+ check_typedef and here, we want to show the true
+ declared type of the variable. */
if (ctype)
- *ctype = get_target_type (type);
+ *ctype = TYPE_TARGET_TYPE (type);
break;
@@ -2020,7 +2053,7 @@ c_type_of_child (struct varobj *parent,
static int
c_variable_editable (struct varobj *var)
{
- switch (TYPE_CODE (get_type (var)))
+ switch (TYPE_CODE (get_value_type (var)))
{
case TYPE_CODE_STRUCT:
case TYPE_CODE_UNION:
@@ -2096,7 +2129,8 @@ cplus_number_of_children (struct varobj
if (!CPLUS_FAKE_CHILD (var))
{
- type = get_type_deref (var);
+ type = get_value_type (var);
+ adjust_value_for_child_access (NULL, &type);
if (((TYPE_CODE (type)) == TYPE_CODE_STRUCT) ||
((TYPE_CODE (type)) == TYPE_CODE_UNION))
@@ -2122,7 +2156,8 @@ cplus_number_of_children (struct varobj
{
int kids[3];
- type = get_type_deref (var->parent);
+ type = get_value_type (var->parent);
+ adjust_value_for_child_access (NULL, &type);
cplus_class_num_children (type, kids);
if (strcmp (var->name, "public") == 0)
@@ -2173,25 +2208,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_value_type (parent->parent);
}
else
- type = get_type_deref (parent);
+ {
+ value = parent->value;
+ type = get_value_type (parent);
+ }
- name = NULL;
- switch (TYPE_CODE (type))
+ adjust_value_for_child_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
@@ -2201,56 +2267,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 = xstrdup (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 a baseclass. */
+ if (cname)
+ *cname = xstrdup (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"
@@ -2262,46 +2326,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 = xstrdup (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;
}
@@ -2314,118 +2381,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:3166
d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_2_children/gdb_mainline:3167
+d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_3_5_references/gdb_mainline:3174
+d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_3_unify/gdb_mainline:3168
d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_expression/gdb_mainline:2562
e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:160326
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Simpify varobj children handling for C++
2007-01-17 18:57 ` Vladimir Prus
@ 2007-01-20 19:42 ` Daniel Jacobowitz
2007-01-24 11:11 ` Vladimir Prus
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2007-01-20 19:42 UTC (permalink / raw)
To: Vladimir Prus; +Cc: gdb-patches
On Wed, Jan 17, 2007 at 09:55:53PM +0300, Vladimir Prus wrote:
> >> @@ -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 */
> >
> > Something tells me you didn't actually need value_ind to handle
> > references... :-)
>
> Why? Because that's a bad idea or because varobj code does not
> need to dereference rereferences? The latter is true,
> attached version of the patch builds on the
> "fix 'editable' attribute for references" patch I've posted earlier
> and now 'adjust_value_for_child_access' asserts that the type
> is not reference.
All I meant was that you added "is TYPE_CODE_PTR or is TYPE_CODE_PTR".
I guess you meant to check TYPE_CODE_REF, but you must not have needed
it if you didn't notice the typo.
> +get_value_type (struct varobj *var)
> {
> struct type *type;
>
> - type = get_type (var);
> + if (var->value)
> + type = value_type (var->value);
> + else
> + type = var->type;
>
> - if (type)
> - {
> - if (TYPE_CODE (type) == TYPE_CODE_REF)
> - type = get_target_type (type);
> - if (TYPE_CODE (type) == TYPE_CODE_PTR)
> - type = get_target_type (type);
> - }
> + if (TYPE_CODE (type) == TYPE_CODE_REF)
> + type = get_target_type (type);
> +
> + type = check_typedef (type);
>
> return type;
> }
I think that if you want to check for references, you need to call
check_typedef first (I just checked that g++ does allow typedefs
to reference types). Otherwise this looks OK to commit.
It would be nice to have some tests for not stripping typedefs,
if that's practical, but it's not really important.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Simpify varobj children handling for C++
2007-01-20 19:42 ` Daniel Jacobowitz
@ 2007-01-24 11:11 ` Vladimir Prus
0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Prus @ 2007-01-24 11:11 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]
Daniel Jacobowitz wrote:
> On Wed, Jan 17, 2007 at 09:55:53PM +0300, Vladimir Prus wrote:
>> >> @@ -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
>> >> */
>> >
>> > Something tells me you didn't actually need value_ind to handle
>> > references... :-)
>>
>> Why? Because that's a bad idea or because varobj code does not
>> need to dereference rereferences? The latter is true,
>> attached version of the patch builds on the
>> "fix 'editable' attribute for references" patch I've posted earlier
>> and now 'adjust_value_for_child_access' asserts that the type
>> is not reference.
>
> All I meant was that you added "is TYPE_CODE_PTR or is TYPE_CODE_PTR".
> I guess you meant to check TYPE_CODE_REF, but you must not have needed
> it if you didn't notice the typo.
Ah ;-)
>> +get_value_type (struct varobj *var)
>> {
>> struct type *type;
>>
>> - type = get_type (var);
>> + if (var->value)
>> + type = value_type (var->value);
>> + else
>> + type = var->type;
>>
>> - if (type)
>> - {
>> - if (TYPE_CODE (type) == TYPE_CODE_REF)
>> - type = get_target_type (type);
>> - if (TYPE_CODE (type) == TYPE_CODE_PTR)
>> - type = get_target_type (type);
>> - }
>> + if (TYPE_CODE (type) == TYPE_CODE_REF)
>> + type = get_target_type (type);
>> +
>> + type = check_typedef (type);
>>
>> return type;
>> }
>
> I think that if you want to check for references, you need to call
> check_typedef first (I just checked that g++ does allow typedefs
> to reference types). Otherwise this looks OK to commit.
The check_typedef added in "editable fix" patch, that's checked in.
The varobj children patch for C++ proper that I've just checked in as
well is attached.
>
> It would be nice to have some tests for not stripping typedefs,
> if that's practical, but it's not really important.
It would be good, but writing comprehensive tests is not a quick task.
It's on my todo, but definitely not today.
- Volodya
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: path_4_unify_cpp_as_committed.diff --]
[-- Type: text/x-diff; name="path_4_unify_cpp_as_committed.diff", Size: 16012 bytes --]
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.8135
diff -u -p -r1.8135 ChangeLog
--- ChangeLog 24 Jan 2007 10:49:27 -0000 1.8135
+++ ChangeLog 24 Jan 2007 11:08:10 -0000
@@ -1,5 +1,22 @@
2007-01-24 Vladimir Prus <vladimir@codesourcery.com>
+ Refactor getting children name, value and type access
+ for varobjs in C++.
+ * varobj.c (get_type_deref): Remove.
+ (adjust_value_for_child_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_child_access.
+
+2007-01-24 Vladimir Prus <vladimir@codesourcery.com>
+
Fix computation of the 'editable' attribute and
value changeability for for references.
* varobj.c (get_value_type): New function.
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.80
diff -u -p -r1.80 varobj.c
--- varobj.c 24 Jan 2007 10:49:29 -0000 1.80
+++ varobj.c 24 Jan 2007 11:08:10 -0000
@@ -178,8 +178,6 @@ static struct type *get_type (struct var
static struct type *get_value_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 *);
@@ -1492,26 +1490,6 @@ get_value_type (struct varobj *var)
return type;
}
-/* This returns the type of the variable, dereferencing references, pointers
- and references to pointers, too. */
-static struct type *
-get_type_deref (struct varobj *var)
-{
- struct type *type;
-
- type = get_type (var);
-
- if (type)
- {
- if (TYPE_CODE (type) == TYPE_CODE_REF)
- type = get_target_type (type);
- if (TYPE_CODE (type) == TYPE_CODE_PTR)
- type = get_target_type (type);
- }
-
- return type;
-}
-
/* This returns the target type (or NULL) of TYPE, also skipping
past typedefs, just like get_type ().
@@ -1773,17 +1751,61 @@ varobj_value_is_changeable_p (struct var
return r;
}
+/* Given the value and the type of a variable object,
+ adjust the value and type to those necessary
+ for getting children of the variable object.
+ This includes dereferencing top-level references
+ 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_child_access (struct value **value,
+ struct type **type)
+{
+ gdb_assert (type && *type);
+
+ *type = check_typedef (*type);
+
+ /* The type of value stored in varobj, that is passed
+ to us, is already supposed to be
+ reference-stripped. */
+
+ gdb_assert (TYPE_CODE (*type) != TYPE_CODE_REF);
+
+ /* 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;
+ }
+ }
+
+ /* The 'get_target_type' function calls check_typedef on
+ result, so we can immediately check type code. No
+ need to call check_typedef here. */
+}
+
/* C */
static int
c_number_of_children (struct varobj *var)
{
- struct type *type;
+ struct type *type = get_value_type (var);
+ int children = 0;
struct type *target;
- int children;
- type = get_type (var);
+ adjust_value_for_child_access (NULL, &type);
target = get_target_type (type);
- children = 0;
switch (TYPE_CODE (type))
{
@@ -1803,30 +1825,19 @@ c_number_of_children (struct varobj *var
break;
case TYPE_CODE_PTR:
- /* This is where things get complicated. 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:
@@ -1892,7 +1903,7 @@ c_describe_child (struct varobj *parent,
char **cname, struct value **cvalue, struct type **ctype)
{
struct value *value = parent->value;
- struct type *type = get_type (parent);
+ struct type *type = get_value_type (parent);
if (cname)
*cname = NULL;
@@ -1901,19 +1912,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_child_access (&value, &type);
switch (TYPE_CODE (type))
{
@@ -1961,8 +1960,11 @@ c_describe_child (struct varobj *parent,
if (cvalue && value)
gdb_value_ind (value, cvalue);
+ /* Don't use get_target_type because it calls
+ check_typedef and here, we want to show the true
+ declared type of the variable. */
if (ctype)
- *ctype = get_target_type (type);
+ *ctype = TYPE_TARGET_TYPE (type);
break;
@@ -2129,7 +2131,8 @@ cplus_number_of_children (struct varobj
if (!CPLUS_FAKE_CHILD (var))
{
- type = get_type_deref (var);
+ type = get_value_type (var);
+ adjust_value_for_child_access (NULL, &type);
if (((TYPE_CODE (type)) == TYPE_CODE_STRUCT) ||
((TYPE_CODE (type)) == TYPE_CODE_UNION))
@@ -2155,7 +2158,8 @@ cplus_number_of_children (struct varobj
{
int kids[3];
- type = get_type_deref (var->parent);
+ type = get_value_type (var->parent);
+ adjust_value_for_child_access (NULL, &type);
cplus_class_num_children (type, kids);
if (strcmp (var->name, "public") == 0)
@@ -2206,25 +2210,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_value_type (parent->parent);
}
else
- type = get_type_deref (parent);
+ {
+ value = parent->value;
+ type = get_value_type (parent);
+ }
- name = NULL;
- switch (TYPE_CODE (type))
+ adjust_value_for_child_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
@@ -2234,56 +2269,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 = xstrdup (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 a baseclass. */
+ if (cname)
+ *cname = xstrdup (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"
@@ -2295,46 +2328,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 = xstrdup (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;
}
@@ -2347,118 +2383,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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-01-24 11:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-23 21:51 Simpify varobj children handling for C++ Vladimir Prus
2006-12-25 8:16 ` Vladimir Prus
2007-01-03 22:32 ` Daniel Jacobowitz
2007-01-17 18:57 ` Vladimir Prus
2007-01-20 19:42 ` Daniel Jacobowitz
2007-01-24 11:11 ` Vladimir Prus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox