* [PATCH 08/12] Iterator varobj_items by their availability
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-24 20:28 ` Keith Seitz
2014-02-14 8:46 ` [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject Yao Qi
` (12 subsequent siblings)
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
This patch adds a new implementation of varobj iterator, which is
based on the availability of children.
V2:
- Fix changelog entry.
- Add comments.
- Use XNEW.
- Fix typo.
- Update copyright year for new added file.
gdb:
2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* Makefile.in (SFILES): Add varobj-iter-avail.c.
(COMMON_OBS): Add varobj-iter-avail.o.
* varobj-iter-avail.c: New file.
* varobj-iter.h (avail_varobj_get_iterator): Declare.
* varobj.c (varobj_get_iterator): Add one more argument
'lang_ops'. All callers updated.
Return iterator for available children.
---
gdb/Makefile.in | 4 +-
gdb/varobj-iter-avail.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++
gdb/varobj-iter.h | 4 +
gdb/varobj.c | 8 ++-
4 files changed, 174 insertions(+), 4 deletions(-)
create mode 100644 gdb/varobj-iter-avail.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 1da4055..88c8f68 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -831,7 +831,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
ui-out.c utils.c ui-file.h ui-file.c \
user-regs.c \
valarith.c valops.c valprint.c value.c varobj.c common/vec.c \
- xml-tdesc.c xml-support.c \
+ varobj-iter-avail.c xml-tdesc.c xml-support.c \
inferior.c gdb_usleep.c \
record.c record-full.c gcore.c \
jit.c \
@@ -995,7 +995,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
ada-lang.o c-lang.o d-lang.o f-lang.o objc-lang.o \
ada-tasks.o ada-varobj.o c-varobj.o \
ui-out.o cli-out.o \
- varobj.o vec.o \
+ varobj.o varobj-iter-avail.o vec.o \
go-lang.o go-valprint.o go-typeprint.o \
jv-lang.o jv-valprint.o jv-typeprint.o jv-varobj.o \
m2-lang.o opencl-lang.o p-lang.o p-typeprint.o p-valprint.o \
diff --git a/gdb/varobj-iter-avail.c b/gdb/varobj-iter-avail.c
new file mode 100644
index 0000000..59d526a
--- /dev/null
+++ b/gdb/varobj-iter-avail.c
@@ -0,0 +1,162 @@
+/* Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "gdb_assert.h"
+#include "value.h"
+#include "valprint.h"
+#include "varobj.h"
+#include "varobj-iter.h"
+
+/* A dynamic varobj iterator "class" for available-children-only
+ varobjs. This inherits struct varobj_iter. */
+
+struct avail_varobj_iter
+{
+ /* The 'base class'. */
+ struct varobj_iter base;
+
+ /* Varobj operations related to language. */
+ const struct lang_varobj_ops *lang_ops;
+};
+
+/* Returns true if VAL is not interesting for an
+ available-children-only varobj. */
+
+static int
+varobj_value_unavailable (struct value *val)
+{
+ volatile struct gdb_exception e;
+ int unavail = 0;
+
+ gdb_assert (val != NULL);
+
+ /* value_entirely_unavailable may need to try reading the value,
+ which may throw for example, a memory error, e.g., when not
+ inspecting a traceframe, we try to read the pointee of a dangling
+ or NULL pointer. */
+ TRY_CATCH (e, RETURN_MASK_ERROR)
+ {
+ struct type *type = value_type (val);
+
+ unavail = (value_entirely_unavailable (val)
+ /* A scalar object that does not have all bits available
+ is also considered unavailable, because all bits
+ contribute to its representation. */
+ || (val_print_scalar_type_p (type)
+ && !value_bytes_available (val,
+ value_embedded_offset (val),
+ TYPE_LENGTH (type))));
+ }
+ if (e.reason < 0)
+ return 1;
+ else
+ return unavail;
+}
+
+/* Implementation of the `next' method for available-children-only
+ varobj iterators. */
+
+static varobj_item *
+avail_varobj_iter_next (struct varobj_iter *self)
+{
+ struct avail_varobj_iter *dis = (struct avail_varobj_iter *) self;
+ int num_children = dis->lang_ops->number_of_children (self->var);
+ int raw_index = self->next_raw_index;
+ varobj_item *item = NULL;
+
+ for (; raw_index < num_children; raw_index++)
+ {
+ struct value *child_value
+ = dis->lang_ops->value_of_child (self->var, raw_index);
+
+ /* The "fake" children will have NULL values. */
+ if (child_value == NULL || !varobj_value_unavailable (child_value))
+ {
+ item = xmalloc (sizeof *item);
+
+ item->name
+ = dis->lang_ops->name_of_child (self->var, raw_index);
+ item->value = child_value;
+
+ raw_index++;
+ self->next_raw_index = raw_index;
+ break;
+ }
+ }
+ return item;
+}
+
+/* Implementation of the `dtor' method of available-children-only
+ varobj iterators. */
+
+static void
+avail_varobj_iter_dtor (struct varobj_iter *self)
+{
+ /* nothing to do */
+}
+
+/* The 'vtable' of available-children-only varobj iterators. */
+
+static const struct varobj_iter_ops avail_varobj_iter_ops =
+{
+ avail_varobj_iter_dtor,
+ avail_varobj_iter_next
+};
+
+/* Constructor of available-children-only varobj iterators. VAR is
+ the varobj whose children the iterator will be iterating over. */
+
+static void
+avail_varobj_iter_ctor (struct avail_varobj_iter *self, struct varobj *var,
+ const struct lang_varobj_ops *lang_ops)
+{
+ self->base.var = var;
+ self->base.ops = &avail_varobj_iter_ops;
+ self->base.next_raw_index = 0;
+ self->lang_ops = lang_ops;
+}
+
+/* Allocate and construct an available-children-only varobj iterator.
+ VAR is the varobj whose children the iterator will be iterating
+ over. */
+
+static struct avail_varobj_iter *
+avail_varobj_iter_new (struct varobj *var, const struct lang_varobj_ops *lang_ops)
+{
+ struct avail_varobj_iter *self;
+
+ self = XNEW (struct avail_varobj_iter);
+ avail_varobj_iter_ctor (self, var, lang_ops);
+ return self;
+}
+
+/* Return a new available-children-only varobj iterator suitable to
+ iterate over VAR's children. */
+
+struct varobj_iter *
+avail_varobj_get_iterator (struct varobj *var,
+ const struct lang_varobj_ops *lang_ops)
+{
+ struct avail_varobj_iter *iter;
+
+ /* Avoid the needless allocation/deallocation of the iterator if
+ there are no raw children to iterate over anyway. */
+ if (lang_ops->number_of_children (var) == 0)
+ return NULL;
+
+ iter = avail_varobj_iter_new (var, lang_ops);
+ return &iter->base;
+}
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
index 9eb672d..ef95a9b 100644
--- a/gdb/varobj-iter.h
+++ b/gdb/varobj-iter.h
@@ -70,3 +70,7 @@ struct varobj_iter_ops
xfree (ITER); \
} \
} while (0)
+
+struct varobj_iter *
+ avail_varobj_get_iterator (struct varobj *var,
+ const struct lang_varobj_ops *lang_ops);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 61309b2..bfa6b1d 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -768,8 +768,11 @@ dynamic_varobj_has_child_method (struct varobj *var)
iterator object suitable for iterating over VAR's children. */
static struct varobj_iter *
-varobj_get_iterator (struct varobj *var)
+varobj_get_iterator (struct varobj *var, const struct lang_varobj_ops *lang_ops)
{
+ if (var->dynamic->available_children_only)
+ return avail_varobj_get_iterator (var, lang_ops);
+
#if HAVE_PYTHON
if (var->dynamic->pretty_printer)
return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
@@ -810,7 +813,8 @@ update_dynamic_varobj_children (struct varobj *var,
if (update_children || var->dynamic->child_iter == NULL)
{
varobj_iter_delete (var->dynamic->child_iter);
- var->dynamic->child_iter = varobj_get_iterator (var);
+ var->dynamic->child_iter = varobj_get_iterator (var,
+ var->root->lang_ops);
varobj_clear_saved_item (var->dynamic);
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 08/12] Iterator varobj_items by their availability
2014-02-14 8:46 ` [PATCH 08/12] Iterator varobj_items by their availability Yao Qi
@ 2014-04-24 20:28 ` Keith Seitz
0 siblings, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-24 20:28 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> V2:
> - Fix changelog entry.
> - Add comments.
> - Use XNEW.
> - Fix typo.
> - Update copyright year for new added file.
>
> gdb:
>
> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * Makefile.in (SFILES): Add varobj-iter-avail.c.
> (COMMON_OBS): Add varobj-iter-avail.o.
> * varobj-iter-avail.c: New file.
> * varobj-iter.h (avail_varobj_get_iterator): Declare.
> * varobj.c (varobj_get_iterator): Add one more argument
> 'lang_ops'. All callers updated.
> Return iterator for available children.
This looks good to me, but I have one little thing I'd like to ask...
> +static int
> +varobj_value_unavailable (struct value *val)
> +{
[snip]
> +/* Implementation of the `next' method for available-children-only
> + varobj iterators. */
> +
> +static varobj_item *
> +avail_varobj_iter_next (struct varobj_iter *self)
> +{
> + struct avail_varobj_iter *dis = (struct avail_varobj_iter *) self;
> + int num_children = dis->lang_ops->number_of_children (self->var);
> + int raw_index = self->next_raw_index;
> + varobj_item *item = NULL;
> +
> + for (; raw_index < num_children; raw_index++)
> + {
> + struct value *child_value
> + = dis->lang_ops->value_of_child (self->var, raw_index);
> +
> + /* The "fake" children will have NULL values. */
> + if (child_value == NULL || !varobj_value_unavailable (child_value))
> + {
Maybe it's just me (and maybe because I am especially slow today), but I
find the use of double-negatives slow me/my comprehension down. The
above test reads, "if child_value is NULL or child_value is not
unavailable..."
What do you think about inverting the name of this function and the test
like so:
if (child_value == NULL || varobj_value_available (child_value))
/* ... */
Then that reads, "if child_value is NULL or the value is available,"
which is immensely easier for me to comprehend. [But maybe I'm the only
one?]
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
2014-02-14 8:46 ` [PATCH 08/12] Iterator varobj_items by their availability Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-23 19:29 ` Keith Seitz
2014-05-21 18:01 ` Tom Tromey
2014-02-14 8:46 ` [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair Yao Qi
` (11 subsequent siblings)
13 siblings, 2 replies; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
In previous patch, "saved_item" is still a PyOjbect and iteration is
still performed over PyObject. This patch continues to decouple
iteration from python code, so it changes its type to "struct
varobj_item *", so that the iterator itself is independent of python.
V2:
- Call varobj_delete_iter in free_variable.
- Fix changelog entries.
- Use XNEW.
gdb:
2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* python/py-varobj.c (py_varobj_iter_next): Move some code
from varobj.c.
* varobj-iter.h (struct varobj_item): Moved from varobj.c.
* varobj.c: Move "varobj-iter.h" inclusion earlier.
(struct varobj_item): Moved to varobj-iter.h".
(varobj_clear_saved_item): New function.
(update_dynamic_varobj_children): Move python-related code to
py-varobj.c.
(free_variable): Call varobj_clear_saved_item and
varobj_iter_delete.
---
gdb/python/py-varobj.c | 17 ++++++++++-
gdb/varobj-iter.h | 13 +++++++-
gdb/varobj.c | 76 ++++++++++++++++-------------------------------
3 files changed, 53 insertions(+), 53 deletions(-)
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index f4b95bd..a83f26a 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -51,6 +51,9 @@ py_varobj_iter_next (struct varobj_iter *self)
struct py_varobj_iter *t = (struct py_varobj_iter *) self;
struct cleanup *back_to;
PyObject *item;
+ PyObject *py_v;
+ varobj_item *vitem;
+ const char *name = NULL;
back_to = varobj_ensure_python_env (self->var);
@@ -98,9 +101,21 @@ py_varobj_iter_next (struct varobj_iter *self)
}
}
+ if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
+ {
+ gdbpy_print_stack ();
+ error (_("Invalid item from the child list"));
+ }
+
+ vitem = xmalloc (sizeof *vitem);
+ vitem->value = convert_value_from_python (py_v);
+ if (vitem->value == NULL)
+ gdbpy_print_stack ();
+ vitem->name = xstrdup (name);
+
self->next_raw_index++;
do_cleanups (back_to);
- return item;
+ return vitem;
}
/* The 'vtable' of pretty-printed python varobj iterators. */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
index 3a530bc..9eb672d 100644
--- a/gdb/varobj-iter.h
+++ b/gdb/varobj-iter.h
@@ -14,9 +14,18 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-struct varobj_iter_ops;
+/* A node or item of varobj, composed of the name and the value. */
+
+typedef struct varobj_item
+{
+ /* Name of this item. */
+ char *name;
-typedef PyObject varobj_item;
+ /* Value of this item. */
+ struct value *value;
+} varobj_item;
+
+struct varobj_iter_ops;
/* A dynamic varobj iterator "class". */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index d20c81e..33a2b0a 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -33,6 +33,7 @@
#include "vec.h"
#include "gdbthread.h"
#include "inferior.h"
+#include "varobj-iter.h"
#if HAVE_PYTHON
#include "python/python.h"
@@ -41,8 +42,6 @@
typedef int PyObject;
#endif
-#include "varobj-iter.h"
-
/* Non-zero if we want to see trace of varobj level stuff. */
unsigned int varobjdebug = 0;
@@ -110,17 +109,6 @@ struct varobj_root
struct varobj_root *next;
};
-/* A node or item of varobj, composed of the name and the value. */
-
-struct varobj_item
-{
- /* Name of this item. */
- char *name;
-
- /* Value of this item. */
- struct value *value;
-};
-
/* Dynamic part of varobj. */
struct varobj_dynamic
@@ -788,6 +776,18 @@ varobj_get_iterator (struct varobj *var)
requested an iterator from a non-dynamic varobj"));
}
+/* Release and clear VAR's saved item, if any. */
+
+static void
+varobj_clear_saved_item (struct varobj_dynamic *var)
+{
+ if (var->saved_item != NULL)
+ {
+ value_free (var->saved_item->value);
+ xfree (var->saved_item);
+ var->saved_item = NULL;
+ }
+}
#endif
static int
@@ -802,14 +802,8 @@ update_dynamic_varobj_children (struct varobj *var,
int to)
{
#if HAVE_PYTHON
- struct cleanup *back_to;
int i;
- if (!gdb_python_initialized)
- return 0;
-
- back_to = varobj_ensure_python_env (var);
-
*cchanged = 0;
if (update_children || var->dynamic->child_iter == NULL)
@@ -817,16 +811,12 @@ update_dynamic_varobj_children (struct varobj *var,
varobj_iter_delete (var->dynamic->child_iter);
var->dynamic->child_iter = varobj_get_iterator (var);
- Py_XDECREF (var->dynamic->saved_item);
- var->dynamic->saved_item = NULL;
+ varobj_clear_saved_item (var->dynamic);
i = 0;
if (var->dynamic->child_iter == NULL)
- {
- do_cleanups (back_to);
- return 0;
- }
+ return 0;
}
else
i = VEC_length (varobj_p, var->children);
@@ -835,10 +825,10 @@ update_dynamic_varobj_children (struct varobj *var,
are more children. */
for (; to < 0 || i < to + 1; ++i)
{
- PyObject *item;
+ varobj_item *item;
/* See if there was a leftover from last time. */
- if (var->dynamic->saved_item)
+ if (var->dynamic->saved_item != NULL)
{
item = var->dynamic->saved_item;
var->dynamic->saved_item = NULL;
@@ -846,6 +836,10 @@ update_dynamic_varobj_children (struct varobj *var,
else
{
item = varobj_iter_next (var->dynamic->child_iter);
+ /* Release vitem->value so its lifetime is not bound to the
+ execution of a command. */
+ if (item != NULL && item->value != NULL)
+ release_value_or_incref (item->value);
}
if (item == NULL)
@@ -858,36 +852,19 @@ update_dynamic_varobj_children (struct varobj *var,
/* We don't want to push the extra child on any report list. */
if (to < 0 || i < to)
{
- PyObject *py_v;
- const char *name;
- struct varobj_item varobj_item;
- struct cleanup *inner;
int can_mention = from < 0 || i >= from;
- inner = make_cleanup_py_decref (item);
-
- if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
- {
- gdbpy_print_stack ();
- error (_("Invalid item from the child list"));
- }
-
- varobj_item.value = convert_value_from_python (py_v);
- if (varobj_item.value == NULL)
- gdbpy_print_stack ();
- varobj_item.name = xstrdup (name);
-
install_dynamic_child (var, can_mention ? changed : NULL,
can_mention ? type_changed : NULL,
can_mention ? new : NULL,
can_mention ? unchanged : NULL,
can_mention ? cchanged : NULL, i,
- &varobj_item);
- do_cleanups (inner);
+ item);
+
+ xfree (item);
}
else
{
- Py_XDECREF (var->dynamic->saved_item);
var->dynamic->saved_item = item;
/* We want to truncate the child list just before this
@@ -913,7 +890,6 @@ update_dynamic_varobj_children (struct varobj *var,
var->num_children = VEC_length (varobj_p, var->children);
- do_cleanups (back_to);
return 1;
#else
gdb_assert_not_reached ("should never be called if Python is not enabled");
@@ -2182,12 +2158,12 @@ free_variable (struct varobj *var)
Py_XDECREF (var->dynamic->constructor);
Py_XDECREF (var->dynamic->pretty_printer);
- Py_XDECREF (var->dynamic->child_iter);
- Py_XDECREF (var->dynamic->saved_item);
do_cleanups (cleanup);
}
#endif
+ varobj_iter_delete (var->dynamic->child_iter);
+ varobj_clear_saved_item (var->dynamic);
value_free (var->value);
/* Free the expression if this is a root variable. */
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject
2014-02-14 8:46 ` [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject Yao Qi
@ 2014-04-23 19:29 ` Keith Seitz
2014-05-21 18:01 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-23 19:29 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> V2:
> - Call varobj_delete_iter in free_variable.
> - Fix changelog entries.
> - Use XNEW.
>
> gdb:
>
> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * python/py-varobj.c (py_varobj_iter_next): Move some code
> from varobj.c.
> * varobj-iter.h (struct varobj_item): Moved from varobj.c.
> * varobj.c: Move "varobj-iter.h" inclusion earlier.
> (struct varobj_item): Moved to varobj-iter.h".
> (varobj_clear_saved_item): New function.
> (update_dynamic_varobj_children): Move python-related code to
> py-varobj.c.
> (free_variable): Call varobj_clear_saved_item and
> varobj_iter_delete.
This looks okay to me. I recommend a maintainer give this a final review.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject
2014-02-14 8:46 ` [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject Yao Qi
2014-04-23 19:29 ` Keith Seitz
@ 2014-05-21 18:01 ` Tom Tromey
2014-05-23 1:33 ` Yao Qi
1 sibling, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2014-05-21 18:01 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao> Yao Qi <yao@codesourcery.com>
Yao> * python/py-varobj.c (py_varobj_iter_next): Move some code
Yao> from varobj.c.
Yao> * varobj-iter.h (struct varobj_item): Moved from varobj.c.
Yao> * varobj.c: Move "varobj-iter.h" inclusion earlier.
Yao> (struct varobj_item): Moved to varobj-iter.h".
Yao> (varobj_clear_saved_item): New function.
Yao> (update_dynamic_varobj_children): Move python-related code to
Yao> py-varobj.c.
Yao> (free_variable): Call varobj_clear_saved_item and
Yao> varobj_iter_delete.
This looks reasonable but I have a question first:
Yao> @@ -802,14 +802,8 @@ update_dynamic_varobj_children (struct varobj *var,
Yao> int to)
Yao> {
Yao> #if HAVE_PYTHON
Yao> - struct cleanup *back_to;
Yao> int i;
Yao> - if (!gdb_python_initialized)
Yao> - return 0;
Where is this check done now? I couldn't find the location.
IIRC this check was added in response to some bug report; I think it
only arises if Python can't be properly initialized somehow.
If it was dropped I think it may need to be reinstated somewhere.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject
2014-05-21 18:01 ` Tom Tromey
@ 2014-05-23 1:33 ` Yao Qi
2014-06-04 20:22 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-05-23 1:33 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 05/22/2014 02:01 AM, Tom Tromey wrote:
> Yao> - if (!gdb_python_initialized)
> Yao> - return 0;
>
> Where is this check done now? I couldn't find the location.
>
Good catch.
> IIRC this check was added in response to some bug report; I think it
> only arises if Python can't be properly initialized somehow.
>
> If it was dropped I think it may need to be reinstated somewhere.
It should be moved to 'next' method of python varobj iterator,
because update_dynamic_varobj_children is independent of python,
and all python related code should be moved to python varobj
iterator.
At the beginning of update_dynamic_varobj_children,
if (update_children || var->dynamic->child_iter == NULL)
{
varobj_iter_delete (var->dynamic->child_iter);
var->dynamic->child_iter = varobj_get_iterator (var,
var->root->lang_ops);
varobj_clear_saved_item (var->dynamic);
i = 0;
if (var->dynamic->child_iter == NULL)
return 0;
}
else
i = VEC_length (varobj_p, var->children);
if gdb_python_initialized is false, varobj_get_iterator returns
NULL and update_dynamic_varobj_children returns zero too. The
behaviour is unchanged.
--
Yao (é½å°§)
In previous patch, "saved_item" is still a PyOjbect and iteration is
still performed over PyObject. This patch continues to decouple
iteration from python code, so it changes its type to "struct
varobj_item *", so that the iterator itself is independent of python.
V2:
- Call varobj_delete_iter in free_variable.
- Fix changelog entries.
- Use XNEW.
V3:
- Return NULL early in py_varobj_iter_next if gdb_python_initialized
is false.
gdb:
2014-05-22 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* python/py-varobj.c (py_varobj_iter_next): Return NULL if
gdb_python_initialized is false. Move some code from varobj.c.
* varobj-iter.h (struct varobj_item): Moved from varobj.c.
* varobj.c: Move "varobj-iter.h" inclusion earlier.
(struct varobj_item): Moved to varobj-iter.h".
(varobj_clear_saved_item): New function.
(update_dynamic_varobj_children): Move python-related code to
py-varobj.c.
(free_variable): Call varobj_clear_saved_item and
varobj_iter_delete.
---
gdb/python/py-varobj.c | 20 ++++++++++++-
gdb/varobj-iter.h | 13 +++++++--
gdb/varobj.c | 76 +++++++++++++++++---------------------------------
3 files changed, 56 insertions(+), 53 deletions(-)
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index 50bea40..40b28f4 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -53,6 +53,12 @@ py_varobj_iter_next (struct varobj_iter *self)
struct py_varobj_iter *t = (struct py_varobj_iter *) self;
struct cleanup *back_to;
PyObject *item;
+ PyObject *py_v;
+ varobj_item *vitem;
+ const char *name = NULL;
+
+ if (!gdb_python_initialized)
+ return NULL;
back_to = varobj_ensure_python_env (self->var);
@@ -100,9 +106,21 @@ py_varobj_iter_next (struct varobj_iter *self)
}
}
+ if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
+ {
+ gdbpy_print_stack ();
+ error (_("Invalid item from the child list"));
+ }
+
+ vitem = xmalloc (sizeof *vitem);
+ vitem->value = convert_value_from_python (py_v);
+ if (vitem->value == NULL)
+ gdbpy_print_stack ();
+ vitem->name = xstrdup (name);
+
self->next_raw_index++;
do_cleanups (back_to);
- return item;
+ return vitem;
}
/* The 'vtable' of pretty-printed python varobj iterators. */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
index 3a530bc..9eb672d 100644
--- a/gdb/varobj-iter.h
+++ b/gdb/varobj-iter.h
@@ -14,9 +14,18 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-struct varobj_iter_ops;
+/* A node or item of varobj, composed of the name and the value. */
+
+typedef struct varobj_item
+{
+ /* Name of this item. */
+ char *name;
-typedef PyObject varobj_item;
+ /* Value of this item. */
+ struct value *value;
+} varobj_item;
+
+struct varobj_iter_ops;
/* A dynamic varobj iterator "class". */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 02cce5a..7e8b364 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -33,6 +33,7 @@
#include "vec.h"
#include "gdbthread.h"
#include "inferior.h"
+#include "varobj-iter.h"
#if HAVE_PYTHON
#include "python/python.h"
@@ -41,8 +42,6 @@
typedef int PyObject;
#endif
-#include "varobj-iter.h"
-
/* Non-zero if we want to see trace of varobj level stuff. */
unsigned int varobjdebug = 0;
@@ -110,17 +109,6 @@ struct varobj_root
struct varobj_root *next;
};
-/* A node or item of varobj, composed of the name and the value. */
-
-struct varobj_item
-{
- /* Name of this item. */
- char *name;
-
- /* Value of this item. */
- struct value *value;
-};
-
/* Dynamic part of varobj. */
struct varobj_dynamic
@@ -788,6 +776,18 @@ varobj_get_iterator (struct varobj *var)
requested an iterator from a non-dynamic varobj"));
}
+/* Release and clear VAR's saved item, if any. */
+
+static void
+varobj_clear_saved_item (struct varobj_dynamic *var)
+{
+ if (var->saved_item != NULL)
+ {
+ value_free (var->saved_item->value);
+ xfree (var->saved_item);
+ var->saved_item = NULL;
+ }
+}
#endif
static int
@@ -802,14 +802,8 @@ update_dynamic_varobj_children (struct varobj *var,
int to)
{
#if HAVE_PYTHON
- struct cleanup *back_to;
int i;
- if (!gdb_python_initialized)
- return 0;
-
- back_to = varobj_ensure_python_env (var);
-
*cchanged = 0;
if (update_children || var->dynamic->child_iter == NULL)
@@ -817,16 +811,12 @@ update_dynamic_varobj_children (struct varobj *var,
varobj_iter_delete (var->dynamic->child_iter);
var->dynamic->child_iter = varobj_get_iterator (var);
- Py_XDECREF (var->dynamic->saved_item);
- var->dynamic->saved_item = NULL;
+ varobj_clear_saved_item (var->dynamic);
i = 0;
if (var->dynamic->child_iter == NULL)
- {
- do_cleanups (back_to);
- return 0;
- }
+ return 0;
}
else
i = VEC_length (varobj_p, var->children);
@@ -835,10 +825,10 @@ update_dynamic_varobj_children (struct varobj *var,
are more children. */
for (; to < 0 || i < to + 1; ++i)
{
- PyObject *item;
+ varobj_item *item;
/* See if there was a leftover from last time. */
- if (var->dynamic->saved_item)
+ if (var->dynamic->saved_item != NULL)
{
item = var->dynamic->saved_item;
var->dynamic->saved_item = NULL;
@@ -846,6 +836,10 @@ update_dynamic_varobj_children (struct varobj *var,
else
{
item = varobj_iter_next (var->dynamic->child_iter);
+ /* Release vitem->value so its lifetime is not bound to the
+ execution of a command. */
+ if (item != NULL && item->value != NULL)
+ release_value_or_incref (item->value);
}
if (item == NULL)
@@ -858,36 +852,19 @@ update_dynamic_varobj_children (struct varobj *var,
/* We don't want to push the extra child on any report list. */
if (to < 0 || i < to)
{
- PyObject *py_v;
- const char *name;
- struct varobj_item varobj_item;
- struct cleanup *inner;
int can_mention = from < 0 || i >= from;
- inner = make_cleanup_py_decref (item);
-
- if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
- {
- gdbpy_print_stack ();
- error (_("Invalid item from the child list"));
- }
-
- varobj_item.value = convert_value_from_python (py_v);
- if (varobj_item.value == NULL)
- gdbpy_print_stack ();
- varobj_item.name = xstrdup (name);
-
install_dynamic_child (var, can_mention ? changed : NULL,
can_mention ? type_changed : NULL,
can_mention ? new : NULL,
can_mention ? unchanged : NULL,
can_mention ? cchanged : NULL, i,
- &varobj_item);
- do_cleanups (inner);
+ item);
+
+ xfree (item);
}
else
{
- Py_XDECREF (var->dynamic->saved_item);
var->dynamic->saved_item = item;
/* We want to truncate the child list just before this
@@ -913,7 +890,6 @@ update_dynamic_varobj_children (struct varobj *var,
var->num_children = VEC_length (varobj_p, var->children);
- do_cleanups (back_to);
return 1;
#else
gdb_assert_not_reached ("should never be called if Python is not enabled");
@@ -2191,12 +2167,12 @@ free_variable (struct varobj *var)
Py_XDECREF (var->dynamic->constructor);
Py_XDECREF (var->dynamic->pretty_printer);
- Py_XDECREF (var->dynamic->child_iter);
- Py_XDECREF (var->dynamic->saved_item);
do_cleanups (cleanup);
}
#endif
+ varobj_iter_delete (var->dynamic->child_iter);
+ varobj_clear_saved_item (var->dynamic);
value_free (var->value);
/* Free the expression if this is a root variable. */
--
1.9.0
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject
2014-05-23 1:33 ` Yao Qi
@ 2014-06-04 20:22 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2014-06-04 20:22 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> 2014-05-22 Pedro Alves <pedro@codesourcery.com>
Yao> Yao Qi <yao@codesourcery.com>
Yao> * python/py-varobj.c (py_varobj_iter_next): Return NULL if
Yao> gdb_python_initialized is false. Move some code from varobj.c.
Yao> * varobj-iter.h (struct varobj_item): Moved from varobj.c.
Yao> * varobj.c: Move "varobj-iter.h" inclusion earlier.
Yao> (struct varobj_item): Moved to varobj-iter.h".
Yao> (varobj_clear_saved_item): New function.
Yao> (update_dynamic_varobj_children): Move python-related code to
Yao> py-varobj.c.
Yao> (free_variable): Call varobj_clear_saved_item and
Yao> varobj_iter_delete.
Thanks, this is ok.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
2014-02-14 8:46 ` [PATCH 08/12] Iterator varobj_items by their availability Yao Qi
2014-02-14 8:46 ` [PATCH 03/12] Iterate over 'struct varobj_item' instead of PyObject Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-23 18:16 ` Keith Seitz
2014-05-21 17:53 ` Tom Tromey
2014-02-14 8:46 ` [PATCH 10/12] Match dynamic="1" in the output of -var-list-children Yao Qi
` (10 subsequent siblings)
13 siblings, 2 replies; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
Hi,
name and value pair is widely used in varobj.c. This patch is to add
a new struct varobj_item to represent them, so that the number of
function arguments can be reduced. Finally, the iteration is done on
'struct varobj_item' instead of PyObject after this patch series.
V2:
- Fix changelog entry.
- Fix one grammatical mistake.
gdb:
2014-02-14 Yao Qi <yao@codesourcery.com>
* varobj.c (struct varobj_item): New structure.
(create_child_with_value): Update declaration.
(varobj_add_child): Replace arguments 'name' and 'value' with
'item'. All callers updated.
(install_dynamic_child): Likewise.
(update_dynamic_varobj_children): Likewise.
(varobj_add_child): Likewise.
(create_child_with_value): Likewise.
---
gdb/varobj.c | 65 +++++++++++++++++++++++++++++++++++----------------------
1 files changed, 40 insertions(+), 25 deletions(-)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index bbe4213..68c54e3 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -108,6 +108,17 @@ struct varobj_root
struct varobj_root *next;
};
+/* A node or item of varobj, composed of the name and the value. */
+
+struct varobj_item
+{
+ /* Name of this item. */
+ char *name;
+
+ /* Value of this item. */
+ struct value *value;
+};
+
/* Dynamic part of varobj. */
struct varobj_dynamic
@@ -169,8 +180,8 @@ static void uninstall_variable (struct varobj *);
static struct varobj *create_child (struct varobj *, int, char *);
static struct varobj *
-create_child_with_value (struct varobj *parent, int index, char *name,
- struct value *value);
+create_child_with_value (struct varobj *parent, int index,
+ struct varobj_item *item);
/* Utility routines */
@@ -214,8 +225,7 @@ static int is_root_p (struct varobj *var);
#if HAVE_PYTHON
static struct varobj *varobj_add_child (struct varobj *var,
- char *name,
- struct value *value);
+ struct varobj_item *item);
#endif /* HAVE_PYTHON */
@@ -714,13 +724,12 @@ install_dynamic_child (struct varobj *var,
VEC (varobj_p) **unchanged,
int *cchanged,
int index,
- char *name,
- struct value *value)
+ struct varobj_item *item)
{
if (VEC_length (varobj_p, var->children) < index + 1)
{
/* There's no child yet. */
- struct varobj *child = varobj_add_child (var, name, value);
+ struct varobj *child = varobj_add_child (var, item);
if (new)
{
@@ -731,14 +740,14 @@ install_dynamic_child (struct varobj *var,
else
{
varobj_p existing = VEC_index (varobj_p, var->children, index);
- int type_updated = update_type_if_necessary (existing, value);
+ int type_updated = update_type_if_necessary (existing, item->value);
if (type_updated)
{
if (type_changed)
VEC_safe_push (varobj_p, *type_changed, existing);
}
- if (install_new_value (existing, value, 0))
+ if (install_new_value (existing, item->value, 0))
{
if (!type_updated && changed)
VEC_safe_push (varobj_p, *changed, existing);
@@ -889,7 +898,7 @@ update_dynamic_varobj_children (struct varobj *var,
{
PyObject *py_v;
const char *name;
- struct value *v;
+ struct varobj_item varobj_item;
struct cleanup *inner;
int can_mention = from < 0 || i >= from;
@@ -901,15 +910,17 @@ update_dynamic_varobj_children (struct varobj *var,
error (_("Invalid item from the child list"));
}
- v = convert_value_from_python (py_v);
- if (v == NULL)
+ varobj_item.value = convert_value_from_python (py_v);
+ if (varobj_item.value == NULL)
gdbpy_print_stack ();
+ varobj_item.name = xstrdup (name);
+
install_dynamic_child (var, can_mention ? changed : NULL,
can_mention ? type_changed : NULL,
can_mention ? new : NULL,
can_mention ? unchanged : NULL,
can_mention ? cchanged : NULL, i,
- xstrdup (name), v);
+ &varobj_item);
do_cleanups (inner);
}
else
@@ -1028,11 +1039,11 @@ varobj_list_children (struct varobj *var, int *from, int *to)
#if HAVE_PYTHON
static struct varobj *
-varobj_add_child (struct varobj *var, char *name, struct value *value)
+varobj_add_child (struct varobj *var, struct varobj_item *item)
{
- varobj_p v = create_child_with_value (var,
+ varobj_p v = create_child_with_value (var,
VEC_length (varobj_p, var->children),
- name, value);
+ item);
VEC_safe_push (varobj_p, var->children, v);
return v;
@@ -2098,13 +2109,17 @@ uninstall_variable (struct varobj *var)
static struct varobj *
create_child (struct varobj *parent, int index, char *name)
{
- return create_child_with_value (parent, index, name,
- value_of_child (parent, index));
+ struct varobj_item item;
+
+ item.name = name;
+ item.value = value_of_child (parent, index);
+
+ return create_child_with_value (parent, index, &item);
}
static struct varobj *
-create_child_with_value (struct varobj *parent, int index, char *name,
- struct value *value)
+create_child_with_value (struct varobj *parent, int index,
+ struct varobj_item *item)
{
struct varobj *child;
char *childs_name;
@@ -2112,7 +2127,7 @@ create_child_with_value (struct varobj *parent, int index, char *name,
child = new_variable ();
/* NAME is allocated by caller. */
- child->name = name;
+ child->name = item->name;
child->index = index;
child->parent = parent;
child->root = parent->root;
@@ -2120,22 +2135,22 @@ create_child_with_value (struct varobj *parent, int index, char *name,
if (varobj_is_anonymous_child (child))
childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index);
else
- childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
+ childs_name = xstrprintf ("%s.%s", parent->obj_name, item->name);
child->obj_name = childs_name;
install_variable (child);
/* Compute the type of the child. Must do this before
calling install_new_value. */
- if (value != NULL)
+ if (item->value != NULL)
/* If the child had no evaluation errors, var->value
will be non-NULL and contain a valid type. */
- child->type = value_actual_type (value, 0, NULL);
+ child->type = value_actual_type (item->value, 0, NULL);
else
/* Otherwise, we must compute the type. */
child->type = (*child->root->lang_ops->type_of_child) (child->parent,
child->index);
- install_new_value (child, value, 1);
+ install_new_value (child, item->value, 1);
return child;
}
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair
2014-02-14 8:46 ` [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair Yao Qi
@ 2014-04-23 18:16 ` Keith Seitz
2014-05-21 17:53 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-23 18:16 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> V2:
> - Fix changelog entry.
> - Fix one grammatical mistake.
>
> gdb:
>
> 2014-02-14 Yao Qi <yao@codesourcery.com>
>
> * varobj.c (struct varobj_item): New structure.
> (create_child_with_value): Update declaration.
> (varobj_add_child): Replace arguments 'name' and 'value' with
> 'item'. All callers updated.
> (install_dynamic_child): Likewise.
> (update_dynamic_varobj_children): Likewise.
> (varobj_add_child): Likewise.
> (create_child_with_value): Likewise.
This looks good to me. I recommend a maintainer look this over for final
review/approval.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair
2014-02-14 8:46 ` [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair Yao Qi
2014-04-23 18:16 ` Keith Seitz
@ 2014-05-21 17:53 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2014-05-21 17:53 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> 2014-02-14 Yao Qi <yao@codesourcery.com>
Yao> * varobj.c (struct varobj_item): New structure.
Yao> (create_child_with_value): Update declaration.
Yao> (varobj_add_child): Replace arguments 'name' and 'value' with
Yao> 'item'. All callers updated.
Yao> (install_dynamic_child): Likewise.
Yao> (update_dynamic_varobj_children): Likewise.
Yao> (varobj_add_child): Likewise.
Yao> (create_child_with_value): Likewise.
This is ok.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 10/12] Match dynamic="1" in the output of -var-list-children
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (2 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 01/12] Use 'struct varobj_item' to represent name and value pair Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-24 20:50 ` Keith Seitz
2014-02-14 8:46 ` [PATCH 11/12] Test case Yao Qi
` (9 subsequent siblings)
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
When I play with pretty-printer and available-children-only, I get the
following output,
-var-list-children ss1
^done,numchild="2",children=[child={name="ss1.a",exp="a",numchild="0",type="struct s",thread-id="1",dynamic="1"},child={name="ss1.b",exp="b",numchild="0",type="struct s",thread-id="1",dynamic="1"}],has_more="0"
existing proc mi_child_regexp doesn't append "dynamic=1" to the pattern,
so it doesn't match output. This patch adds "dynamic=1" to the pattern.
I am not satisfied with the regexp construction for each child in
mi_child_regexp. In each list, there are three mandatory fields,
"name", "exp", and "numchild". There are also some optional fields,
"value", "type" and "dynamic". The current regexp construction code
is hard to be extended (add for "dynamic"). I suggest that we can
pass the list to mi_list_varobj_children like this,
{ name exp numchild optional }
the list has four elements, and OPTIONAL is an array, which index can
be "value", "type" or "dynamic". The existing usage of
mi_list_varobj_children like:
mi_list_varobj_children "struct_declarations" {
{struct_declarations.integer integer 0 int}
} "test"
will be rewritten to:
mi_list_varobj_children "struct_declarations" {
{struct_declarations.integer integer 0 {type int}}
} "test"
if we want to match "dynamic" attribute, we can write:
mi_list_varobj_children "struct_declarations" {
{struct_declarations.integer integer 0 {type int dynamic 1}}
} "test"
Since mi_list_varobj_children has been widely used in test suite, I'd
like to defer the change after this patch series.
V2:
- Remove empty 'else' block.
gdb/testsuite:
2014-02-14 Yao Qi <yao@codesourcery.com>
* lib/mi-support.exp (mi_child_regexp): Append 'dynamic="1"' to
children_exp.
---
gdb/testsuite/lib/mi-support.exp | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 1e8fee6..df68bd2 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1410,21 +1410,24 @@ proc mi_child_regexp {children add_child} {
set name [lindex $item 0]
set exp [lindex $item 1]
set numchild [lindex $item 2]
+
+ set line "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\""
+
if {[llength $item] == 5} {
set type [lindex $item 3]
set value [lindex $item 4]
- lappend children_exp\
- "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",value=\"$value\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
+ append line ",value=\"$value\",type=\"$type\""
} elseif {[llength $item] == 4} {
set type [lindex $item 3]
- lappend children_exp\
- "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\",type=\"$type\"(,thread-id=\"\[0-9\]+\")?}"
- } else {
- lappend children_exp\
- "$pre{name=\"$name\",exp=\"$exp\",numchild=\"$numchild\"(,thread-id=\"\[0-9\]+\")?}"
+ append line ",type=\"$type\""
}
+
+ append line \
+ "(,thread-id=\"\[0-9\]+\")?(,dynamic=\"1\")?}"
+
+ lappend children_exp $line
}
return [join $children_exp ","]
}
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 10/12] Match dynamic="1" in the output of -var-list-children
2014-02-14 8:46 ` [PATCH 10/12] Match dynamic="1" in the output of -var-list-children Yao Qi
@ 2014-04-24 20:50 ` Keith Seitz
0 siblings, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-24 20:50 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> I am not satisfied with the regexp construction for each child in
> mi_child_regexp.
Welcome, like-minded contributor! :-)
> will be rewritten to:
>
> mi_list_varobj_children "struct_declarations" {
> {struct_declarations.integer integer 0 {type int}}
> } "test"
>
> if we want to match "dynamic" attribute, we can write:
>
> mi_list_varobj_children "struct_declarations" {
> {struct_declarations.integer integer 0 {type int dynamic 1}}
> } "test"
>
> Since mi_list_varobj_children has been widely used in test suite, I'd
> like to defer the change after this patch series.
There are a number of different strategies for testing varobj in the
test suite. Not too long ago, I added another which uses "trees" not
altogether unlike what you are proposing.
Have you looked into varobj_tree stuff in the test suite? Does it fit
your needs? If so, it might be better to use this facility and (start
to) deprecate the mi_list_varobj_children interface.
>
> V2:
> - Remove empty 'else' block.
>
> gdb/testsuite:
>
> 2014-02-14 Yao Qi <yao@codesourcery.com>
>
> * lib/mi-support.exp (mi_child_regexp): Append 'dynamic="1"' to
> children_exp.
This looks okay to me; I recommend a maintainer approve this.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 11/12] Test case
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (3 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 10/12] Match dynamic="1" in the output of -var-list-children Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-18 11:45 ` Yao Qi
2014-02-14 8:46 ` [PATCH 09/12] Delete varobj's children on traceframe is changed Yao Qi
` (8 subsequent siblings)
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
This patch adds two test cases for the new option.
mi-available-children-only-cxx.exp focuses on the C++ fake children.
V2:
- Wrap condition with {}.
- Remove hard-coded breakpoint number, but it depends on this patch
[PATCH] Accept convenience variable in commands -break-passcount and -break-commands
https://sourceware.org/ml/gdb-patches/2014-01/msg00936.html
gdb/testsuite:
2014-02-14 Yao Qi <yao@codesourcery.com>
* gdb.trace/available-children-only.c: New.
* gdb.trace/mi-available-children-only.exp: New.
* gdb.trace/mi-available-children-only-cxx.exp: New.
* gdb.trace/available-children-only.cc: New.
gdb:
2014-02-14 Yao Qi <yao@codesourcery.com>
* config/djgpp/fnchange.lst: Add mapping for
available-children-only.c, mi-available-children-only.exp,
mi-available-children-only-cxx.exp and
mi-available-children-only.cc.
---
gdb/config/djgpp/fnchange.lst | 4 +
gdb/testsuite/gdb.trace/available-children-only.c | 69 +++++++
gdb/testsuite/gdb.trace/available-children-only.cc | 45 +++++
.../gdb.trace/mi-available-children-only-cxx.exp | 126 +++++++++++++
.../gdb.trace/mi-available-children-only.exp | 198 ++++++++++++++++++++
5 files changed, 442 insertions(+), 0 deletions(-)
create mode 100644 gdb/testsuite/gdb.trace/available-children-only.c
create mode 100644 gdb/testsuite/gdb.trace/available-children-only.cc
create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only.exp
diff --git a/gdb/config/djgpp/fnchange.lst b/gdb/config/djgpp/fnchange.lst
index cbf11c6..b94c77e 100644
--- a/gdb/config/djgpp/fnchange.lst
+++ b/gdb/config/djgpp/fnchange.lst
@@ -506,6 +506,10 @@
@V@/gdb/testsuite/gdb.mi/mi2-var-display.exp @V@/gdb/testsuite/gdb.mi/mi2vardisplay.exp
@V@/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp @V@/gdb/testsuite/gdb.mi/minonstop-exit.exp
@V@/gdb/testsuite/gdb.mi/non-stop-exit.c @V@/gdb/testsuite/gdb.mi/nonstop-exit.c
+@V@/gdb/testsuite/gdb.trace/available-children-only.c @V@/gdb/testsuite/gdb.trace/availnly.c
+@V@/gdb/testsuite/gdb.trace/mi-available-children-only.exp @V@/gdb/testsuite/gdb.trace/availnly.exp
+@V@/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp @V@/gdb/testsuite/gdb.trace/availcxx.exp
+@V@/gdb/testsuite/gdb.trace/available-children-only.cc @V@/gdb/testsuite/gdb.trace/availcxx.cc
@V@/gdb/testsuite/gdb.threads/watchthreads2.c @V@/gdb/testsuite/gdb.threads/watchth2.c
@V@/gdb/testsuite/gdb.threads/watchthreads2.exp @V@/gdb/testsuite/gdb.threads/watchth2.exp
@V@/gdb/amd64-linux-tdep.c @V@/gdb/amd64-ltdep.c
diff --git a/gdb/testsuite/gdb.trace/available-children-only.c b/gdb/testsuite/gdb.trace/available-children-only.c
new file mode 100644
index 0000000..b13f130
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/available-children-only.c
@@ -0,0 +1,69 @@
+/* Copyright 2013-2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Two trace actions are set, and they collect different fields to
+ reproduce the case that children appear in the different orders. */
+
+struct simple
+{
+ int a; /* Collected by both. */
+ int b; /* Collected by action 2. */
+ struct
+ {
+ struct
+ {
+ int g; /* Collected by action 1. */
+ int h; /* Collected by action 2. */
+ } s3;
+ int d; /* Collected by action 1. */
+ } s1;
+
+ struct
+ {
+ int e;
+ int f; /* Collected by action 1. */
+ } s2;
+};
+
+struct simple s;
+
+static void
+marker1 (void)
+{
+}
+
+static void
+marker2 (void)
+{
+}
+
+static void
+end (void)
+{
+}
+
+int
+main (void)
+{
+ marker1 ();
+
+ marker2 ();
+
+ end ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/available-children-only.cc b/gdb/testsuite/gdb.trace/available-children-only.cc
new file mode 100644
index 0000000..f9f06e7
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/available-children-only.cc
@@ -0,0 +1,45 @@
+/* Copyright 2013-2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+class Fake
+{
+ private:
+ int pri1; /* Collected. */
+
+ public:
+ int pub1;
+
+};
+
+Fake fake;
+
+static void
+marker (void)
+{}
+
+static void
+end (void)
+{}
+
+int
+main (void)
+{
+ marker ();
+
+ end ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
new file mode 100644
index 0000000..bec692c
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
@@ -0,0 +1,126 @@
+# Copyright 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test for C++ fake children.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile available-children-only.cc
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+ executable {debug c++}] != "" } {
+ untested "Couldn't compile ${srcfile}"
+ return -1
+}
+
+# Test target supports tracepoints or not.
+
+clean_restart $testfile
+
+if { ![runto_main] } {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+if { ![gdb_target_supports_trace] } {
+ unsupported "Current target does not support trace"
+ return -1
+}
+gdb_exit
+
+if { [mi_gdb_start] } {
+ continue
+}
+
+mi_run_to_main
+
+mi_gdb_test "-break-insert -a marker" "\\^done.*" \
+ "trace marker"
+
+mi_gdb_test "-break-commands \$bpnum \"collect fake.pri1\" \"end\" " \
+ {\^done} "set action on marker"
+
+mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
+mi_continue_to end
+mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
+# Save trace frames to trace file.
+set tracefile [standard_output_file ${testfile}]
+mi_gdb_test "-trace-save ${tracefile}.tfile" \
+ ".*" \
+ "save tfile trace"
+
+# In live target, '--available-children-only' shouldn't have any
+# effects.
+
+mi_gdb_test "-trace-find frame-number 0" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+ "-trace-find frame-number 0"
+
+mi_gdb_test "-var-create --available-children-only fake @ fake" \
+ {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only fake"
+
+mi_list_varobj_children { fake --available-children-only } {
+ { fake.public public 0 }
+ { fake.private private 0 }
+} "-var-list-children --available-children-only fake"
+
+mi_gdb_test "-var-info-num-children --available-children-only fake" \
+ "\\^done,numchild=\"2\"" \
+ "-var-info-num-children --available-children-only fake"
+
+mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
+
+# Select a traceframe, and '--available-children-only' have some
+# effects.
+
+proc check_with_traceframe { } {
+ global decimal
+
+ with_test_prefix "w/ setting traceframe" {
+ mi_gdb_test "-trace-find frame-number 0" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+ "-trace-find frame-number 0"
+
+ mi_gdb_test "-var-create --available-children-only fake @ fake" \
+ {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only fake"
+
+ mi_list_varobj_children "fake" {
+ { fake.public public 1 }
+ { fake.private private 1 }
+ } "-var-list-children fake"
+
+ mi_gdb_test "-var-info-num-children fake" \
+ "\\^done,numchild=\"2\"" "-var-info-num-children fake"
+
+ mi_list_varobj_children {"fake" "--available-children-only" } {
+ { fake.public public 0 }
+ { fake.private private 0 }
+ } "-var-list-children --available-children-only fake"
+
+ mi_gdb_test "-var-info-num-children --available-children-only fake" \
+ "\\^done,numchild=\"2\"" \
+ "-var-info-num-children --available-children-only fake"
+
+ mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
+ }
+ mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
+ "-trace-find none"
+}
+
+check_with_traceframe
diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only.exp b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
new file mode 100644
index 0000000..e22c90f
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
@@ -0,0 +1,198 @@
+# Copyright 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile available-children-only.c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested mi-available-children-only.exp
+ return -1
+}
+
+# Test target supports tracepoints or not.
+
+clean_restart $testfile
+
+if { ![runto_main] } {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+if { ![gdb_target_supports_trace] } {
+ unsupported "Current target does not support trace"
+ return -1
+}
+gdb_exit
+
+if { [mi_gdb_start] } {
+ continue
+}
+
+mi_run_to_main
+mi_gdb_test "-break-insert -a marker1" "\\^done.*" \
+ "trace marker1"
+
+mi_gdb_test "-break-commands \$tpnum \"collect s.a\" \"collect s.s1.d\" \"collect s.s1.s3.g\" \"collect s.s2.f\" \"end\" " \
+ {\^done} "set action on marker1"
+
+mi_gdb_test "-break-insert -a marker2" "\\^done.*" \
+ "trace marker2"
+
+mi_gdb_test "-break-commands \$tpnum \"collect s.a\" \"collect s.b\" \"collect s.s1.s3.h\" \"end\" " \
+ {\^done} "set action on marker2"
+
+mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
+mi_continue_to end
+mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
+
+# Save trace frames to trace file.
+set tracefile [standard_output_file ${testfile}]
+mi_gdb_test "-trace-save ${tracefile}.tfile" \
+ ".*" \
+ "save tfile trace"
+
+# In live target, '--available-children-only' shouldn't have any
+# effects.
+mi_gdb_test "-var-create --available-children-only s1 @ s" \
+ {\^done,name="s1",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only s1"
+
+mi_list_varobj_children { "s1" "--available-children-only" } {
+ { s1.a a 0 int }
+ { s1.b b 0 int }
+ { s1.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+ { s1.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
+} "-var-list-children --available-children-only s1"
+
+mi_gdb_test "-var-info-num-children --available-children-only s1" \
+ "\\^done,numchild=\"4\"" \
+ "-var-info-num-children --available-children-only s1"
+
+mi_gdb_test "-var-delete s1" {\^done,ndeleted="5"} "-var-delete s1"
+
+# Select a traceframe, and '--available-children-only' have some
+# effects.
+
+proc check_with_traceframe { } {
+ global decimal
+
+ mi_gdb_test "-trace-find frame-number 0" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+ "-trace-find frame-number 0"
+
+ with_test_prefix "traceframe 0" {
+ mi_gdb_test "-var-create --available-children-only s2 @ s" \
+ {\^done,name="s2",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only s2"
+
+ mi_gdb_test "-var-list-children s2" \
+ "\\^done,numchild=\"4\",.*,has_more=\"0\"" \
+ "-var-list-children s2"
+
+ mi_gdb_test "-var-info-num-children s2" \
+ "\\^done,numchild=\"4\"" \
+ "-var-info-num-children s2"
+
+ # "s2" should have children "a", "s1" and "s2".
+ mi_list_varobj_children { "s2" "--available-children-only" } {
+ { s2.a a 0 int }
+ { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+ { s2.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
+ } "-var-list-children --available-children-only s2"
+
+ mi_gdb_test "-var-info-num-children --available-children-only s2" \
+ "\\^done,numchild=\"3\"" \
+ "-var-info-num-children --available-children-only s2"
+
+ mi_list_varobj_children { "s2.s1" "--available-children-only" } {
+ { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
+ { s2.s1.d d 0 int }
+ } "-var-list-children --available-children-only s2.s1"
+
+ }
+
+ mi_gdb_test "-trace-find frame-number 1" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"1\",frame=\{.*" \
+ "-trace-find frame-number 1"
+
+ with_test_prefix "traceframe 1" {
+ mi_list_varobj_children { "s2" "--available-children-only" } {
+ { s2.a a 0 int }
+ { s2.b b 0 int }
+ { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+ } "-var-list-children --available-children-only s2"
+
+ mi_list_varobj_children { "s2.s1" "--available-children-only" } {
+ { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
+ } "-var-list-children --available-children-only s2.s1"
+
+ mi_list_varobj_children { "s2.s1.s3" "--available-children-only" } {
+ { s2.s1.s3.h h 0 int }
+ } "-var-list-children --available-children-only s2.s1.s3"
+
+ mi_gdb_test "-var-info-num-children --available-children-only s2" \
+ "\\^done,numchild=\"3\"" \
+ "-var-info-num-children --available-children-only s2"
+
+ mi_gdb_test "-var-delete s2" {\^done,ndeleted="6"} "-var-delete s2"
+ }
+
+ mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
+ "-trace-find none"
+}
+
+check_with_traceframe
+
+# Test when changing target from remote to ${target}.
+
+proc test_from_remote { target } {
+ global mi_gdb_prompt decimal
+ global tracefile
+
+ with_test_prefix "remote to ${target}" {
+ # Change target to ${target}.
+ mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
+ "change target to ${target}"
+
+ with_test_prefix "w/ setting traceframe" {
+ check_with_traceframe
+ }
+ }
+}
+
+test_from_remote "tfile"
+
+proc test_from_exec { target } {
+ global binfile
+ global tracefile
+
+ mi_gdb_exit
+ if [mi_gdb_start] {
+ return
+ }
+ mi_gdb_load ${binfile}
+
+ with_test_prefix "exec to ${target}" {
+ mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
+ "change target to ${target}"
+
+ check_with_traceframe
+ }
+}
+
+test_from_exec "tfile"
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 11/12] Test case
2014-02-14 8:46 ` [PATCH 11/12] Test case Yao Qi
@ 2014-04-18 11:45 ` Yao Qi
2014-04-24 20:53 ` Keith Seitz
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-04-18 11:45 UTC (permalink / raw)
To: gdb-patches
On 02/14/2014 04:44 PM, Yao Qi wrote:
> - Remove hard-coded breakpoint number, but it depends on this patch
>
> [PATCH] Accept convenience variable in commands -break-passcount and -break-commands
> https://sourceware.org/ml/gdb-patches/2014-01/msg00936.html
This patch above isn't acceptable as its current shape, so I revert
this patch back to use hard-coded breakpoint number.
--
Yao (é½å°§)
gdb/testsuite:
2014-04-18 Yao Qi <yao@codesourcery.com>
* gdb.trace/available-children-only.c: New.
* gdb.trace/mi-available-children-only.exp: New.
* gdb.trace/mi-available-children-only-cxx.exp: New.
* gdb.trace/available-children-only.cc: New.
gdb:
2014-04-18 Yao Qi <yao@codesourcery.com>
* config/djgpp/fnchange.lst: Add mapping for
available-children-only.c, mi-available-children-only.exp,
mi-available-children-only-cxx.exp and
mi-available-children-only.cc.
---
gdb/config/djgpp/fnchange.lst | 4 +
gdb/testsuite/gdb.trace/available-children-only.c | 69 +++++++
gdb/testsuite/gdb.trace/available-children-only.cc | 45 +++++
.../gdb.trace/mi-available-children-only-cxx.exp | 126 +++++++++++++
.../gdb.trace/mi-available-children-only.exp | 198 +++++++++++++++++++++
5 files changed, 442 insertions(+)
create mode 100644 gdb/testsuite/gdb.trace/available-children-only.c
create mode 100644 gdb/testsuite/gdb.trace/available-children-only.cc
create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
create mode 100644 gdb/testsuite/gdb.trace/mi-available-children-only.exp
diff --git a/gdb/config/djgpp/fnchange.lst b/gdb/config/djgpp/fnchange.lst
index cbf11c6..b94c77e 100644
--- a/gdb/config/djgpp/fnchange.lst
+++ b/gdb/config/djgpp/fnchange.lst
@@ -506,6 +506,10 @@
@V@/gdb/testsuite/gdb.mi/mi2-var-display.exp @V@/gdb/testsuite/gdb.mi/mi2vardisplay.exp
@V@/gdb/testsuite/gdb.mi/mi-nonstop-exit.exp @V@/gdb/testsuite/gdb.mi/minonstop-exit.exp
@V@/gdb/testsuite/gdb.mi/non-stop-exit.c @V@/gdb/testsuite/gdb.mi/nonstop-exit.c
+@V@/gdb/testsuite/gdb.trace/available-children-only.c @V@/gdb/testsuite/gdb.trace/availnly.c
+@V@/gdb/testsuite/gdb.trace/mi-available-children-only.exp @V@/gdb/testsuite/gdb.trace/availnly.exp
+@V@/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp @V@/gdb/testsuite/gdb.trace/availcxx.exp
+@V@/gdb/testsuite/gdb.trace/available-children-only.cc @V@/gdb/testsuite/gdb.trace/availcxx.cc
@V@/gdb/testsuite/gdb.threads/watchthreads2.c @V@/gdb/testsuite/gdb.threads/watchth2.c
@V@/gdb/testsuite/gdb.threads/watchthreads2.exp @V@/gdb/testsuite/gdb.threads/watchth2.exp
@V@/gdb/amd64-linux-tdep.c @V@/gdb/amd64-ltdep.c
diff --git a/gdb/testsuite/gdb.trace/available-children-only.c b/gdb/testsuite/gdb.trace/available-children-only.c
new file mode 100644
index 0000000..b13f130
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/available-children-only.c
@@ -0,0 +1,69 @@
+/* Copyright 2013-2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Two trace actions are set, and they collect different fields to
+ reproduce the case that children appear in the different orders. */
+
+struct simple
+{
+ int a; /* Collected by both. */
+ int b; /* Collected by action 2. */
+ struct
+ {
+ struct
+ {
+ int g; /* Collected by action 1. */
+ int h; /* Collected by action 2. */
+ } s3;
+ int d; /* Collected by action 1. */
+ } s1;
+
+ struct
+ {
+ int e;
+ int f; /* Collected by action 1. */
+ } s2;
+};
+
+struct simple s;
+
+static void
+marker1 (void)
+{
+}
+
+static void
+marker2 (void)
+{
+}
+
+static void
+end (void)
+{
+}
+
+int
+main (void)
+{
+ marker1 ();
+
+ marker2 ();
+
+ end ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/available-children-only.cc b/gdb/testsuite/gdb.trace/available-children-only.cc
new file mode 100644
index 0000000..f9f06e7
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/available-children-only.cc
@@ -0,0 +1,45 @@
+/* Copyright 2013-2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+class Fake
+{
+ private:
+ int pri1; /* Collected. */
+
+ public:
+ int pub1;
+
+};
+
+Fake fake;
+
+static void
+marker (void)
+{}
+
+static void
+end (void)
+{}
+
+int
+main (void)
+{
+ marker ();
+
+ end ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
new file mode 100644
index 0000000..cc6b0f2
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-available-children-only-cxx.exp
@@ -0,0 +1,126 @@
+# Copyright 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test for C++ fake children.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile available-children-only.cc
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+ executable {debug c++}] != "" } {
+ untested "Couldn't compile ${srcfile}"
+ return -1
+}
+
+# Test target supports tracepoints or not.
+
+clean_restart $testfile
+
+if { ![runto_main] } {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+if { ![gdb_target_supports_trace] } {
+ unsupported "Current target does not support trace"
+ return -1
+}
+gdb_exit
+
+if { [mi_gdb_start] } {
+ continue
+}
+
+mi_run_to_main
+
+mi_gdb_test "-break-insert -a marker" "\\^done.*" \
+ "trace marker"
+
+mi_gdb_test "-break-commands 2 \"collect fake.pri1\" \"end\" " \
+ {\^done} "set action on marker"
+
+mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
+mi_continue_to end
+mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
+# Save trace frames to trace file.
+set tracefile [standard_output_file ${testfile}]
+mi_gdb_test "-trace-save ${tracefile}.tfile" \
+ ".*" \
+ "save tfile trace"
+
+# In live target, '--available-children-only' shouldn't have any
+# effects.
+
+mi_gdb_test "-trace-find frame-number 0" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+ "-trace-find frame-number 0"
+
+mi_gdb_test "-var-create --available-children-only fake @ fake" \
+ {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only fake"
+
+mi_list_varobj_children { fake --available-children-only } {
+ { fake.public public 0 }
+ { fake.private private 0 }
+} "-var-list-children --available-children-only fake"
+
+mi_gdb_test "-var-info-num-children --available-children-only fake" \
+ "\\^done,numchild=\"2\"" \
+ "-var-info-num-children --available-children-only fake"
+
+mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
+
+# Select a traceframe, and '--available-children-only' have some
+# effects.
+
+proc check_with_traceframe { } {
+ global decimal
+
+ with_test_prefix "w/ setting traceframe" {
+ mi_gdb_test "-trace-find frame-number 0" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+ "-trace-find frame-number 0"
+
+ mi_gdb_test "-var-create --available-children-only fake @ fake" \
+ {\^done,name="fake",numchild="0",value=".*",type="Fake",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only fake"
+
+ mi_list_varobj_children "fake" {
+ { fake.public public 1 }
+ { fake.private private 1 }
+ } "-var-list-children fake"
+
+ mi_gdb_test "-var-info-num-children fake" \
+ "\\^done,numchild=\"2\"" "-var-info-num-children fake"
+
+ mi_list_varobj_children {"fake" "--available-children-only" } {
+ { fake.public public 0 }
+ { fake.private private 0 }
+ } "-var-list-children --available-children-only fake"
+
+ mi_gdb_test "-var-info-num-children --available-children-only fake" \
+ "\\^done,numchild=\"2\"" \
+ "-var-info-num-children --available-children-only fake"
+
+ mi_gdb_test "-var-delete fake" {\^done,ndeleted="3"} "-var-delete fake"
+ }
+ mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
+ "-trace-find none"
+}
+
+check_with_traceframe
diff --git a/gdb/testsuite/gdb.trace/mi-available-children-only.exp b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
new file mode 100644
index 0000000..247c32c
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-available-children-only.exp
@@ -0,0 +1,198 @@
+# Copyright 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile available-children-only.c
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested mi-available-children-only.exp
+ return -1
+}
+
+# Test target supports tracepoints or not.
+
+clean_restart $testfile
+
+if { ![runto_main] } {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+if { ![gdb_target_supports_trace] } {
+ unsupported "Current target does not support trace"
+ return -1
+}
+gdb_exit
+
+if { [mi_gdb_start] } {
+ continue
+}
+
+mi_run_to_main
+mi_gdb_test "-break-insert -a marker1" "\\^done.*" \
+ "trace marker1"
+
+mi_gdb_test "-break-commands 2 \"collect s.a\" \"collect s.s1.d\" \"collect s.s1.s3.g\" \"collect s.s2.f\" \"end\" " \
+ {\^done} "set action on marker1"
+
+mi_gdb_test "-break-insert -a marker2" "\\^done.*" \
+ "trace marker2"
+
+mi_gdb_test "-break-commands 3 \"collect s.a\" \"collect s.b\" \"collect s.s1.s3.h\" \"end\" " \
+ {\^done} "set action on marker2"
+
+mi_gdb_test "-trace-start" ".*\\^done.*" "trace start"
+mi_continue_to end
+mi_gdb_test "-trace-stop" "\\^done.*" "trace stop"
+
+# Save trace frames to trace file.
+set tracefile [standard_output_file ${testfile}]
+mi_gdb_test "-trace-save ${tracefile}.tfile" \
+ ".*" \
+ "save tfile trace"
+
+# In live target, '--available-children-only' shouldn't have any
+# effects.
+mi_gdb_test "-var-create --available-children-only s1 @ s" \
+ {\^done,name="s1",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only s1"
+
+mi_list_varobj_children { "s1" "--available-children-only" } {
+ { s1.a a 0 int }
+ { s1.b b 0 int }
+ { s1.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+ { s1.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
+} "-var-list-children --available-children-only s1"
+
+mi_gdb_test "-var-info-num-children --available-children-only s1" \
+ "\\^done,numchild=\"4\"" \
+ "-var-info-num-children --available-children-only s1"
+
+mi_gdb_test "-var-delete s1" {\^done,ndeleted="5"} "-var-delete s1"
+
+# Select a traceframe, and '--available-children-only' have some
+# effects.
+
+proc check_with_traceframe { } {
+ global decimal
+
+ mi_gdb_test "-trace-find frame-number 0" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"0\",frame=\{.*" \
+ "-trace-find frame-number 0"
+
+ with_test_prefix "traceframe 0" {
+ mi_gdb_test "-var-create --available-children-only s2 @ s" \
+ {\^done,name="s2",numchild="0",value=".*",type="struct simple",dynamic="1",has_more="1"} \
+ "-var-create --available-children-only s2"
+
+ mi_gdb_test "-var-list-children s2" \
+ "\\^done,numchild=\"4\",.*,has_more=\"0\"" \
+ "-var-list-children s2"
+
+ mi_gdb_test "-var-info-num-children s2" \
+ "\\^done,numchild=\"4\"" \
+ "-var-info-num-children s2"
+
+ # "s2" should have children "a", "s1" and "s2".
+ mi_list_varobj_children { "s2" "--available-children-only" } {
+ { s2.a a 0 int }
+ { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+ { s2.s2 s2 0 "struct \\{\\.\\.\\.\\}" }
+ } "-var-list-children --available-children-only s2"
+
+ mi_gdb_test "-var-info-num-children --available-children-only s2" \
+ "\\^done,numchild=\"3\"" \
+ "-var-info-num-children --available-children-only s2"
+
+ mi_list_varobj_children { "s2.s1" "--available-children-only" } {
+ { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
+ { s2.s1.d d 0 int }
+ } "-var-list-children --available-children-only s2.s1"
+
+ }
+
+ mi_gdb_test "-trace-find frame-number 1" \
+ ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"1\",frame=\{.*" \
+ "-trace-find frame-number 1"
+
+ with_test_prefix "traceframe 1" {
+ mi_list_varobj_children { "s2" "--available-children-only" } {
+ { s2.a a 0 int }
+ { s2.b b 0 int }
+ { s2.s1 s1 0 "struct \\{\\.\\.\\.\\}" }
+ } "-var-list-children --available-children-only s2"
+
+ mi_list_varobj_children { "s2.s1" "--available-children-only" } {
+ { s2.s1.s3 s3 0 "struct \\{\\.\\.\\.\\}" }
+ } "-var-list-children --available-children-only s2.s1"
+
+ mi_list_varobj_children { "s2.s1.s3" "--available-children-only" } {
+ { s2.s1.s3.h h 0 int }
+ } "-var-list-children --available-children-only s2.s1.s3"
+
+ mi_gdb_test "-var-info-num-children --available-children-only s2" \
+ "\\^done,numchild=\"3\"" \
+ "-var-info-num-children --available-children-only s2"
+
+ mi_gdb_test "-var-delete s2" {\^done,ndeleted="6"} "-var-delete s2"
+ }
+
+ mi_gdb_test "-trace-find none" ".*\\^done,found=\"0\".*" \
+ "-trace-find none"
+}
+
+check_with_traceframe
+
+# Test when changing target from remote to ${target}.
+
+proc test_from_remote { target } {
+ global mi_gdb_prompt decimal
+ global tracefile
+
+ with_test_prefix "remote to ${target}" {
+ # Change target to ${target}.
+ mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
+ "change target to ${target}"
+
+ with_test_prefix "w/ setting traceframe" {
+ check_with_traceframe
+ }
+ }
+}
+
+test_from_remote "tfile"
+
+proc test_from_exec { target } {
+ global binfile
+ global tracefile
+
+ mi_gdb_exit
+ if [mi_gdb_start] {
+ return
+ }
+ mi_gdb_load ${binfile}
+
+ with_test_prefix "exec to ${target}" {
+ mi_gdb_test "-target-select ${target} ${tracefile}.${target}" ".*\\^connected.*" \
+ "change target to ${target}"
+
+ check_with_traceframe
+ }
+}
+
+test_from_exec "tfile"
--
1.9.0
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 11/12] Test case
2014-04-18 11:45 ` Yao Qi
@ 2014-04-24 20:53 ` Keith Seitz
0 siblings, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-24 20:53 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 04/18/2014 04:42 AM, Yao Qi wrote:
> On 02/14/2014 04:44 PM, Yao Qi wrote:
>> - Remove hard-coded breakpoint number, but it depends on this patch
>>
>> [PATCH] Accept convenience variable in commands -break-passcount and -break-commands
>> https://sourceware.org/ml/gdb-patches/2014-01/msg00936.html
>
> This patch above isn't acceptable as its current shape, so I revert
> this patch back to use hard-coded breakpoint number.
>
Yeah, I apologize for that. Something tells me we should consider
extending the mi_create_breakpoint API to not only return the breakpoint
regexp but also the number of the breakpoint that was set. That would
make things like this quite a bit easier in the future.
I really only have three (related) nits about this patch:
In gdb.trace/mi-available-children-only-cxx.exp:
+mi_gdb_test "-break-insert -a marker" "\\^done.*" \
+ "trace marker"
+
and in gdb.trace/mi-available-children-only.exp:
+mi_gdb_test "-break-insert -a marker1" "\\^done.*" \
+ "trace marker1"
[snip]
+mi_gdb_test "-break-insert -a marker2" "\\^done.*" \
+ "trace marker2"
Please use mi_create_breakpoint unless there is some feature that is not
supported by that API.
Otherwise, this looks okay. I recommend a maintainer approve with these
changes.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 09/12] Delete varobj's children on traceframe is changed.
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (4 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 11/12] Test case Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-24 20:45 ` Keith Seitz
2014-02-14 8:46 ` [PATCH 04/12] Remove #if HAVE_PYTHON Yao Qi
` (7 subsequent siblings)
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
Hi,
The memory availability varies on trace frames. When
--available-children-only is used, the varobj tree structure changes
when trace frame is changed. GDB has to remove varobj's children if
it is marked as 'available_children_only'. For example, in traceframe
1, foo.a and foo.c is collected, and in traceframe 2, foo.b is
collected,
struct foo
{
int a; /* Collected in traceframe 1 */
int b; /* Collected in traceframe 2 */
int c; /* Collected in traceframe 1 */
};
When available-children-only is used, the expected result is that in
traceframe 1, foo has two children (a and c), and foo has one child
(b) in traceframe 2. Without this patch, foo has a, b, and c in
traceframe 2, which is wrong.
In this patch, we install a traceframe_changed observer to clear
varobjs marked as 'available_children_only'.
V2:
- Move common code to varobj_clear_children.
- Update comments.
gdb:
2014-02-14 Yao Qi <yao@codesourcery.com>
* varobj.c: Include "observer.h".
(varobj_clear_children): New functiuon.
(varobj_set_available_children_only): Move some code to
varobj_clear_children. Call varobj_clear_children.
(varobj_delete_if_available_children_only): New function.
(varobj_traceframe_changed): New function.
(_initialize_varobj): Install varobj_traceframe_changed to
traceframe_changed observer.
---
gdb/varobj.c | 47 +++++++++++++++++++++++++++++++++++++++--------
1 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index bfa6b1d..f0ea383 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -24,6 +24,7 @@
#include "gdbcmd.h"
#include "block.h"
#include "valprint.h"
+#include "observer.h"
#include "gdb_assert.h"
#include <string.h>
@@ -919,6 +920,21 @@ varobj_get_num_children (struct varobj *var)
return var->num_children >= 0 ? var->num_children : 0;
}
+/* Clear children of VAR and stuff needed for iteration over
+ children. */
+
+static void
+varobj_clear_children (struct varobj *var)
+{
+ /* If there are any children now, wipe them. */
+ varobj_delete (var, NULL, /* children only */ 1);
+ var->num_children = -1;
+
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = NULL;
+ varobj_clear_saved_item (var->dynamic);
+}
+
/* Update the field available_children_only of variable object VAR
with AVAILABLE_ONLY. */
@@ -927,15 +943,9 @@ varobj_set_available_children_only (struct varobj *var, int available_only)
{
if (var->dynamic->available_children_only != available_only)
{
- /* If there are any children now, wipe them. */
- varobj_delete (var, NULL, /* children only */ 1);
- var->num_children = -1;
- var->dynamic->available_children_only = available_only;
+ varobj_clear_children (var);
- /* We're starting over, so get rid of any iterator. */
- varobj_iter_delete (var->dynamic->child_iter);
- var->dynamic->child_iter = NULL;
- varobj_clear_saved_item (var->dynamic);
+ var->dynamic->available_children_only = available_only;
}
}
@@ -2753,6 +2763,25 @@ all_root_varobjs (void (*func) (struct varobj *var, void *data), void *data)
(*func) (var_root->rootvar, data);
}
}
+
+/* Delete VAR's children if it is marked as 'available_children_only'. */
+
+static void
+varobj_delete_if_available_children_only (struct varobj *var, void *data)
+{
+ if (var->dynamic->available_children_only)
+ varobj_clear_children (var);
+
+}
+
+/* The callback installed for traceframe_changed events. */
+
+static void
+varobj_traceframe_changed (int tfnum, int tpnum)
+{
+ all_root_varobjs (varobj_delete_if_available_children_only , NULL);
+}
+
\f
extern void _initialize_varobj (void);
void
@@ -2770,6 +2799,8 @@ _initialize_varobj (void)
_("When non-zero, varobj debugging is enabled."),
NULL, show_varobjdebug,
&setlist, &showlist);
+
+ observer_attach_traceframe_changed (varobj_traceframe_changed);
}
/* Invalidate varobj VAR if it is tied to locals and re-create it if it is
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 09/12] Delete varobj's children on traceframe is changed.
2014-02-14 8:46 ` [PATCH 09/12] Delete varobj's children on traceframe is changed Yao Qi
@ 2014-04-24 20:45 ` Keith Seitz
0 siblings, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-24 20:45 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> V2:
> - Move common code to varobj_clear_children.
> - Update comments.
>
> gdb:
>
> 2014-02-14 Yao Qi <yao@codesourcery.com>
>
> * varobj.c: Include "observer.h".
> (varobj_clear_children): New functiuon.
> (varobj_set_available_children_only): Move some code to
> varobj_clear_children. Call varobj_clear_children.
> (varobj_delete_if_available_children_only): New function.
> (varobj_traceframe_changed): New function.
> (_initialize_varobj): Install varobj_traceframe_changed to
> traceframe_changed observer.
This looks okay to me. I recommend a maintainer give this a final review.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 04/12] Remove #if HAVE_PYTHON
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (5 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 09/12] Delete varobj's children on traceframe is changed Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-23 19:25 ` Keith Seitz
2014-05-21 17:52 ` Tom Tromey
2014-02-14 8:46 ` [PATCH 07/12] MI option --available-children-only Yao Qi
` (6 subsequent siblings)
13 siblings, 2 replies; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
This patch removes some unnecessary "#if HAVE_PYTHON" so that more
code is generalized.
gdb:
2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* varobj.c: Remove "#if HAVE_PYTHON" and "#endif".
(varobj_get_iterator): Wrap up code for pretty-printer by
"#if HAVE_PYTHON" and "#endif".
(update_dynamic_varobj_children): Likewise.
---
gdb/varobj.c | 20 +++++---------------
1 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 33a2b0a..590aba6 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -212,13 +212,9 @@ static char *my_value_of_variable (struct varobj *var,
static int is_root_p (struct varobj *var);
-#if HAVE_PYTHON
-
static struct varobj *varobj_add_child (struct varobj *var,
struct varobj_item *item);
-#endif /* HAVE_PYTHON */
-
/* Private data */
/* Mappings of varobj_display_formats enums to gdb's format codes. */
@@ -701,8 +697,6 @@ varobj_restrict_range (VEC (varobj_p) *children, int *from, int *to)
}
}
-#if HAVE_PYTHON
-
/* A helper for update_dynamic_varobj_children that installs a new
child when needed. */
@@ -747,6 +741,8 @@ install_dynamic_child (struct varobj *var,
}
}
+#if HAVE_PYTHON
+
static int
dynamic_varobj_has_child_method (struct varobj *var)
{
@@ -762,6 +758,7 @@ dynamic_varobj_has_child_method (struct varobj *var)
do_cleanups (back_to);
return result;
}
+#endif
/* A factory for creating dynamic varobj's iterators. Returns an
iterator object suitable for iterating over VAR's children. */
@@ -769,8 +766,10 @@ dynamic_varobj_has_child_method (struct varobj *var)
static struct varobj_iter *
varobj_get_iterator (struct varobj *var)
{
+#if HAVE_PYTHON
if (var->dynamic->pretty_printer)
return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
+#endif
gdb_assert_not_reached (_("\
requested an iterator from a non-dynamic varobj"));
@@ -788,7 +787,6 @@ varobj_clear_saved_item (struct varobj_dynamic *var)
var->saved_item = NULL;
}
}
-#endif
static int
update_dynamic_varobj_children (struct varobj *var,
@@ -801,7 +799,6 @@ update_dynamic_varobj_children (struct varobj *var,
int from,
int to)
{
-#if HAVE_PYTHON
int i;
*cchanged = 0;
@@ -891,9 +888,6 @@ update_dynamic_varobj_children (struct varobj *var,
var->num_children = VEC_length (varobj_p, var->children);
return 1;
-#else
- gdb_assert_not_reached ("should never be called if Python is not enabled");
-#endif
}
int
@@ -970,8 +964,6 @@ varobj_list_children (struct varobj *var, int *from, int *to)
return var->children;
}
-#if HAVE_PYTHON
-
static struct varobj *
varobj_add_child (struct varobj *var, struct varobj_item *item)
{
@@ -983,8 +975,6 @@ varobj_add_child (struct varobj *var, struct varobj_item *item)
return v;
}
-#endif /* HAVE_PYTHON */
-
/* Obtain the type of an object Variable as a string similar to the one gdb
prints on the console. */
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 04/12] Remove #if HAVE_PYTHON
2014-02-14 8:46 ` [PATCH 04/12] Remove #if HAVE_PYTHON Yao Qi
@ 2014-04-23 19:25 ` Keith Seitz
2014-05-21 17:52 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-23 19:25 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> This patch removes some unnecessary "#if HAVE_PYTHON" so that more
> code is generalized.
>
> gdb:
>
> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * varobj.c: Remove "#if HAVE_PYTHON" and "#endif".
> (varobj_get_iterator): Wrap up code for pretty-printer by
> "#if HAVE_PYTHON" and "#endif".
> (update_dynamic_varobj_children): Likewise.
Removing some #ifdef's -- yeah! I recommend a maintainer approve this patch.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 04/12] Remove #if HAVE_PYTHON
2014-02-14 8:46 ` [PATCH 04/12] Remove #if HAVE_PYTHON Yao Qi
2014-04-23 19:25 ` Keith Seitz
@ 2014-05-21 17:52 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2014-05-21 17:52 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao> Yao Qi <yao@codesourcery.com>
Yao> * varobj.c: Remove "#if HAVE_PYTHON" and "#endif".
Yao> (varobj_get_iterator): Wrap up code for pretty-printer by
Yao> "#if HAVE_PYTHON" and "#endif".
Yao> (update_dynamic_varobj_children): Likewise.
This is ok.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 07/12] MI option --available-children-only
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (6 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 04/12] Remove #if HAVE_PYTHON Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-24 20:02 ` Keith Seitz
2014-02-14 8:46 ` [PATCH 06/12] Use varobj_is_dynamic_p more widely Yao Qi
` (5 subsequent siblings)
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
This patch adds option --available-children-only to three MI commands,
parse it, and set corresponding field of 'struct varobj_dynamic'.
V2:
- Document varobj_set_available_children_only.
- Return "available-children-only" from "-list-features".
gdb:
2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* mi/mi-cmd-var.c (mi_cmd_var_create): Use mi_getopt to parse
options. Handle "--available-children-only".
(mi_cmd_var_info_num_children): Likewise.
(mi_cmd_var_list_children): Likewise.
* mi/mi-main.c (mi_cmd_list_features): Add
"available-children-only".
* varobj.c (struct varobj_dynamic) <available_children_only>:
New.
(new_variable, new_root_variable): Update declaration.
(varobj_is_dynamic_p): Return true if available-children-only
is true.
(varobj_create): Add new argument "available_children_only".
Callers update.
(varobj_get_num_children): Handle "available_children_only"
(varobj_set_available_children_only): New function.
(varobj_list_children): Handle "available_children_only".
(install_new_value): Likewise.
(varobj_update): Likewise.
(new_variable): Add new argument "available_children_only".
(new_root_variable): Likewise.
(varobj_invalidate_iter): Likewise.
* varobj.h (varobj_create): Update declaration.
(varobj_set_available_children_only): Declare.
---
gdb/mi/mi-cmd-var.c | 130 ++++++++++++++++++++++++++++++++++++++++++++-------
gdb/mi/mi-main.c | 1 +
gdb/varobj.c | 54 ++++++++++++++++-----
gdb/varobj.h | 11 ++++-
4 files changed, 166 insertions(+), 30 deletions(-)
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 9059616..a0cc91a 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -105,19 +105,50 @@ mi_cmd_var_create (char *command, char **argv, int argc)
char *expr;
struct cleanup *old_cleanups;
enum varobj_type var_type;
+ int available_children_only = 0;
+ int oind = 0;
+ enum opt
+ {
+ AVAILABLE_CHILDREN_ONLY,
+ };
+ static const struct mi_opt opts[] =
+ {
+ {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
+ { 0, 0, 0 }
+ };
- if (argc != 3)
- error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
+ /* Parse arguments. In this instance we are just looking for
+ --available_children_only. */
+ while (1)
+ {
+ char *oarg;
+ int opt = mi_getopt ("-var-create", argc, argv,
+ opts, &oind, &oarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case AVAILABLE_CHILDREN_ONLY:
+ available_children_only = 1;
+ break;
+ }
+ }
- name = xstrdup (argv[0]);
+
+ if (argc - oind != 3)
+ error (_("Usage: -var-create [--available-children-only] "
+ "NAME FRAME EXPRESSION"));
+
+
+ name = xstrdup (argv[oind]);
/* Add cleanup for name. Must be free_current_contents as name can
be reallocated. */
old_cleanups = make_cleanup (free_current_contents, &name);
- frame = xstrdup (argv[1]);
+ frame = xstrdup (argv[oind + 1]);
make_cleanup (xfree, frame);
- expr = xstrdup (argv[2]);
+ expr = xstrdup (argv[oind + 2]);
make_cleanup (xfree, expr);
if (strcmp (name, "-") == 0)
@@ -143,7 +174,8 @@ mi_cmd_var_create (char *command, char **argv, int argc)
"Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
name, frame, hex_string (frameaddr), expr);
- var = varobj_create (name, expr, frameaddr, var_type);
+ var = varobj_create (name, expr, frameaddr, var_type,
+ available_children_only);
if (var == NULL)
error (_("-var-create: unable to create variable object"));
@@ -328,12 +360,45 @@ mi_cmd_var_info_num_children (char *command, char **argv, int argc)
{
struct ui_out *uiout = current_uiout;
struct varobj *var;
+ int available_children_only = 0;
+ int oind = 0;
+ enum opt
+ {
+ AVAILABLE_CHILDREN_ONLY,
+ };
+ static const struct mi_opt opts[] =
+ {
+ {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
+ { 0, 0, 0 }
+ };
- if (argc != 1)
- error (_("-var-info-num-children: Usage: NAME."));
+ /* Parse arguments. In this instance we are just looking for
+ --available_children_only. */
+ while (1)
+ {
+ char *oarg;
+ int opt = mi_getopt ("-var-info-num-children", argc, argv,
+ opts, &oind, &oarg);
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case AVAILABLE_CHILDREN_ONLY:
+ available_children_only = 1;
+ break;
+ }
+ }
+
+ if (argc - oind != 1)
+ {
+ error (_("-var-info-num-children: Usage: "
+ "[--available-children-only] NAME."));
+ }
/* Get varobj handle, if a valid var obj name was specified. */
- var = varobj_get_handle (argv[0]);
+ var = varobj_get_handle (argv[oind]);
+
+ varobj_set_available_children_only (var, available_children_only);
ui_out_field_int (uiout, "numchild", varobj_get_num_children (var));
}
@@ -381,18 +446,47 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
int ix;
int from, to;
char *display_hint;
+ int available_children_only = 0;
+ int oind = 0;
+ enum opt
+ {
+ AVAILABLE_CHILDREN_ONLY,
+ };
+ static const struct mi_opt opts[] =
+ {
+ {"-available-children-only", AVAILABLE_CHILDREN_ONLY, 0},
+ { 0, 0, 0 }
+ };
+
+ /* Parse arguments. In this instance we are just looking for
+ --available_children_only. */
+ while (1)
+ {
+ char *oarg;
+ int opt = mi_getopt_allow_unknown ("-var-list-children", argc, argv,
+ opts, &oind, &oarg);
+
+ if (opt < 0)
+ break;
+ switch ((enum opt) opt)
+ {
+ case AVAILABLE_CHILDREN_ONLY:
+ available_children_only = 1;
+ break;
+ }
+ }
- if (argc < 1 || argc > 4)
- error (_("-var-list-children: Usage: "
+ if (argc - oind < 1 || argc - oind > 4)
+ error (_("-var-list-children: Usage: [--available-children-only] "
"[PRINT_VALUES] NAME [FROM TO]"));
/* Get varobj handle, if a valid var obj name was specified. */
- if (argc == 1 || argc == 3)
- var = varobj_get_handle (argv[0]);
+ if (argc - oind == 1 || argc - oind == 3)
+ var = varobj_get_handle (argv[oind]);
else
- var = varobj_get_handle (argv[1]);
+ var = varobj_get_handle (argv[oind + 1]);
- if (argc > 2)
+ if (argc - oind > 2)
{
from = atoi (argv[argc - 2]);
to = atoi (argv[argc - 1]);
@@ -403,10 +497,12 @@ mi_cmd_var_list_children (char *command, char **argv, int argc)
to = -1;
}
+ varobj_set_available_children_only (var, available_children_only);
+
children = varobj_list_children (var, &from, &to);
ui_out_field_int (uiout, "numchild", to - from);
- if (argc == 2 || argc == 4)
- print_values = mi_parse_print_values (argv[0]);
+ if (argc - oind == 2 || argc - oind == 4)
+ print_values = mi_parse_print_values (argv[oind]);
else
print_values = PRINT_NO_VALUES;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 161307a..6983e49 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1817,6 +1817,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
ui_out_field_string (uiout, NULL, "undefined-command-error-code");
ui_out_field_string (uiout, NULL, "exec-run-start-option");
+ ui_out_field_string (uiout, NULL, "available-children-only");
if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON)))
ui_out_field_string (uiout, NULL, "python");
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 8905087..61309b2 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -119,6 +119,10 @@ struct varobj_dynamic
can avoid that. */
int children_requested;
+ /* True if this varobj only includes available/collected objects in
+ its list of children. */
+ int available_children_only;
+
/* The pretty-printer constructor. If NULL, then the default
pretty-printer will be looked up. If None, then no
pretty-printer will be installed. */
@@ -175,9 +179,9 @@ create_child_with_value (struct varobj *parent, int index,
/* Utility routines */
-static struct varobj *new_variable (void);
+static struct varobj *new_variable (int available_children_only);
-static struct varobj *new_root_variable (void);
+static struct varobj *new_root_variable (int available_children_only);
static void free_variable (struct varobj *var);
@@ -285,14 +289,14 @@ find_frame_addr_in_frame_chain (CORE_ADDR frame_addr)
}
struct varobj *
-varobj_create (char *objname,
- char *expression, CORE_ADDR frame, enum varobj_type type)
+varobj_create (char *objname, char *expression, CORE_ADDR frame,
+ enum varobj_type type, int available_children_only)
{
struct varobj *var;
struct cleanup *old_chain;
/* Fill out a varobj structure for the (root) variable being constructed. */
- var = new_root_variable ();
+ var = new_root_variable (available_children_only);
old_chain = make_cleanup_free_variable (var);
if (expression != NULL)
@@ -911,6 +915,26 @@ varobj_get_num_children (struct varobj *var)
return var->num_children >= 0 ? var->num_children : 0;
}
+/* Update the field available_children_only of variable object VAR
+ with AVAILABLE_ONLY. */
+
+void
+varobj_set_available_children_only (struct varobj *var, int available_only)
+{
+ if (var->dynamic->available_children_only != available_only)
+ {
+ /* If there are any children now, wipe them. */
+ varobj_delete (var, NULL, /* children only */ 1);
+ var->num_children = -1;
+ var->dynamic->available_children_only = available_only;
+
+ /* We're starting over, so get rid of any iterator. */
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = NULL;
+ varobj_clear_saved_item (var->dynamic);
+ }
+}
+
/* Creates a list of the immediate children of a variable object;
the return code is the number of such children or -1 on error. */
@@ -1071,7 +1095,8 @@ varobj_get_attributes (struct varobj *var)
int
varobj_is_dynamic_p (struct varobj *var)
{
- return var->dynamic->pretty_printer != NULL;
+ return (var->dynamic->pretty_printer != NULL
+ || var->dynamic->available_children_only);
}
char *
@@ -2049,7 +2074,7 @@ create_child_with_value (struct varobj *parent, int index,
struct varobj *child;
char *childs_name;
- child = new_variable ();
+ child = new_variable (parent->dynamic->available_children_only);
/* NAME is allocated by caller. */
child->name = item->name;
@@ -2086,8 +2111,9 @@ create_child_with_value (struct varobj *parent, int index,
*/
/* Allocate memory and initialize a new variable. */
+
static struct varobj *
-new_variable (void)
+new_variable (int available_children_only)
{
struct varobj *var;
@@ -2116,15 +2142,17 @@ new_variable (void)
var->dynamic->pretty_printer = 0;
var->dynamic->child_iter = 0;
var->dynamic->saved_item = 0;
+ var->dynamic->available_children_only = available_children_only;
return var;
}
/* Allocate memory and initialize a new root variable. */
+
static struct varobj *
-new_root_variable (void)
+new_root_variable (int available_children_only)
{
- struct varobj *var = new_variable ();
+ struct varobj *var = new_variable (available_children_only);
var->root = (struct varobj_root *) xmalloc (sizeof (struct varobj_root));
var->root->lang_ops = NULL;
@@ -2397,7 +2425,8 @@ value_of_root (struct varobj **var_handle, int *type_changed)
char *old_type, *new_type;
tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
- USE_SELECTED_FRAME);
+ USE_SELECTED_FRAME,
+ var->dynamic->available_children_only);
if (tmp_var == NULL)
{
return NULL;
@@ -2757,7 +2786,8 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
/* Try to create a varobj with same expression. If we succeed
replace the old varobj, otherwise invalidate it. */
tmp_var = varobj_create (NULL, var->name, (CORE_ADDR) 0,
- USE_CURRENT_FRAME);
+ USE_CURRENT_FRAME,
+ var->dynamic->available_children_only);
if (tmp_var != NULL)
{
tmp_var->obj_name = xstrdup (var->obj_name);
diff --git a/gdb/varobj.h b/gdb/varobj.h
index f35f59b..516804a 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -225,7 +225,8 @@ const struct lang_varobj_ops ada_varobj_ops;
extern struct varobj *varobj_create (char *objname,
char *expression, CORE_ADDR frame,
- enum varobj_type type);
+ enum varobj_type type,
+ int available_children_only);
extern char *varobj_gen_name (void);
@@ -310,6 +311,14 @@ extern int varobj_is_dynamic_p (struct varobj *var);
extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
+/* Marks the varobj VAR as listing available children only or not,
+ depending on the boolean AVAILABLE_ONLY. If the
+ available-only-ness of the varobj changes compared to its present
+ state with this call, the varobj's current list of children is
+ deleted. */
+extern void varobj_set_available_children_only (struct varobj *var,
+ int available_only);
+
extern int varobj_default_value_is_changeable_p (struct varobj *var);
extern int varobj_value_is_changeable_p (struct varobj *var);
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 07/12] MI option --available-children-only
2014-02-14 8:46 ` [PATCH 07/12] MI option --available-children-only Yao Qi
@ 2014-04-24 20:02 ` Keith Seitz
0 siblings, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-24 20:02 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> This patch adds option --available-children-only to three MI commands,
> parse it, and set corresponding field of 'struct varobj_dynamic'.
>
> V2:
> - Document varobj_set_available_children_only.
> - Return "available-children-only" from "-list-features".
>
> gdb:
>
> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * mi/mi-cmd-var.c (mi_cmd_var_create): Use mi_getopt to parse
> options. Handle "--available-children-only".
> (mi_cmd_var_info_num_children): Likewise.
> (mi_cmd_var_list_children): Likewise.
> * mi/mi-main.c (mi_cmd_list_features): Add
> "available-children-only".
> * varobj.c (struct varobj_dynamic) <available_children_only>:
> New.
> (new_variable, new_root_variable): Update declaration.
> (varobj_is_dynamic_p): Return true if available-children-only
> is true.
> (varobj_create): Add new argument "available_children_only".
> Callers update.
> (varobj_get_num_children): Handle "available_children_only"
> (varobj_set_available_children_only): New function.
> (varobj_list_children): Handle "available_children_only".
> (install_new_value): Likewise.
> (varobj_update): Likewise.
> (new_variable): Add new argument "available_children_only".
> (new_root_variable): Likewise.
> (varobj_invalidate_iter): Likewise.
> * varobj.h (varobj_create): Update declaration.
> (varobj_set_available_children_only): Declare.
In general, I think this looks good. However, I am really bothered by
adding --available-children-only to -var-info-num-children and
-var-list-children.
When --available-children-only is passed to these commands, it
permanently overrides the varobj's current property value. I don't think
clients would expect that passing this flag should affect future
invocations of varobj commands. I know *I* wouldn't expect that
side-effect-type behavior.
I think a cleaner approach would be to remove the
--available-children-only option from -var-info-num-children and
-var-list-children and add a new command
[-var-show-available-children-only (?)] to allow the UI to flip this
property if it wants to.
The rest of the patch looks okay to me.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 06/12] Use varobj_is_dynamic_p more widely
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (7 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 07/12] MI option --available-children-only Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-24 18:17 ` Keith Seitz
2014-05-21 18:04 ` Tom Tromey
2014-02-14 8:46 ` [PATCH 02/12] Generalize varobj iterator Yao Qi
` (4 subsequent siblings)
13 siblings, 2 replies; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
Use varobj_is_dynamic_p more widely so that the callers of
varobj_is_dynamic_p are unchanged when we add available-children-only
stuff in varobj_is_dynamic_p.
gdb:
2014-02-14 Yao Qi <yao@codesourcery.com>
* varobj.c (varobj_get_num_children): Call
varobj_is_dynamic_p.
(varobj_list_children): Likewise.
(varobj_update): Likewise. Update comments.
---
gdb/varobj.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 66596b5..8905087 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -895,7 +895,7 @@ varobj_get_num_children (struct varobj *var)
{
if (var->num_children == -1)
{
- if (var->dynamic->pretty_printer != NULL)
+ if (varobj_is_dynamic_p (var))
{
int dummy;
@@ -922,7 +922,7 @@ varobj_list_children (struct varobj *var, int *from, int *to)
var->dynamic->children_requested = 1;
- if (var->dynamic->pretty_printer != NULL)
+ if (varobj_is_dynamic_p (var))
{
/* This, in theory, can result in the number of children changing without
frontend noticing. But well, calling -var-list-children on the same
@@ -1715,10 +1715,9 @@ varobj_update (struct varobj **varp, int explicit)
}
}
- /* We probably should not get children of a varobj that has a
- pretty-printer, but for which -var-list-children was never
- invoked. */
- if (v->dynamic->pretty_printer != NULL)
+ /* We probably should not get children of a dynamic varobj, but
+ for which -var-list-children was never invoked. */
+ if (varobj_is_dynamic_p (v))
{
VEC (varobj_p) *changed = 0, *type_changed = 0, *unchanged = 0;
VEC (varobj_p) *new = 0;
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH 02/12] Generalize varobj iterator
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (8 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 06/12] Use varobj_is_dynamic_p more widely Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-23 19:24 ` Keith Seitz
2014-05-21 17:51 ` Tom Tromey
2014-02-14 8:46 ` [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p Yao Qi
` (3 subsequent siblings)
13 siblings, 2 replies; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
This patch generalizes varobj iterator, in a python-independent way.
Note varobj_item is still a typedef of PyObject, we can only focus on
API changes, and leave the data type changes to the next patch. As a
result, we include "varobj-iter.h" after the typedef of PyObject in
varobj.c, but it is an intermediate state. Finally, varobj-iter.h is
independent of PyObject.
This change is helpful to move some python-related code out of
varobj.c.
V2:
- Fix a missing cleanup.
- Fix typos.
- Use XNEW.
- Check against NULL explicitly.
- Update copyright year for new added files.
gdb:
2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
(SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
(HFILES_NO_SRCDIR): Add "varobj-iter.h".
(py-varobj.o): New rule.
* python/py-varobj.c: New file.
* python/python-internal.h (py_varobj_get_iterator): Declare.
* varobj-iter.h: New file.
* varobj.c: Include "varobj-iter.h"
(struct varobj) <child_iter>: Change its type from "PyObject *"
to "struct varobj_iter *".
<saved_item>: Likewise.
[HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
[HAVE_PYTHON] (varobj_get_iterator): New function.
(update_dynamic_varobj_children) [HAVE_PYTHON]: Move
python-specific code to python/py-varobj.c.
(install_visualizer): Call varobj_iter_delete instead of
Py_XDECREF.
* varobj.h (varobj_ensure_python_env): Declare.
---
gdb/Makefile.in | 13 +++-
gdb/python/py-varobj.c | 183 ++++++++++++++++++++++++++++++++++++++++++
gdb/python/python-internal.h | 4 +
gdb/varobj-iter.h | 63 ++++++++++++++
gdb/varobj.c | 112 ++++++++------------------
gdb/varobj.h | 2 +
6 files changed, 297 insertions(+), 80 deletions(-)
create mode 100644 gdb/python/py-varobj.c
create mode 100644 gdb/varobj-iter.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ea56854..1da4055 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -369,7 +369,8 @@ SUBDIR_PYTHON_OBS = \
py-threadevent.o \
py-type.o \
py-utils.o \
- py-value.o
+ py-value.o \
+ py-varobj.o
SUBDIR_PYTHON_SRCS = \
python/python.c \
@@ -405,7 +406,8 @@ SUBDIR_PYTHON_SRCS = \
python/py-threadevent.c \
python/py-type.c \
python/py-utils.c \
- python/py-value.c
+ python/py-value.c \
+ python/py-varobj.c
SUBDIR_PYTHON_DEPS =
SUBDIR_PYTHON_LDFLAGS=
SUBDIR_PYTHON_CFLAGS=
@@ -856,7 +858,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
-ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
+ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \
+nto-tdep.h serial.h \
c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \
cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \
cli/cli-script.h macrotab.h symtab.h common/version.h \
@@ -2480,6 +2483,10 @@ py-value.o: $(srcdir)/python/py-value.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c
$(POSTCOMPILE)
+py-varobj.o: $(srcdir)/python/py-varobj.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c
+ $(POSTCOMPILE)
+
#
# Dependency tracking. Most of this is conditional on GNU Make being
# found by configure; if GNU Make is not found, we fall back to a
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
new file mode 100644
index 0000000..f4b95bd
--- /dev/null
+++ b/gdb/python/py-varobj.c
@@ -0,0 +1,183 @@
+/* Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "varobj.h"
+#include "varobj-iter.h"
+
+/* A dynamic varobj iterator "class" for python pretty-printed
+ varobjs. This inherits struct varobj_iter. */
+
+struct py_varobj_iter
+{
+ /* The 'base class'. */
+ struct varobj_iter base;
+
+ /* The python iterator returned by the printer's 'children' method,
+ or NULL if not available. */
+ PyObject *iter;
+};
+
+/* Implementation of the 'dtor' method of pretty-printed varobj
+ iterators. */
+
+static void
+py_varobj_iter_dtor (struct varobj_iter *self)
+{
+ struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
+
+ Py_XDECREF (dis->iter);
+}
+
+/* Implementation of the 'next' method of pretty-printed varobj
+ iterators. */
+
+static varobj_item *
+py_varobj_iter_next (struct varobj_iter *self)
+{
+ struct py_varobj_iter *t = (struct py_varobj_iter *) self;
+ struct cleanup *back_to;
+ PyObject *item;
+
+ back_to = varobj_ensure_python_env (self->var);
+
+ item = PyIter_Next (t->iter);
+
+ if (item == NULL)
+ {
+ /* Normal end of iteration. */
+ if (!PyErr_Occurred ())
+ return NULL;
+
+ /* If we got a memory error, just use the text as the item. */
+ if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
+ {
+ PyObject *type, *value, *trace;
+ char *name_str, *value_str;
+
+ PyErr_Fetch (&type, &value, &trace);
+ value_str = gdbpy_exception_to_string (type, value);
+ Py_XDECREF (type);
+ Py_XDECREF (value);
+ Py_XDECREF (trace);
+ if (value_str == NULL)
+ {
+ gdbpy_print_stack ();
+ return NULL;
+ }
+
+ name_str = xstrprintf ("<error at %d>",
+ self->next_raw_index++);
+ item = Py_BuildValue ("(ss)", name_str, value_str);
+ xfree (name_str);
+ xfree (value_str);
+ if (item == NULL)
+ {
+ gdbpy_print_stack ();
+ return NULL;
+ }
+ }
+ else
+ {
+ /* Any other kind of error. */
+ gdbpy_print_stack ();
+ return NULL;
+ }
+ }
+
+ self->next_raw_index++;
+ do_cleanups (back_to);
+ return item;
+}
+
+/* The 'vtable' of pretty-printed python varobj iterators. */
+
+static const struct varobj_iter_ops py_varobj_iter_ops =
+{
+ py_varobj_iter_dtor,
+ py_varobj_iter_next
+};
+
+/* Constructor of pretty-printed varobj iterators. VAR is the varobj
+ whose children the iterator will be iterating over. PYITER is the
+ python iterator actually responsible for the iteration. */
+
+static void
+py_varobj_iter_ctor (struct py_varobj_iter *self,
+ struct varobj *var, PyObject *pyiter)
+{
+ self->base.var = var;
+ self->base.ops = &py_varobj_iter_ops;
+ self->base.next_raw_index = 0;
+ self->iter = pyiter;
+}
+
+/* Allocate and construct a pretty-printed varobj iterator. VAR is
+ the varobj whose children the iterator will be iterating over.
+ PYITER is the python iterator actually responsible for the
+ iteration. */
+
+static struct py_varobj_iter *
+py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
+{
+ struct py_varobj_iter *self;
+
+ self = XNEW (struct py_varobj_iter);
+ py_varobj_iter_ctor (self, var, pyiter);
+ return self;
+}
+
+/* Return a new pretty-printed varobj iterator suitable to iterate
+ over VAR's children. */
+
+struct varobj_iter *
+py_varobj_get_iterator (struct varobj *var, PyObject *printer)
+{
+ PyObject *children;
+ int i;
+ PyObject *iter;
+ struct py_varobj_iter *py_iter;
+ struct cleanup *back_to = varobj_ensure_python_env (var);
+
+ if (!PyObject_HasAttr (printer, gdbpy_children_cst))
+ {
+ do_cleanups (back_to);
+ return NULL;
+ }
+
+ children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
+ NULL);
+ if (children == NULL)
+ {
+ gdbpy_print_stack ();
+ error (_("Null value returned for children"));
+ }
+
+ make_cleanup_py_decref (children);
+
+ iter = PyObject_GetIter (children);
+ if (iter == NULL)
+ {
+ gdbpy_print_stack ();
+ error (_("Could not get children iterator"));
+ }
+
+ py_iter = py_varobj_iter_new (var, iter);
+
+ do_cleanups (back_to);
+
+ return &py_iter->base;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 07650f7..5eae13f 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -520,4 +520,8 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
PyObject *object)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+struct varobj_iter;
+struct varobj;
+struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
+ PyObject *printer);
#endif /* GDB_PYTHON_INTERNAL_H */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
new file mode 100644
index 0000000..3a530bc
--- /dev/null
+++ b/gdb/varobj-iter.h
@@ -0,0 +1,63 @@
+/* Iterator of varobj.
+ Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+struct varobj_iter_ops;
+
+typedef PyObject varobj_item;
+
+/* A dynamic varobj iterator "class". */
+
+struct varobj_iter
+{
+ /* The 'vtable'. */
+ const struct varobj_iter_ops *ops;
+
+ /* The varobj this iterator is listing children for. */
+ struct varobj *var;
+
+ /* The next raw index we will try to check is available. If it is
+ equal to number_of_children, then we've already iterated the
+ whole set. */
+ int next_raw_index;
+};
+
+/* The vtable of the varobj iterator class. */
+
+struct varobj_iter_ops
+{
+ /* Destructor. Releases everything from SELF (but not SELF
+ itself). */
+ void (*dtor) (struct varobj_iter *self);
+
+ /* Returns the next object or NULL if it has reached the end. */
+ varobj_item *(*next) (struct varobj_iter *self);
+};
+
+/* Returns the next varobj or NULL if it has reached the end. */
+
+#define varobj_iter_next(ITER) (ITER)->ops->next (ITER)
+
+/* Delete a varobj_iter object. */
+
+#define varobj_iter_delete(ITER) \
+ do \
+ { \
+ if ((ITER) != NULL) \
+ { \
+ (ITER)->ops->dtor (ITER); \
+ xfree (ITER); \
+ } \
+ } while (0)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 68c54e3..d20c81e 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -41,6 +41,8 @@
typedef int PyObject;
#endif
+#include "varobj-iter.h"
+
/* Non-zero if we want to see trace of varobj level stuff. */
unsigned int varobjdebug = 0;
@@ -140,14 +142,14 @@ struct varobj_dynamic
/* The iterator returned by the printer's 'children' method, or NULL
if not available. */
- PyObject *child_iter;
+ struct varobj_iter *child_iter;
/* We request one extra item from the iterator, so that we can
report to the caller whether there are more items than we have
already reported. However, we don't want to install this value
when we read it, because that will mess up future updates. So,
we stash it here instead. */
- PyObject *saved_item;
+ varobj_item *saved_item;
};
struct cpstack
@@ -256,7 +258,7 @@ is_root_p (struct varobj *var)
#ifdef HAVE_PYTHON
/* Helper function to install a Python environment suitable for
use during operations on VAR. */
-static struct cleanup *
+struct cleanup *
varobj_ensure_python_env (struct varobj *var)
{
return ensure_python_env (var->root->exp->gdbarch,
@@ -773,6 +775,19 @@ dynamic_varobj_has_child_method (struct varobj *var)
return result;
}
+/* A factory for creating dynamic varobj's iterators. Returns an
+ iterator object suitable for iterating over VAR's children. */
+
+static struct varobj_iter *
+varobj_get_iterator (struct varobj *var)
+{
+ if (var->dynamic->pretty_printer)
+ return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
+
+ gdb_assert_not_reached (_("\
+requested an iterator from a non-dynamic varobj"));
+}
+
#endif
static int
@@ -788,9 +803,7 @@ update_dynamic_varobj_children (struct varobj *var,
{
#if HAVE_PYTHON
struct cleanup *back_to;
- PyObject *children;
int i;
- PyObject *printer = var->dynamic->pretty_printer;
if (!gdb_python_initialized)
return 0;
@@ -798,37 +811,22 @@ update_dynamic_varobj_children (struct varobj *var,
back_to = varobj_ensure_python_env (var);
*cchanged = 0;
- if (!PyObject_HasAttr (printer, gdbpy_children_cst))
- {
- do_cleanups (back_to);
- return 0;
- }
if (update_children || var->dynamic->child_iter == NULL)
{
- children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
- NULL);
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = varobj_get_iterator (var);
- if (!children)
- {
- gdbpy_print_stack ();
- error (_("Null value returned for children"));
- }
+ Py_XDECREF (var->dynamic->saved_item);
+ var->dynamic->saved_item = NULL;
- make_cleanup_py_decref (children);
+ i = 0;
- Py_XDECREF (var->dynamic->child_iter);
- var->dynamic->child_iter = PyObject_GetIter (children);
if (var->dynamic->child_iter == NULL)
{
- gdbpy_print_stack ();
- error (_("Could not get children iterator"));
+ do_cleanups (back_to);
+ return 0;
}
-
- Py_XDECREF (var->dynamic->saved_item);
- var->dynamic->saved_item = NULL;
-
- i = 0;
}
else
i = VEC_length (varobj_p, var->children);
@@ -838,7 +836,6 @@ update_dynamic_varobj_children (struct varobj *var,
for (; to < 0 || i < to + 1; ++i)
{
PyObject *item;
- int force_done = 0;
/* See if there was a leftover from last time. */
if (var->dynamic->saved_item)
@@ -847,52 +844,17 @@ update_dynamic_varobj_children (struct varobj *var,
var->dynamic->saved_item = NULL;
}
else
- item = PyIter_Next (var->dynamic->child_iter);
-
- if (!item)
{
- /* Normal end of iteration. */
- if (!PyErr_Occurred ())
- break;
-
- /* If we got a memory error, just use the text as the
- item. */
- if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
- {
- PyObject *type, *value, *trace;
- char *name_str, *value_str;
-
- PyErr_Fetch (&type, &value, &trace);
- value_str = gdbpy_exception_to_string (type, value);
- Py_XDECREF (type);
- Py_XDECREF (value);
- Py_XDECREF (trace);
- if (!value_str)
- {
- gdbpy_print_stack ();
- break;
- }
-
- name_str = xstrprintf ("<error at %d>", i);
- item = Py_BuildValue ("(ss)", name_str, value_str);
- xfree (name_str);
- xfree (value_str);
- if (!item)
- {
- gdbpy_print_stack ();
- break;
- }
-
- force_done = 1;
- }
- else
- {
- /* Any other kind of error. */
- gdbpy_print_stack ();
- break;
- }
+ item = varobj_iter_next (var->dynamic->child_iter);
}
+ if (item == NULL)
+ {
+ /* Iteration is done. Remove iterator from VAR. */
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = NULL;
+ break;
+ }
/* We don't want to push the extra child on any report list. */
if (to < 0 || i < to)
{
@@ -932,9 +894,6 @@ update_dynamic_varobj_children (struct varobj *var,
element. */
break;
}
-
- if (force_done)
- break;
}
if (i < VEC_length (varobj_p, var->children))
@@ -953,9 +912,8 @@ update_dynamic_varobj_children (struct varobj *var,
*cchanged = 1;
var->num_children = VEC_length (varobj_p, var->children);
-
- do_cleanups (back_to);
+ do_cleanups (back_to);
return 1;
#else
gdb_assert_not_reached ("should never be called if Python is not enabled");
@@ -1245,7 +1203,7 @@ install_visualizer (struct varobj_dynamic *var, PyObject *constructor,
Py_XDECREF (var->pretty_printer);
var->pretty_printer = visualizer;
- Py_XDECREF (var->child_iter);
+ varobj_iter_delete (var->child_iter);
var->child_iter = NULL;
}
diff --git a/gdb/varobj.h b/gdb/varobj.h
index e4007bf..d3d81bf 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to);
extern int varobj_pretty_printed_p (struct varobj *var);
+extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
+
extern int varobj_default_value_is_changeable_p (struct varobj *var);
extern int varobj_value_is_changeable_p (struct varobj *var);
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/12] Generalize varobj iterator
2014-02-14 8:46 ` [PATCH 02/12] Generalize varobj iterator Yao Qi
@ 2014-04-23 19:24 ` Keith Seitz
2014-05-21 17:51 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-23 19:24 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> V2:
> - Fix a missing cleanup.
> - Fix typos.
> - Use XNEW.
> - Check against NULL explicitly.
> - Update copyright year for new added files.
>
> gdb:
>
> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
> (SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
> (HFILES_NO_SRCDIR): Add "varobj-iter.h".
> (py-varobj.o): New rule.
> * python/py-varobj.c: New file.
> * python/python-internal.h (py_varobj_get_iterator): Declare.
> * varobj-iter.h: New file.
> * varobj.c: Include "varobj-iter.h"
> (struct varobj) <child_iter>: Change its type from "PyObject *"
> to "struct varobj_iter *".
> <saved_item>: Likewise.
> [HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
> [HAVE_PYTHON] (varobj_get_iterator): New function.
> (update_dynamic_varobj_children) [HAVE_PYTHON]: Move
> python-specific code to python/py-varobj.c.
> (install_visualizer): Call varobj_iter_delete instead of
> Py_XDECREF.
> * varobj.h (varobj_ensure_python_env): Declare.
This looks okay to me, too. I recommend a maintainer give this a final
review.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 02/12] Generalize varobj iterator
2014-02-14 8:46 ` [PATCH 02/12] Generalize varobj iterator Yao Qi
2014-04-23 19:24 ` Keith Seitz
@ 2014-05-21 17:51 ` Tom Tromey
2014-05-23 1:23 ` Yao Qi
1 sibling, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2014-05-21 17:51 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao> Yao Qi <yao@codesourcery.com>
Yao> * Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
Yao> (SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
Yao> (HFILES_NO_SRCDIR): Add "varobj-iter.h".
Yao> (py-varobj.o): New rule.
Yao> * python/py-varobj.c: New file.
Yao> * python/python-internal.h (py_varobj_get_iterator): Declare.
Yao> * varobj-iter.h: New file.
Yao> * varobj.c: Include "varobj-iter.h"
Yao> (struct varobj) <child_iter>: Change its type from "PyObject *"
Yao> to "struct varobj_iter *".
Yao> <saved_item>: Likewise.
Yao> [HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
Yao> [HAVE_PYTHON] (varobj_get_iterator): New function.
Yao> (update_dynamic_varobj_children) [HAVE_PYTHON]: Move
Yao> python-specific code to python/py-varobj.c.
Yao> (install_visualizer): Call varobj_iter_delete instead of
Yao> Py_XDECREF.
Yao> * varobj.h (varobj_ensure_python_env): Declare.
Yao> +static void
Yao> +py_varobj_iter_dtor (struct varobj_iter *self)
Yao> +{
Yao> + struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
Yao> +
Yao> + Py_XDECREF (dis->iter);
I think this has to acquire the GIL before calling Py_XDECREF.
Yao> +static void
Yao> +py_varobj_iter_ctor (struct py_varobj_iter *self,
Yao> + struct varobj *var, PyObject *pyiter)
Yao> +{
Yao> + self->base.var = var;
Yao> + self->base.ops = &py_varobj_iter_ops;
Yao> + self->base.next_raw_index = 0;
Yao> + self->iter = pyiter;
[...]
Yao> +static struct py_varobj_iter *
Yao> +py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
Yao> +{
[...]
Yao> + py_varobj_iter_ctor (self, var, pyiter);
I think these two functions should be documented as stealing a reference
to PYITER. It's helpful to see such comments later.
Using CPYCHECKER_STEALS_REFERENCE_TO_ARG would also be nice, but I
appreciate that this isn't easy for everybody to test.
Yao> + iter = PyObject_GetIter (children);
Yao> + if (iter == NULL)
Yao> + {
The "if" looks misindented.
Yao> +struct varobj_iter;
Yao> +struct varobj;
Yao> +struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
Yao> + PyObject *printer);
Yao> #endif /* GDB_PYTHON_INTERNAL_H */
Please leave a blank line before the #endif.
Yao> +extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
Extra space after "extern".
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/12] Generalize varobj iterator
2014-05-21 17:51 ` Tom Tromey
@ 2014-05-23 1:23 ` Yao Qi
2014-06-04 20:21 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-05-23 1:23 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 05/22/2014 01:51 AM, Tom Tromey wrote:
> Yao> +static void
> Yao> +py_varobj_iter_dtor (struct varobj_iter *self)
> Yao> +{
> Yao> + struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
> Yao> +
> Yao> + Py_XDECREF (dis->iter);
>
> I think this has to acquire the GIL before calling Py_XDECREF.
>
PyGILState_Ensure and PyGILState_Release are added.
> Yao> +static void
> Yao> +py_varobj_iter_ctor (struct py_varobj_iter *self,
> Yao> + struct varobj *var, PyObject *pyiter)
> Yao> +{
> Yao> + self->base.var = var;
> Yao> + self->base.ops = &py_varobj_iter_ops;
> Yao> + self->base.next_raw_index = 0;
> Yao> + self->iter = pyiter;
> [...]
> Yao> +static struct py_varobj_iter *
> Yao> +py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
> Yao> +{
> [...]
> Yao> + py_varobj_iter_ctor (self, var, pyiter);
>
> I think these two functions should be documented as stealing a reference
> to PYITER. It's helpful to see such comments later.
>
> Using CPYCHECKER_STEALS_REFERENCE_TO_ARG would also be nice, but I
> appreciate that this isn't easy for everybody to test.
I add CPYCHECKER_STEALS_REFERENCE_TO_ARG to these two functions, and
verified by cpychecker. Some errors go away.
>
> Yao> + iter = PyObject_GetIter (children);
> Yao> + if (iter == NULL)
> Yao> + {
>
> The "if" looks misindented.
>
Fixed.
> Yao> +struct varobj_iter;
> Yao> +struct varobj;
> Yao> +struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
> Yao> + PyObject *printer);
> Yao> #endif /* GDB_PYTHON_INTERNAL_H */
>
> Please leave a blank line before the #endif.
>
Fixed.
> Yao> +extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
>
> Extra space after "extern".
Fixed.
--
Yao (é½å°§)
This patch generalizes varobj iterator, in a python-independent way.
Note varobj_item is still a typedef of PyObject, we can only focus on
API changes, and leave the data type changes to the next patch. As a
result, we include "varobj-iter.h" after the typedef of PyObject in
varobj.c, but it is an intermediate state. Finally, varobj-iter.h is
independent of PyObject.
This change is helpful to move some python-related code out of
varobj.c.
V2:
- Fix a missing cleanup.
- Fix typos.
- Use XNEW.
- Check against NULL explicitly.
- Update copyright year for new added files.
V3:
- Call PyGILState_Ensure before Py_XDECREF.
- Use CPYCHECKER_STEALS_REFERENCE_TO_ARG.
- Code indentation.
gdb:
2014-05-22 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
(SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
(HFILES_NO_SRCDIR): Add "varobj-iter.h".
(py-varobj.o): New rule.
* python/py-varobj.c: New file.
* python/python-internal.h (py_varobj_get_iterator): Declare.
* varobj-iter.h: New file.
* varobj.c: Include "varobj-iter.h"
(struct varobj) <child_iter>: Change its type from "PyObject *"
to "struct varobj_iter *".
<saved_item>: Likewise.
[HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
[HAVE_PYTHON] (varobj_get_iterator): New function.
(update_dynamic_varobj_children) [HAVE_PYTHON]: Move
python-specific code to python/py-varobj.c.
(install_visualizer): Call varobj_iter_delete instead of
Py_XDECREF.
* varobj.h (varobj_ensure_python_env): Declare.
---
gdb/Makefile.in | 13 ++-
gdb/python/py-varobj.c | 185
+++++++++++++++++++++++++++++++++++++++++++
gdb/python/python-internal.h | 5 ++
gdb/varobj-iter.h | 63 +++++++++++++++
gdb/varobj.c | 112 ++++++++------------------
gdb/varobj.h | 2 +
6 files changed, 300 insertions(+), 80 deletions(-)
create mode 100644 gdb/python/py-varobj.c
create mode 100644 gdb/varobj-iter.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index f2c16ec..da6437b 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -369,7 +369,8 @@ SUBDIR_PYTHON_OBS = \
py-threadevent.o \
py-type.o \
py-utils.o \
- py-value.o
+ py-value.o \
+ py-varobj.o
SUBDIR_PYTHON_SRCS = \
python/python.c \
@@ -405,7 +406,8 @@ SUBDIR_PYTHON_SRCS = \
python/py-threadevent.c \
python/py-type.c \
python/py-utils.c \
- python/py-value.c
+ python/py-value.c \
+ python/py-varobj.c
SUBDIR_PYTHON_DEPS =
SUBDIR_PYTHON_LDFLAGS=
SUBDIR_PYTHON_CFLAGS=
@@ -857,7 +859,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h
ppcfbsd-tdep.h \
ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
-ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
+ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \
+nto-tdep.h serial.h \
c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h
cli/cli-setshow.h \
cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \
cli/cli-script.h macrotab.h symtab.h common/version.h \
@@ -2482,6 +2485,10 @@ py-value.o: $(srcdir)/python/py-value.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c
$(POSTCOMPILE)
+py-varobj.o: $(srcdir)/python/py-varobj.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c
+ $(POSTCOMPILE)
+
#
# Dependency tracking. Most of this is conditional on GNU Make being
# found by configure; if GNU Make is not found, we fall back to a
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
new file mode 100644
index 0000000..50bea40
--- /dev/null
+++ b/gdb/python/py-varobj.c
@@ -0,0 +1,185 @@
+/* Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see
<http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "varobj.h"
+#include "varobj-iter.h"
+
+/* A dynamic varobj iterator "class" for python pretty-printed
+ varobjs. This inherits struct varobj_iter. */
+
+struct py_varobj_iter
+{
+ /* The 'base class'. */
+ struct varobj_iter base;
+
+ /* The python iterator returned by the printer's 'children' method,
+ or NULL if not available. */
+ PyObject *iter;
+};
+
+/* Implementation of the 'dtor' method of pretty-printed varobj
+ iterators. */
+
+static void
+py_varobj_iter_dtor (struct varobj_iter *self)
+{
+ struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
+ PyGILState_STATE state = PyGILState_Ensure ();
+
+ Py_XDECREF (dis->iter);
+ PyGILState_Release (state);
+}
+
+/* Implementation of the 'next' method of pretty-printed varobj
+ iterators. */
+
+static varobj_item *
+py_varobj_iter_next (struct varobj_iter *self)
+{
+ struct py_varobj_iter *t = (struct py_varobj_iter *) self;
+ struct cleanup *back_to;
+ PyObject *item;
+
+ back_to = varobj_ensure_python_env (self->var);
+
+ item = PyIter_Next (t->iter);
+
+ if (item == NULL)
+ {
+ /* Normal end of iteration. */
+ if (!PyErr_Occurred ())
+ return NULL;
+
+ /* If we got a memory error, just use the text as the item. */
+ if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
+ {
+ PyObject *type, *value, *trace;
+ char *name_str, *value_str;
+
+ PyErr_Fetch (&type, &value, &trace);
+ value_str = gdbpy_exception_to_string (type, value);
+ Py_XDECREF (type);
+ Py_XDECREF (value);
+ Py_XDECREF (trace);
+ if (value_str == NULL)
+ {
+ gdbpy_print_stack ();
+ return NULL;
+ }
+
+ name_str = xstrprintf ("<error at %d>",
+ self->next_raw_index++);
+ item = Py_BuildValue ("(ss)", name_str, value_str);
+ xfree (name_str);
+ xfree (value_str);
+ if (item == NULL)
+ {
+ gdbpy_print_stack ();
+ return NULL;
+ }
+ }
+ else
+ {
+ /* Any other kind of error. */
+ gdbpy_print_stack ();
+ return NULL;
+ }
+ }
+
+ self->next_raw_index++;
+ do_cleanups (back_to);
+ return item;
+}
+
+/* The 'vtable' of pretty-printed python varobj iterators. */
+
+static const struct varobj_iter_ops py_varobj_iter_ops =
+{
+ py_varobj_iter_dtor,
+ py_varobj_iter_next
+};
+
+/* Constructor of pretty-printed varobj iterators. VAR is the varobj
+ whose children the iterator will be iterating over. PYITER is the
+ python iterator actually responsible for the iteration. */
+
+static void CPYCHECKER_STEALS_REFERENCE_TO_ARG (3)
+py_varobj_iter_ctor (struct py_varobj_iter *self,
+ struct varobj *var, PyObject *pyiter)
+{
+ self->base.var = var;
+ self->base.ops = &py_varobj_iter_ops;
+ self->base.next_raw_index = 0;
+ self->iter = pyiter;
+}
+
+/* Allocate and construct a pretty-printed varobj iterator. VAR is
+ the varobj whose children the iterator will be iterating over.
+ PYITER is the python iterator actually responsible for the
+ iteration. */
+
+static struct py_varobj_iter * CPYCHECKER_STEALS_REFERENCE_TO_ARG (2)
+py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
+{
+ struct py_varobj_iter *self;
+
+ self = XNEW (struct py_varobj_iter);
+ py_varobj_iter_ctor (self, var, pyiter);
+ return self;
+}
+
+/* Return a new pretty-printed varobj iterator suitable to iterate
+ over VAR's children. */
+
+struct varobj_iter *
+py_varobj_get_iterator (struct varobj *var, PyObject *printer)
+{
+ PyObject *children;
+ int i;
+ PyObject *iter;
+ struct py_varobj_iter *py_iter;
+ struct cleanup *back_to = varobj_ensure_python_env (var);
+
+ if (!PyObject_HasAttr (printer, gdbpy_children_cst))
+ {
+ do_cleanups (back_to);
+ return NULL;
+ }
+
+ children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
+ NULL);
+ if (children == NULL)
+ {
+ gdbpy_print_stack ();
+ error (_("Null value returned for children"));
+ }
+
+ make_cleanup_py_decref (children);
+
+ iter = PyObject_GetIter (children);
+ if (iter == NULL)
+ {
+ gdbpy_print_stack ();
+ error (_("Could not get children iterator"));
+ }
+
+ py_iter = py_varobj_iter_new (var, iter);
+
+ do_cleanups (back_to);
+
+ return &py_iter->base;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 07650f7..3aab045 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -520,4 +520,9 @@ int gdb_pymodule_addobject (PyObject *module, const
char *name,
PyObject *object)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+struct varobj_iter;
+struct varobj;
+struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
+ PyObject *printer);
+
#endif /* GDB_PYTHON_INTERNAL_H */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
new file mode 100644
index 0000000..3a530bc
--- /dev/null
+++ b/gdb/varobj-iter.h
@@ -0,0 +1,63 @@
+/* Iterator of varobj.
+ Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see
<http://www.gnu.org/licenses/>. */
+
+struct varobj_iter_ops;
+
+typedef PyObject varobj_item;
+
+/* A dynamic varobj iterator "class". */
+
+struct varobj_iter
+{
+ /* The 'vtable'. */
+ const struct varobj_iter_ops *ops;
+
+ /* The varobj this iterator is listing children for. */
+ struct varobj *var;
+
+ /* The next raw index we will try to check is available. If it is
+ equal to number_of_children, then we've already iterated the
+ whole set. */
+ int next_raw_index;
+};
+
+/* The vtable of the varobj iterator class. */
+
+struct varobj_iter_ops
+{
+ /* Destructor. Releases everything from SELF (but not SELF
+ itself). */
+ void (*dtor) (struct varobj_iter *self);
+
+ /* Returns the next object or NULL if it has reached the end. */
+ varobj_item *(*next) (struct varobj_iter *self);
+};
+
+/* Returns the next varobj or NULL if it has reached the end. */
+
+#define varobj_iter_next(ITER) (ITER)->ops->next (ITER)
+
+/* Delete a varobj_iter object. */
+
+#define varobj_iter_delete(ITER) \
+ do \
+ { \
+ if ((ITER) != NULL) \
+ { \
+ (ITER)->ops->dtor (ITER); \
+ xfree (ITER); \
+ } \
+ } while (0)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 0467a79..02cce5a 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -41,6 +41,8 @@
typedef int PyObject;
#endif
+#include "varobj-iter.h"
+
/* Non-zero if we want to see trace of varobj level stuff. */
unsigned int varobjdebug = 0;
@@ -140,14 +142,14 @@ struct varobj_dynamic
/* The iterator returned by the printer's 'children' method, or NULL
if not available. */
- PyObject *child_iter;
+ struct varobj_iter *child_iter;
/* We request one extra item from the iterator, so that we can
report to the caller whether there are more items than we have
already reported. However, we don't want to install this value
when we read it, because that will mess up future updates. So,
we stash it here instead. */
- PyObject *saved_item;
+ varobj_item *saved_item;
};
struct cpstack
@@ -256,7 +258,7 @@ is_root_p (struct varobj *var)
#ifdef HAVE_PYTHON
/* Helper function to install a Python environment suitable for
use during operations on VAR. */
-static struct cleanup *
+struct cleanup *
varobj_ensure_python_env (struct varobj *var)
{
return ensure_python_env (var->root->exp->gdbarch,
@@ -773,6 +775,19 @@ dynamic_varobj_has_child_method (struct varobj *var)
return result;
}
+/* A factory for creating dynamic varobj's iterators. Returns an
+ iterator object suitable for iterating over VAR's children. */
+
+static struct varobj_iter *
+varobj_get_iterator (struct varobj *var)
+{
+ if (var->dynamic->pretty_printer)
+ return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
+
+ gdb_assert_not_reached (_("\
+requested an iterator from a non-dynamic varobj"));
+}
+
#endif
static int
@@ -788,9 +803,7 @@ update_dynamic_varobj_children (struct varobj *var,
{
#if HAVE_PYTHON
struct cleanup *back_to;
- PyObject *children;
int i;
- PyObject *printer = var->dynamic->pretty_printer;
if (!gdb_python_initialized)
return 0;
@@ -798,37 +811,22 @@ update_dynamic_varobj_children (struct varobj *var,
back_to = varobj_ensure_python_env (var);
*cchanged = 0;
- if (!PyObject_HasAttr (printer, gdbpy_children_cst))
- {
- do_cleanups (back_to);
- return 0;
- }
if (update_children || var->dynamic->child_iter == NULL)
{
- children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
- NULL);
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = varobj_get_iterator (var);
- if (!children)
- {
- gdbpy_print_stack ();
- error (_("Null value returned for children"));
- }
+ Py_XDECREF (var->dynamic->saved_item);
+ var->dynamic->saved_item = NULL;
- make_cleanup_py_decref (children);
+ i = 0;
- Py_XDECREF (var->dynamic->child_iter);
- var->dynamic->child_iter = PyObject_GetIter (children);
if (var->dynamic->child_iter == NULL)
{
- gdbpy_print_stack ();
- error (_("Could not get children iterator"));
+ do_cleanups (back_to);
+ return 0;
}
-
- Py_XDECREF (var->dynamic->saved_item);
- var->dynamic->saved_item = NULL;
-
- i = 0;
}
else
i = VEC_length (varobj_p, var->children);
@@ -838,7 +836,6 @@ update_dynamic_varobj_children (struct varobj *var,
for (; to < 0 || i < to + 1; ++i)
{
PyObject *item;
- int force_done = 0;
/* See if there was a leftover from last time. */
if (var->dynamic->saved_item)
@@ -847,52 +844,17 @@ update_dynamic_varobj_children (struct varobj *var,
var->dynamic->saved_item = NULL;
}
else
- item = PyIter_Next (var->dynamic->child_iter);
-
- if (!item)
{
- /* Normal end of iteration. */
- if (!PyErr_Occurred ())
- break;
-
- /* If we got a memory error, just use the text as the
- item. */
- if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
- {
- PyObject *type, *value, *trace;
- char *name_str, *value_str;
-
- PyErr_Fetch (&type, &value, &trace);
- value_str = gdbpy_exception_to_string (type, value);
- Py_XDECREF (type);
- Py_XDECREF (value);
- Py_XDECREF (trace);
- if (!value_str)
- {
- gdbpy_print_stack ();
- break;
- }
-
- name_str = xstrprintf ("<error at %d>", i);
- item = Py_BuildValue ("(ss)", name_str, value_str);
- xfree (name_str);
- xfree (value_str);
- if (!item)
- {
- gdbpy_print_stack ();
- break;
- }
-
- force_done = 1;
- }
- else
- {
- /* Any other kind of error. */
- gdbpy_print_stack ();
- break;
- }
+ item = varobj_iter_next (var->dynamic->child_iter);
}
+ if (item == NULL)
+ {
+ /* Iteration is done. Remove iterator from VAR. */
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = NULL;
+ break;
+ }
/* We don't want to push the extra child on any report list. */
if (to < 0 || i < to)
{
@@ -932,9 +894,6 @@ update_dynamic_varobj_children (struct varobj *var,
element. */
break;
}
-
- if (force_done)
- break;
}
if (i < VEC_length (varobj_p, var->children))
@@ -953,9 +912,8 @@ update_dynamic_varobj_children (struct varobj *var,
*cchanged = 1;
var->num_children = VEC_length (varobj_p, var->children);
-
- do_cleanups (back_to);
+ do_cleanups (back_to);
return 1;
#else
gdb_assert_not_reached ("should never be called if Python is not
enabled");
@@ -1245,7 +1203,7 @@ install_visualizer (struct varobj_dynamic *var,
PyObject *constructor,
Py_XDECREF (var->pretty_printer);
var->pretty_printer = visualizer;
- Py_XDECREF (var->child_iter);
+ varobj_iter_delete (var->child_iter);
var->child_iter = NULL;
}
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 1199e0b..991c2e8 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to);
extern int varobj_pretty_printed_p (struct varobj *var);
+extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
+
extern int varobj_default_value_is_changeable_p (struct varobj *var);
extern int varobj_value_is_changeable_p (struct varobj *var);
--
1.9.0
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/12] Generalize varobj iterator
2014-05-23 1:23 ` Yao Qi
@ 2014-06-04 20:21 ` Tom Tromey
2014-06-05 5:36 ` Yao Qi
0 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2014-06-04 20:21 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> On 05/22/2014 01:51 AM, Tom Tromey wrote:
Yao> +static void
Yao> +py_varobj_iter_dtor (struct varobj_iter *self)
Yao> +{
Yao> + struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
Yao> +
Yao> + Py_XDECREF (dis->iter);
Tom> I think this has to acquire the GIL before calling Py_XDECREF.
Yao> PyGILState_Ensure and PyGILState_Release are added.
I think all transitions from gdb->python should use ensure_python_env.
That way the other things needed by gdb's python layer are guaranteed to
be set.
In this situation I think it could potentially cause a problem if
Py_XDECREF calls a user-written __del__ method somewhere.
thanks,
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/12] Generalize varobj iterator
2014-06-04 20:21 ` Tom Tromey
@ 2014-06-05 5:36 ` Yao Qi
2014-06-05 18:21 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-06-05 5:36 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 06/05/2014 04:20 AM, Tom Tromey wrote:
> Tom> I think this has to acquire the GIL before calling Py_XDECREF.
>
> Yao> PyGILState_Ensure and PyGILState_Release are added.
>
> I think all transitions from gdb->python should use ensure_python_env.
> That way the other things needed by gdb's python layer are guaranteed to
> be set.
>
> In this situation I think it could potentially cause a problem if
> Py_XDECREF calls a user-written __del__ method somewhere.
Ah, you are right. Call varobj_ensure_python_env in
py_varobj_iter_dtor then.
--
Yao (é½å°§)
Subject: [PATCH] Generalize varobj iterator
This patch generalizes varobj iterator, in a python-independent way.
Note varobj_item is still a typedef of PyObject, we can only focus on
API changes, and leave the data type changes to the next patch. As a
result, we include "varobj-iter.h" after the typedef of PyObject in
varobj.c, but it is an intermediate state. Finally, varobj-iter.h is
independent of PyObject.
This change is helpful to move some python-related code out of
varobj.c.
V2:
- Fix a missing cleanup.
- Fix typos.
- Use XNEW.
- Check against NULL explicitly.
- Update copyright year for new added files.
V3:
- Call PyGILState_Ensure before Py_XDECREF.
- Use CPYCHECKER_STEALS_REFERENCE_TO_ARG.
- Code indentation.
V4:
- use varobj_ensure_python_env instead of PyGILState_Ensure.
gdb:
2014-06-05 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
(SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
(HFILES_NO_SRCDIR): Add "varobj-iter.h".
(py-varobj.o): New rule.
* python/py-varobj.c: New file.
* python/python-internal.h (py_varobj_get_iterator): Declare.
* varobj-iter.h: New file.
* varobj.c: Include "varobj-iter.h"
(struct varobj) <child_iter>: Change its type from "PyObject *"
to "struct varobj_iter *".
<saved_item>: Likewise.
[HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
[HAVE_PYTHON] (varobj_get_iterator): New function.
(update_dynamic_varobj_children) [HAVE_PYTHON]: Move
python-specific code to python/py-varobj.c.
(install_visualizer): Call varobj_iter_delete instead of
Py_XDECREF.
* varobj.h (varobj_ensure_python_env): Declare.
---
gdb/Makefile.in | 13 ++-
gdb/python/py-varobj.c | 186 +++++++++++++++++++++++++++++++++++++++++++
gdb/python/python-internal.h | 5 ++
gdb/varobj-iter.h | 63 +++++++++++++++
gdb/varobj.c | 112 ++++++++------------------
gdb/varobj.h | 2 +
6 files changed, 301 insertions(+), 80 deletions(-)
create mode 100644 gdb/python/py-varobj.c
create mode 100644 gdb/varobj-iter.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e6c6c33..2158120 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -376,7 +376,8 @@ SUBDIR_PYTHON_OBS = \
py-threadevent.o \
py-type.o \
py-utils.o \
- py-value.o
+ py-value.o \
+ py-varobj.o
SUBDIR_PYTHON_SRCS = \
python/python.c \
@@ -413,7 +414,8 @@ SUBDIR_PYTHON_SRCS = \
python/py-threadevent.c \
python/py-type.c \
python/py-utils.c \
- python/py-value.c
+ python/py-value.c \
+ python/py-varobj.c
SUBDIR_PYTHON_DEPS =
SUBDIR_PYTHON_LDFLAGS=
SUBDIR_PYTHON_CFLAGS=
@@ -865,7 +867,8 @@ proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
i386bsd-nat.h xml-support.h xml-tdesc.h alphabsd-tdep.h gdb_obstack.h \
-ia64-tdep.h ada-lang.h varobj.h frv-tdep.h nto-tdep.h serial.h \
+ia64-tdep.h ada-lang.h ada-varobj.h varobj.h varobj-iter.h frv-tdep.h \
+nto-tdep.h serial.h \
c-lang.h d-lang.h go-lang.h frame.h event-loop.h block.h cli/cli-setshow.h \
cli/cli-decode.h cli/cli-cmds.h cli/cli-utils.h \
cli/cli-script.h macrotab.h symtab.h common/version.h \
@@ -2506,6 +2509,10 @@ py-value.o: $(srcdir)/python/py-value.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-value.c
$(POSTCOMPILE)
+py-varobj.o: $(srcdir)/python/py-varobj.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-varobj.c
+ $(POSTCOMPILE)
+
#
# Dependency tracking. Most of this is conditional on GNU Make being
# found by configure; if GNU Make is not found, we fall back to a
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
new file mode 100644
index 0000000..2b95348
--- /dev/null
+++ b/gdb/python/py-varobj.c
@@ -0,0 +1,186 @@
+/* Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "varobj.h"
+#include "varobj-iter.h"
+
+/* A dynamic varobj iterator "class" for python pretty-printed
+ varobjs. This inherits struct varobj_iter. */
+
+struct py_varobj_iter
+{
+ /* The 'base class'. */
+ struct varobj_iter base;
+
+ /* The python iterator returned by the printer's 'children' method,
+ or NULL if not available. */
+ PyObject *iter;
+};
+
+/* Implementation of the 'dtor' method of pretty-printed varobj
+ iterators. */
+
+static void
+py_varobj_iter_dtor (struct varobj_iter *self)
+{
+ struct py_varobj_iter *dis = (struct py_varobj_iter *) self;
+ struct cleanup *back_to = varobj_ensure_python_env (self->var);
+
+ Py_XDECREF (dis->iter);
+
+ do_cleanups (back_to);
+}
+
+/* Implementation of the 'next' method of pretty-printed varobj
+ iterators. */
+
+static varobj_item *
+py_varobj_iter_next (struct varobj_iter *self)
+{
+ struct py_varobj_iter *t = (struct py_varobj_iter *) self;
+ struct cleanup *back_to;
+ PyObject *item;
+
+ back_to = varobj_ensure_python_env (self->var);
+
+ item = PyIter_Next (t->iter);
+
+ if (item == NULL)
+ {
+ /* Normal end of iteration. */
+ if (!PyErr_Occurred ())
+ return NULL;
+
+ /* If we got a memory error, just use the text as the item. */
+ if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
+ {
+ PyObject *type, *value, *trace;
+ char *name_str, *value_str;
+
+ PyErr_Fetch (&type, &value, &trace);
+ value_str = gdbpy_exception_to_string (type, value);
+ Py_XDECREF (type);
+ Py_XDECREF (value);
+ Py_XDECREF (trace);
+ if (value_str == NULL)
+ {
+ gdbpy_print_stack ();
+ return NULL;
+ }
+
+ name_str = xstrprintf ("<error at %d>",
+ self->next_raw_index++);
+ item = Py_BuildValue ("(ss)", name_str, value_str);
+ xfree (name_str);
+ xfree (value_str);
+ if (item == NULL)
+ {
+ gdbpy_print_stack ();
+ return NULL;
+ }
+ }
+ else
+ {
+ /* Any other kind of error. */
+ gdbpy_print_stack ();
+ return NULL;
+ }
+ }
+
+ self->next_raw_index++;
+ do_cleanups (back_to);
+ return item;
+}
+
+/* The 'vtable' of pretty-printed python varobj iterators. */
+
+static const struct varobj_iter_ops py_varobj_iter_ops =
+{
+ py_varobj_iter_dtor,
+ py_varobj_iter_next
+};
+
+/* Constructor of pretty-printed varobj iterators. VAR is the varobj
+ whose children the iterator will be iterating over. PYITER is the
+ python iterator actually responsible for the iteration. */
+
+static void CPYCHECKER_STEALS_REFERENCE_TO_ARG (3)
+py_varobj_iter_ctor (struct py_varobj_iter *self,
+ struct varobj *var, PyObject *pyiter)
+{
+ self->base.var = var;
+ self->base.ops = &py_varobj_iter_ops;
+ self->base.next_raw_index = 0;
+ self->iter = pyiter;
+}
+
+/* Allocate and construct a pretty-printed varobj iterator. VAR is
+ the varobj whose children the iterator will be iterating over.
+ PYITER is the python iterator actually responsible for the
+ iteration. */
+
+static struct py_varobj_iter * CPYCHECKER_STEALS_REFERENCE_TO_ARG (2)
+py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
+{
+ struct py_varobj_iter *self;
+
+ self = XNEW (struct py_varobj_iter);
+ py_varobj_iter_ctor (self, var, pyiter);
+ return self;
+}
+
+/* Return a new pretty-printed varobj iterator suitable to iterate
+ over VAR's children. */
+
+struct varobj_iter *
+py_varobj_get_iterator (struct varobj *var, PyObject *printer)
+{
+ PyObject *children;
+ int i;
+ PyObject *iter;
+ struct py_varobj_iter *py_iter;
+ struct cleanup *back_to = varobj_ensure_python_env (var);
+
+ if (!PyObject_HasAttr (printer, gdbpy_children_cst))
+ {
+ do_cleanups (back_to);
+ return NULL;
+ }
+
+ children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
+ NULL);
+ if (children == NULL)
+ {
+ gdbpy_print_stack ();
+ error (_("Null value returned for children"));
+ }
+
+ make_cleanup_py_decref (children);
+
+ iter = PyObject_GetIter (children);
+ if (iter == NULL)
+ {
+ gdbpy_print_stack ();
+ error (_("Could not get children iterator"));
+ }
+
+ py_iter = py_varobj_iter_new (var, iter);
+
+ do_cleanups (back_to);
+
+ return &py_iter->base;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 9c06621..fed2eca 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -542,4 +542,9 @@ int gdb_pymodule_addobject (PyObject *module, const char *name,
PyObject *object)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+struct varobj_iter;
+struct varobj;
+struct varobj_iter *py_varobj_get_iterator (struct varobj *var,
+ PyObject *printer);
+
#endif /* GDB_PYTHON_INTERNAL_H */
diff --git a/gdb/varobj-iter.h b/gdb/varobj-iter.h
new file mode 100644
index 0000000..3a530bc
--- /dev/null
+++ b/gdb/varobj-iter.h
@@ -0,0 +1,63 @@
+/* Iterator of varobj.
+ Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+struct varobj_iter_ops;
+
+typedef PyObject varobj_item;
+
+/* A dynamic varobj iterator "class". */
+
+struct varobj_iter
+{
+ /* The 'vtable'. */
+ const struct varobj_iter_ops *ops;
+
+ /* The varobj this iterator is listing children for. */
+ struct varobj *var;
+
+ /* The next raw index we will try to check is available. If it is
+ equal to number_of_children, then we've already iterated the
+ whole set. */
+ int next_raw_index;
+};
+
+/* The vtable of the varobj iterator class. */
+
+struct varobj_iter_ops
+{
+ /* Destructor. Releases everything from SELF (but not SELF
+ itself). */
+ void (*dtor) (struct varobj_iter *self);
+
+ /* Returns the next object or NULL if it has reached the end. */
+ varobj_item *(*next) (struct varobj_iter *self);
+};
+
+/* Returns the next varobj or NULL if it has reached the end. */
+
+#define varobj_iter_next(ITER) (ITER)->ops->next (ITER)
+
+/* Delete a varobj_iter object. */
+
+#define varobj_iter_delete(ITER) \
+ do \
+ { \
+ if ((ITER) != NULL) \
+ { \
+ (ITER)->ops->dtor (ITER); \
+ xfree (ITER); \
+ } \
+ } while (0)
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 0467a79..02cce5a 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -41,6 +41,8 @@
typedef int PyObject;
#endif
+#include "varobj-iter.h"
+
/* Non-zero if we want to see trace of varobj level stuff. */
unsigned int varobjdebug = 0;
@@ -140,14 +142,14 @@ struct varobj_dynamic
/* The iterator returned by the printer's 'children' method, or NULL
if not available. */
- PyObject *child_iter;
+ struct varobj_iter *child_iter;
/* We request one extra item from the iterator, so that we can
report to the caller whether there are more items than we have
already reported. However, we don't want to install this value
when we read it, because that will mess up future updates. So,
we stash it here instead. */
- PyObject *saved_item;
+ varobj_item *saved_item;
};
struct cpstack
@@ -256,7 +258,7 @@ is_root_p (struct varobj *var)
#ifdef HAVE_PYTHON
/* Helper function to install a Python environment suitable for
use during operations on VAR. */
-static struct cleanup *
+struct cleanup *
varobj_ensure_python_env (struct varobj *var)
{
return ensure_python_env (var->root->exp->gdbarch,
@@ -773,6 +775,19 @@ dynamic_varobj_has_child_method (struct varobj *var)
return result;
}
+/* A factory for creating dynamic varobj's iterators. Returns an
+ iterator object suitable for iterating over VAR's children. */
+
+static struct varobj_iter *
+varobj_get_iterator (struct varobj *var)
+{
+ if (var->dynamic->pretty_printer)
+ return py_varobj_get_iterator (var, var->dynamic->pretty_printer);
+
+ gdb_assert_not_reached (_("\
+requested an iterator from a non-dynamic varobj"));
+}
+
#endif
static int
@@ -788,9 +803,7 @@ update_dynamic_varobj_children (struct varobj *var,
{
#if HAVE_PYTHON
struct cleanup *back_to;
- PyObject *children;
int i;
- PyObject *printer = var->dynamic->pretty_printer;
if (!gdb_python_initialized)
return 0;
@@ -798,37 +811,22 @@ update_dynamic_varobj_children (struct varobj *var,
back_to = varobj_ensure_python_env (var);
*cchanged = 0;
- if (!PyObject_HasAttr (printer, gdbpy_children_cst))
- {
- do_cleanups (back_to);
- return 0;
- }
if (update_children || var->dynamic->child_iter == NULL)
{
- children = PyObject_CallMethodObjArgs (printer, gdbpy_children_cst,
- NULL);
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = varobj_get_iterator (var);
- if (!children)
- {
- gdbpy_print_stack ();
- error (_("Null value returned for children"));
- }
+ Py_XDECREF (var->dynamic->saved_item);
+ var->dynamic->saved_item = NULL;
- make_cleanup_py_decref (children);
+ i = 0;
- Py_XDECREF (var->dynamic->child_iter);
- var->dynamic->child_iter = PyObject_GetIter (children);
if (var->dynamic->child_iter == NULL)
{
- gdbpy_print_stack ();
- error (_("Could not get children iterator"));
+ do_cleanups (back_to);
+ return 0;
}
-
- Py_XDECREF (var->dynamic->saved_item);
- var->dynamic->saved_item = NULL;
-
- i = 0;
}
else
i = VEC_length (varobj_p, var->children);
@@ -838,7 +836,6 @@ update_dynamic_varobj_children (struct varobj *var,
for (; to < 0 || i < to + 1; ++i)
{
PyObject *item;
- int force_done = 0;
/* See if there was a leftover from last time. */
if (var->dynamic->saved_item)
@@ -847,52 +844,17 @@ update_dynamic_varobj_children (struct varobj *var,
var->dynamic->saved_item = NULL;
}
else
- item = PyIter_Next (var->dynamic->child_iter);
-
- if (!item)
{
- /* Normal end of iteration. */
- if (!PyErr_Occurred ())
- break;
-
- /* If we got a memory error, just use the text as the
- item. */
- if (PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
- {
- PyObject *type, *value, *trace;
- char *name_str, *value_str;
-
- PyErr_Fetch (&type, &value, &trace);
- value_str = gdbpy_exception_to_string (type, value);
- Py_XDECREF (type);
- Py_XDECREF (value);
- Py_XDECREF (trace);
- if (!value_str)
- {
- gdbpy_print_stack ();
- break;
- }
-
- name_str = xstrprintf ("<error at %d>", i);
- item = Py_BuildValue ("(ss)", name_str, value_str);
- xfree (name_str);
- xfree (value_str);
- if (!item)
- {
- gdbpy_print_stack ();
- break;
- }
-
- force_done = 1;
- }
- else
- {
- /* Any other kind of error. */
- gdbpy_print_stack ();
- break;
- }
+ item = varobj_iter_next (var->dynamic->child_iter);
}
+ if (item == NULL)
+ {
+ /* Iteration is done. Remove iterator from VAR. */
+ varobj_iter_delete (var->dynamic->child_iter);
+ var->dynamic->child_iter = NULL;
+ break;
+ }
/* We don't want to push the extra child on any report list. */
if (to < 0 || i < to)
{
@@ -932,9 +894,6 @@ update_dynamic_varobj_children (struct varobj *var,
element. */
break;
}
-
- if (force_done)
- break;
}
if (i < VEC_length (varobj_p, var->children))
@@ -953,9 +912,8 @@ update_dynamic_varobj_children (struct varobj *var,
*cchanged = 1;
var->num_children = VEC_length (varobj_p, var->children);
-
- do_cleanups (back_to);
+ do_cleanups (back_to);
return 1;
#else
gdb_assert_not_reached ("should never be called if Python is not enabled");
@@ -1245,7 +1203,7 @@ install_visualizer (struct varobj_dynamic *var, PyObject *constructor,
Py_XDECREF (var->pretty_printer);
var->pretty_printer = visualizer;
- Py_XDECREF (var->child_iter);
+ varobj_iter_delete (var->child_iter);
var->child_iter = NULL;
}
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 1199e0b..991c2e8 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -308,6 +308,8 @@ extern int varobj_has_more (struct varobj *var, int to);
extern int varobj_pretty_printed_p (struct varobj *var);
+extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
+
extern int varobj_default_value_is_changeable_p (struct varobj *var);
extern int varobj_value_is_changeable_p (struct varobj *var);
--
1.9.0
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 02/12] Generalize varobj iterator
2014-06-05 5:36 ` Yao Qi
@ 2014-06-05 18:21 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2014-06-05 18:21 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> Ah, you are right. Call varobj_ensure_python_env in
Yao> py_varobj_iter_dtor then.
Thanks.
Yao> 2014-06-05 Pedro Alves <pedro@codesourcery.com>
Yao> Yao Qi <yao@codesourcery.com>
Yao> * Makefile.in (SUBDIR_PYTHON_OBS): Add "py-varobj.o".
Yao> (SUBDIR_PYTHON_SRCS): Add "python/py-varobj.c".
Yao> (HFILES_NO_SRCDIR): Add "varobj-iter.h".
Yao> (py-varobj.o): New rule.
Yao> * python/py-varobj.c: New file.
Yao> * python/python-internal.h (py_varobj_get_iterator): Declare.
Yao> * varobj-iter.h: New file.
Yao> * varobj.c: Include "varobj-iter.h"
Yao> (struct varobj) <child_iter>: Change its type from "PyObject *"
Yao> to "struct varobj_iter *".
Yao> <saved_item>: Likewise.
Yao> [HAVE_PYTHON] (varobj_ensure_python_env): Make it extern.
Yao> [HAVE_PYTHON] (varobj_get_iterator): New function.
Yao> (update_dynamic_varobj_children) [HAVE_PYTHON]: Move
Yao> python-specific code to python/py-varobj.c.
Yao> (install_visualizer): Call varobj_iter_delete instead of
Yao> Py_XDECREF.
Yao> * varobj.h (varobj_ensure_python_env): Declare.
This is ok.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (9 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 02/12] Generalize varobj iterator Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-04-24 17:39 ` Keith Seitz
2014-05-21 18:02 ` Tom Tromey
2014-02-14 8:46 ` [PATCH 12/12] NEWS and Doc on --available-children-only Yao Qi
` (2 subsequent siblings)
13 siblings, 2 replies; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
We think varobj with --available-children-only behaves like a dynamic
varobj, so dyanmic varobj is not pretty-printer specific. We rename
varobj_pretty_printed_p to varobj_is_dynamic_p, so that we can handle
available-children-only checking in varobj_is_dynamic_p in the next
patch.
gdb:
2014-02-14 Yao Qi <yao@codesourcery.com>
* varobj.c (varobj_pretty_printed_p): Rename to ...
(varobj_is_dynamic_p): ... this. New function.
* varobj.h (varobj_pretty_printed_p): Remove declaration.
(varobj_is_dynamic_p): Declare.
* mi/mi-cmd-var.c (print_varobj): All callers updated.
(mi_print_value_p, varobj_update_one): Likewise.
---
gdb/mi/mi-cmd-var.c | 6 +++---
gdb/varobj.c | 4 +++-
gdb/varobj.h | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index f68dba6..9059616 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -88,7 +88,7 @@ print_varobj (struct varobj *var, enum print_values print_values,
xfree (display_hint);
}
- if (varobj_pretty_printed_p (var))
+ if (varobj_is_dynamic_p (var))
ui_out_field_int (uiout, "dynamic", 1);
}
@@ -352,7 +352,7 @@ mi_print_value_p (struct varobj *var, enum print_values print_values)
if (print_values == PRINT_ALL_VALUES)
return 1;
- if (varobj_pretty_printed_p (var))
+ if (varobj_is_dynamic_p (var))
return 1;
type = varobj_get_gdb_type (var);
@@ -777,7 +777,7 @@ varobj_update_one (struct varobj *var, enum print_values print_values,
xfree (display_hint);
}
- if (varobj_pretty_printed_p (r->varobj))
+ if (varobj_is_dynamic_p (r->varobj))
ui_out_field_int (uiout, "dynamic", 1);
varobj_get_child_range (r->varobj, &from, &to);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 590aba6..66596b5 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1066,8 +1066,10 @@ varobj_get_attributes (struct varobj *var)
return attributes;
}
+/* Return true if VAR is a dynamic varobj. */
+
int
-varobj_pretty_printed_p (struct varobj *var)
+varobj_is_dynamic_p (struct varobj *var)
{
return var->dynamic->pretty_printer != NULL;
}
diff --git a/gdb/varobj.h b/gdb/varobj.h
index d3d81bf..f35f59b 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -306,7 +306,7 @@ extern void varobj_enable_pretty_printing (void);
extern int varobj_has_more (struct varobj *var, int to);
-extern int varobj_pretty_printed_p (struct varobj *var);
+extern int varobj_is_dynamic_p (struct varobj *var);
extern struct cleanup *varobj_ensure_python_env (struct varobj *var);
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p
2014-02-14 8:46 ` [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p Yao Qi
@ 2014-04-24 17:39 ` Keith Seitz
2014-05-21 18:02 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Keith Seitz @ 2014-04-24 17:39 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 02/14/2014 12:44 AM, Yao Qi wrote:
> We think varobj with --available-children-only behaves like a dynamic
> varobj, so dyanmic varobj is not pretty-printer specific. We rename
> varobj_pretty_printed_p to varobj_is_dynamic_p, so that we can handle
> available-children-only checking in varobj_is_dynamic_p in the next
> patch.
>
> gdb:
>
> 2014-02-14 Yao Qi <yao@codesourcery.com>
>
> * varobj.c (varobj_pretty_printed_p): Rename to ...
> (varobj_is_dynamic_p): ... this. New function.
> * varobj.h (varobj_pretty_printed_p): Remove declaration.
> (varobj_is_dynamic_p): Declare.
> * mi/mi-cmd-var.c (print_varobj): All callers updated.
> (mi_print_value_p, varobj_update_one): Likewise.
This looks fine. I recommend a maintainer approve this.
Keith
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p
2014-02-14 8:46 ` [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p Yao Qi
2014-04-24 17:39 ` Keith Seitz
@ 2014-05-21 18:02 ` Tom Tromey
1 sibling, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2014-05-21 18:02 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
Yao> 2014-02-14 Yao Qi <yao@codesourcery.com>
Yao> * varobj.c (varobj_pretty_printed_p): Rename to ...
Yao> (varobj_is_dynamic_p): ... this. New function.
Yao> * varobj.h (varobj_pretty_printed_p): Remove declaration.
Yao> (varobj_is_dynamic_p): Declare.
Yao> * mi/mi-cmd-var.c (print_varobj): All callers updated.
Yao> (mi_print_value_p, varobj_update_one): Likewise.
Ok.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (10 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 05/12] Rename varobj_pretty_printed_p to varobj_is_dynamic_p Yao Qi
@ 2014-02-14 8:46 ` Yao Qi
2014-02-14 9:40 ` Eli Zaretskii
2014-03-18 1:36 ` [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
2014-06-12 7:37 ` Yao Qi
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-14 8:46 UTC (permalink / raw)
To: gdb-patches
gdb/doc:
2014-02-14 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* gdb.texinfo (GDB/MI Variable Objects): Update doc of MI
commands "-var-create", "-var-info-num-children" and
"-var-list-children".
(GDB/MI Support Commands): Document the new
"available-children-only" entry in the output of the
"-list-features" command.
gdb:
2014-02-14 Yao Qi <yao@codesourcery.com>
* NEWS: Mention new option "--available-children-only".
---
gdb/NEWS | 7 +++++
gdb/doc/gdb.texinfo | 62 +++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 57 insertions(+), 12 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7fc3669..d77f75f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,13 @@ PowerPC64 GNU/Linux little-endian powerpc64le-*-linux*
** The commands -break-passcount and -break-commands accept convenience
variable as breakpoint number.
+ ** The commands -var-create, -var-info-num-children and
+ -var-list-children now accept an option
+ "--available-children-only". When used, only the available
+ children are displayed. Support for this feature can be verified
+ using the "-list-features" command, which should contain
+ "available-children-only".
+
*** Changes in GDB 7.7
* Improved support for process record-replay and reverse debugging on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 035573e..0c9bfd9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32504,7 +32504,7 @@ may work differently in future versions of @value{GDBN}.
@subsubheading Synopsis
@smallexample
- -var-create @{@var{name} | "-"@}
+ -var-create [--available-children-only] @{@var{name} | "-"@}
@{@var{frame-addr} | "*" | "@@"@} @var{expression}
@end smallexample
@@ -32526,6 +32526,11 @@ object must be created.
@var{expression} is any expression valid on the current language set (must not
begin with a @samp{*}), or one of the following:
+If the @samp{--available-children-only} option is specified, then only
+available or collected children of the varobj are considered. This
+affects the @samp{numchild} and @samp{has_more} attributes in the
+output result.
+
@itemize @bullet
@item
@samp{*@var{addr}}, where @var{addr} is the address of a memory cell
@@ -32556,8 +32561,9 @@ The name of the varobj.
@item numchild
The number of children of the varobj. This number is not necessarily
-reliable for a dynamic varobj. Instead, you must examine the
-@samp{has_more} attribute.
+reliable for a dynamic varobj or for a non-dynamic varobj if the
+@samp{--available-children-only} option was specified. Instead, you
+must examine the @samp{has_more} attribute.
@item value
The varobj's scalar value. For a varobj whose type is some sort of
@@ -32576,8 +32582,11 @@ If a variable object is bound to a specific thread, then this is the
thread's identifier.
@item has_more
-For a dynamic varobj, this indicates whether there appear to be any
-children available. For a non-dynamic varobj, this will be 0.
+For a dynamic varobj, or for a non-dynamic varobj when the
+@samp{--available-children-only} option is specified, this indicates
+whether there appear to be more children that haven't been listed yet,
+which can be listed with a following @code{-var-list-children}
+command. For the other cases, this will be 0.
@item dynamic
This attribute will be present and have the value @samp{1} if the
@@ -32663,7 +32672,7 @@ Returns the format used to display the value of the object @var{name}.
@subsubheading Synopsis
@smallexample
- -var-info-num-children @var{name}
+ -var-info-num-children [--available-children-only] @var{name}
@end smallexample
Returns the number of children of a variable object @var{name}:
@@ -32672,10 +32681,19 @@ Returns the number of children of a variable object @var{name}:
numchild=@var{n}
@end smallexample
-Note that this number is not completely reliable for a dynamic varobj.
-It will return the current number of children, but more children may
-be available.
+If the @samp{--available-children-only} option is specified, then only
+available or collected children of the varobj are considered.
+
+Note that this number is not completely reliable for a dynamic varobj
+or for a non-dynamic varobj that is listing only available or
+collected children. It will return the current number of children,
+but more children may be available.
+If for a given varobj, the presence of the
+@samp{--available-children-only} option changes between invocations of
+the @code{-var-create}, @code{-var-list-children} or
+@code{-var-info-num-children} commands, the list of children is
+reinitialized.
@subheading The @code{-var-list-children} Command
@findex -var-list-children
@@ -32683,7 +32701,9 @@ be available.
@subsubheading Synopsis
@smallexample
- -var-list-children [@var{print-values}] @var{name} [@var{from} @var{to}]
+ -var-list-children
+ [--available-children-only]
+ [@var{print-values}] @var{name} [@var{from} @var{to}]
@end smallexample
@anchor{-var-list-children}
@@ -32712,6 +32732,15 @@ then the front end could call @code{-var-set-update-range} with a
different range to ensure that future updates are restricted to just
the visible items.
+If the @samp{--available-children-only} option is specified, then only
+available or collected children of the varobj are considered.
+
+If for a given varobj, the presence of the
+@samp{--available-children-only} option changes between invocations of
+the @code{-var-create}, @code{-var-list-children} or
+@code{-var-info-num-children} commands, the list of children is
+reinitialized.
+
For each child the following results are returned:
@table @var
@@ -33021,8 +33050,12 @@ If the varobj's type changed, then this field will be present and will
hold the new type.
@item new_num_children
-For a dynamic varobj, if the number of children changed, or if the
-type changed, this will be the new number of children.
+For a dynamic varobj, or non-dynamic varobj that had been created as
+consequence of the @samp{--available-children-only} option to the
+@code{-var-create}, @code{-var-list-children} or
+@code{-var-info-num-children} commands, if the number of children
+changed, or if the type changed, this will be the new number of
+children.
The @samp{numchild} field in other varobj responses is generally not
valid for a dynamic varobj -- it will show the number of children that
@@ -35296,6 +35329,11 @@ records, produced when trying to execute an undefined @sc{gdb/mi} command
@item exec-run-start-option
Indicates that the @code{-exec-run} command supports the @option{--start}
option (@pxref{GDB/MI Program Execution}).
+@item available-children-only
+Indicates that the @code{-var-create}, @code{-var-info-num-children}
+and @code{-var-list-children} commands support the
+@option{--available-children-only}.
+
@end ftable
@subheading The @code{-list-target-features} Command
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-14 8:46 ` [PATCH 12/12] NEWS and Doc on --available-children-only Yao Qi
@ 2014-02-14 9:40 ` Eli Zaretskii
2014-02-17 9:46 ` Yao Qi
0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2014-02-14 9:40 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> From: Yao Qi <yao@codesourcery.com>
> Date: Fri, 14 Feb 2014 16:44:31 +0800
>
> gdb/doc:
>
> 2014-02-14 Pedro Alves <pedro@codesourcery.com>
> Yao Qi <yao@codesourcery.com>
>
> * gdb.texinfo (GDB/MI Variable Objects): Update doc of MI
> commands "-var-create", "-var-info-num-children" and
> "-var-list-children".
> (GDB/MI Support Commands): Document the new
> "available-children-only" entry in the output of the
> "-list-features" command.
Thanks. I have a few comments:
> + ** The commands -var-create, -var-info-num-children and
> + -var-list-children now accept an option
> + "--available-children-only". When used, only the available
> + children are displayed. Support for this feature can be verified
> + using the "-list-features" command, which should contain
> + "available-children-only".
This doesn't really say much about the new option, because what
"available children" means is not explained at all. Can you add a
sentence about that, or, better, reword this sentence
When used, only the available children are displayed.
so that the second part explains which children will be displayed and
which won't?
> +If the @samp{--available-children-only} option is specified, then only
> +available or collected children of the varobj are considered.
Again, without explaining what "available children" means, or maybe
which children might be "unavailable", this description just repeats
the name of the option.
> The number of children of the varobj. This number is not necessarily
> -reliable for a dynamic varobj. Instead, you must examine the
> -@samp{has_more} attribute.
> +reliable for a dynamic varobj or for a non-dynamic varobj if the
> +@samp{--available-children-only} option was specified.
When --available-children-only is specified, isn't it true that this
number is unreliable for _any_ varobj? If so, please rephrase to make
that explicit.
> -For a dynamic varobj, this indicates whether there appear to be any
> -children available. For a non-dynamic varobj, this will be 0.
> +For a dynamic varobj, or for a non-dynamic varobj when the
> +@samp{--available-children-only} option is specified, this indicates
> +whether there appear to be more children that haven't been listed yet,
> +which can be listed with a following @code{-var-list-children}
> +command. For the other cases, this will be 0.
Same here.
> +Note that this number is not completely reliable for a dynamic varobj
> +or for a non-dynamic varobj that is listing only available or
> +collected children. It will return the current number of children,
> +but more children may be available.
And here.
> +If for a given varobj, the presence of the
> +@samp{--available-children-only} option changes between invocations of
> +the @code{-var-create}, @code{-var-list-children} or
> +@code{-var-info-num-children} commands, the list of children is
> +reinitialized.
"Presence of the ... option changes" sounds awkward. Does this
sentence say anything important? IOW, does reinitialization of the
list of children have any user-visible effects? If not, perhaps it is
better to delete it. If you think this is needed, then I'd rephrase
like this:
If you use any of the commands @code{-var-create},
@code{-var-list-children}, or @code{-var-info-num-children} with the
@samp{--available-children-only} option followed by any of these
commands without that option, or vice versa, the list of children is
reinitialized.
> @smallexample
> - -var-list-children [@var{print-values}] @var{name} [@var{from} @var{to}]
> + -var-list-children
> + [--available-children-only]
> + [@var{print-values}] @var{name} [@var{from} @var{to}]
> @end smallexample
Will the reader understand that this is one long line broken for
readability? If not, perhaps you should tell that explicitly.
> +If the @samp{--available-children-only} option is specified, then only
> +available or collected children of the varobj are considered.
Once again, "available children" needs to be explained.
> +If for a given varobj, the presence of the
> +@samp{--available-children-only} option changes between invocations of
> +the @code{-var-create}, @code{-var-list-children} or
> +@code{-var-info-num-children} commands, the list of children is
> +reinitialized.
Same comment as above.
> @item new_num_children
> -For a dynamic varobj, if the number of children changed, or if the
> -type changed, this will be the new number of children.
> +For a dynamic varobj, or non-dynamic varobj that had been created as
> +consequence of the @samp{--available-children-only} option to the
> +@code{-var-create}, @code{-var-list-children} or
> +@code{-var-info-num-children} commands, if the number of children
> +changed, or if the type changed, this will be the new number of
> +children.
This sentence is now too long and complicated to be clear. Suggest to
break into 2 sentences. E.g., leave the original sentence as is, and
then add something like
Similarly for a non-dynamic varobj that was created as result of the
@samp{--available-children-only} option ...
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-14 9:40 ` Eli Zaretskii
@ 2014-02-17 9:46 ` Yao Qi
2014-02-17 15:04 ` Eli Zaretskii
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-17 9:46 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 02/14/2014 05:40 PM, Eli Zaretskii wrote:
> This doesn't really say much about the new option, because what
> "available children" means is not explained at all. Can you add a
> sentence about that, or, better, reword this sentence
>
> When used, only the available children are displayed.
>
> so that the second part explains which children will be displayed and
> which won't?
IMO, "available children" is not a new term in GDB. We've already had
some in doc:
Note that this number is not completely reliable for a dynamic varobj.
It will return the current number of children, but more children may
^^^^^^^^^^^^
be available.
^^^^^^^^^^^^^
The @samp{numchild} field in other varobj responses is generally not
valid for a dynamic varobj -- it will show the number of children that
@value{GDBN} knows about, but because dynamic varobjs lazily
instantiate their children, this will not reflect the number of
children which may be available.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"available children" means children, whose values are available, or
value available children. How about replacing "available children"
with "value available children"? Is it clear?
>
>> > The number of children of the varobj. This number is not necessarily
>> > -reliable for a dynamic varobj. Instead, you must examine the
>> > -@samp{has_more} attribute.
>> > +reliable for a dynamic varobj or for a non-dynamic varobj if the
>> > +@samp{--available-children-only} option was specified.
> When --available-children-only is specified, isn't it true that this
> number is unreliable for _any_ varobj? If so, please rephrase to make
> that explicit.
>
OK. I mention that explicitly in doc.
>> > -For a dynamic varobj, this indicates whether there appear to be any
>> > -children available. For a non-dynamic varobj, this will be 0.
>> > +For a dynamic varobj, or for a non-dynamic varobj when the
>> > +@samp{--available-children-only} option is specified, this indicates
>> > +whether there appear to be more children that haven't been listed yet,
>> > +which can be listed with a following @code{-var-list-children}
>> > +command. For the other cases, this will be 0.
> Same here.
>
Fixed.
>> > +Note that this number is not completely reliable for a dynamic varobj
>> > +or for a non-dynamic varobj that is listing only available or
>> > +collected children. It will return the current number of children,
>> > +but more children may be available.
> And here.
>
Fixed.
>> > +If for a given varobj, the presence of the
>> > +@samp{--available-children-only} option changes between invocations of
>> > +the @code{-var-create}, @code{-var-list-children} or
>> > +@code{-var-info-num-children} commands, the list of children is
>> > +reinitialized.
> "Presence of the ... option changes" sounds awkward. Does this
> sentence say anything important? IOW, does reinitialization of the
> list of children have any user-visible effects? If not, perhaps it is
> better to delete it. If you think this is needed, then I'd rephrase
> like this:
IMO, it has user-visible effects, which describes the expected behaviour
of sequence "-var-create --available-children-only" and
"-var-list-children". This is important to MI front end.
>
> If you use any of the commands @code{-var-create},
> @code{-var-list-children}, or @code{-var-info-num-children} with the
> @samp{--available-children-only} option followed by any of these
> commands without that option, or vice versa, the list of children is
> reinitialized.
It is good to me. I'll pick it up.
>
>> > @smallexample
>> > - -var-list-children [@var{print-values}] @var{name} [@var{from} @var{to}]
>> > + -var-list-children
>> > + [--available-children-only]
>> > + [@var{print-values}] @var{name} [@var{from} @var{to}]
>> > @end smallexample
> Will the reader understand that this is one long line broken for
> readability? If not, perhaps you should tell that explicitly.
>
Oh, let us put them in a single line to avoid confusion.
>> > +If the @samp{--available-children-only} option is specified, then only
>> > +available or collected children of the varobj are considered.
> Once again, "available children" needs to be explained.
>
>> > +If for a given varobj, the presence of the
>> > +@samp{--available-children-only} option changes between invocations of
>> > +the @code{-var-create}, @code{-var-list-children} or
>> > +@code{-var-info-num-children} commands, the list of children is
>> > +reinitialized.
> Same comment as above.
>
>> > @item new_num_children
>> > -For a dynamic varobj, if the number of children changed, or if the
>> > -type changed, this will be the new number of children.
>> > +For a dynamic varobj, or non-dynamic varobj that had been created as
>> > +consequence of the @samp{--available-children-only} option to the
>> > +@code{-var-create}, @code{-var-list-children} or
>> > +@code{-var-info-num-children} commands, if the number of children
>> > +changed, or if the type changed, this will be the new number of
>> > +children.
> This sentence is now too long and complicated to be clear. Suggest to
> break into 2 sentences. E.g., leave the original sentence as is, and
> then add something like
>
> Similarly for a non-dynamic varobj that was created as result of the
> @samp{--available-children-only} option ...
>
Fixed as you suggested. How about the updated patch below?
--
Yao (é½å°§)
gdb/doc:
2014-02-17 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* gdb.texinfo (GDB/MI Variable Objects): Update doc of MI
commands "-var-create", "-var-info-num-children" and
"-var-list-children".
(GDB/MI Support Commands): Document the new
"available-children-only" entry in the output of the
"-list-features" command.
gdb:
2014-02-17 Yao Qi <yao@codesourcery.com>
* NEWS: Mention new option "--available-children-only".
---
gdb/NEWS | 7 +++++++
gdb/doc/gdb.texinfo | 47 +++++++++++++++++++++++++++++++++++++++++------
2 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7fc3669..1eff6b3 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,13 @@ PowerPC64 GNU/Linux little-endian powerpc64le-*-linux*
** The commands -break-passcount and -break-commands accept convenience
variable as breakpoint number.
+ ** The commands -var-create, -var-info-num-children and
+ -var-list-children now accept an option
+ "--available-children-only". When used, only the value available
+ children are displayed. Support for this feature can be verified
+ using the "-list-features" command, which should contain
+ "available-children-only".
+
*** Changes in GDB 7.7
* Improved support for process record-replay and reverse debugging on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 035573e..03d395f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32504,7 +32504,7 @@ may work differently in future versions of @value{GDBN}.
@subsubheading Synopsis
@smallexample
- -var-create @{@var{name} | "-"@}
+ -var-create [--available-children-only] @{@var{name} | "-"@}
@{@var{frame-addr} | "*" | "@@"@} @var{expression}
@end smallexample
@@ -32526,6 +32526,11 @@ object must be created.
@var{expression} is any expression valid on the current language set (must not
begin with a @samp{*}), or one of the following:
+If the @samp{--available-children-only} option is specified, then only
+value available or collected children of the varobj are considered.
+This affects the @samp{numchild} and @samp{has_more} attributes in the
+output result.
+
@itemize @bullet
@item
@samp{*@var{addr}}, where @var{addr} is the address of a memory cell
@@ -32557,7 +32562,8 @@ The name of the varobj.
@item numchild
The number of children of the varobj. This number is not necessarily
reliable for a dynamic varobj. Instead, you must examine the
-@samp{has_more} attribute.
+@samp{has_more} attribute. If the @samp{--available-children-only}
+option was specified, this number is not reliable either.
@item value
The varobj's scalar value. For a varobj whose type is some sort of
@@ -32577,7 +32583,9 @@ thread's identifier.
@item has_more
For a dynamic varobj, this indicates whether there appear to be any
-children available. For a non-dynamic varobj, this will be 0.
+children available. When the @samp{--available-children-only} option
+is specified, this indicates whether there appear to be more children
+that haven't been listed yet. For the other cases, this will be 0.
@item dynamic
This attribute will be present and have the value @samp{1} if the
@@ -32663,7 +32671,7 @@ Returns the format used to display the value of the object @var{name}.
@subsubheading Synopsis
@smallexample
- -var-info-num-children @var{name}
+ -var-info-num-children [--available-children-only] @var{name}
@end smallexample
Returns the number of children of a variable object @var{name}:
@@ -32672,10 +32680,20 @@ Returns the number of children of a variable object @var{name}:
numchild=@var{n}
@end smallexample
+If the @samp{--available-children-only} option is specified, then only
+value available or collected children of the varobj are considered.
+
Note that this number is not completely reliable for a dynamic varobj.
+When the @samp{--available-children-only} option is specified, this
+number is not reliable either for a varobj.
It will return the current number of children, but more children may
be available.
+If you use any of the commands @code{-var-create},
+@code{-var-list-children}, or @code{-var-info-num-children} with the
+@samp{--available-children-only} option followed by any of these
+commands without that option, or vice versa, the list of children is
+reinitialized.
@subheading The @code{-var-list-children} Command
@findex -var-list-children
@@ -32683,7 +32701,7 @@ be available.
@subsubheading Synopsis
@smallexample
- -var-list-children [@var{print-values}] @var{name} [@var{from} @var{to}]
+ -var-list-children [--available-children-only] [@var{print-values}] @var{name} [@var{from} @var{to}]
@end smallexample
@anchor{-var-list-children}
@@ -32712,6 +32730,15 @@ then the front end could call @code{-var-set-update-range} with a
different range to ensure that future updates are restricted to just
the visible items.
+If the @samp{--available-children-only} option is specified, then only
+value available or collected children of the varobj are considered.
+
+If you use any of the commands @code{-var-create},
+@code{-var-list-children}, or @code{-var-info-num-children} with the
+@samp{--available-children-only} option followed by any of these
+commands without that option, or vice versa, the list of children is
+reinitialized.
+
For each child the following results are returned:
@table @var
@@ -33022,7 +33049,10 @@ hold the new type.
@item new_num_children
For a dynamic varobj, if the number of children changed, or if the
-type changed, this will be the new number of children.
+type changed, this will be the new number of children. Similarly
+for a non-dynamic varobj that was created as result of the
+@samp{--available-children-only} option to the @code{-var-create},
+@code{-var-list-children} or @code{-var-info-num-children} commands.
The @samp{numchild} field in other varobj responses is generally not
valid for a dynamic varobj -- it will show the number of children that
@@ -35296,6 +35326,11 @@ records, produced when trying to execute an undefined @sc{gdb/mi} command
@item exec-run-start-option
Indicates that the @code{-exec-run} command supports the @option{--start}
option (@pxref{GDB/MI Program Execution}).
+@item available-children-only
+Indicates that the @code{-var-create}, @code{-var-info-num-children}
+and @code{-var-list-children} commands support the
+@option{--available-children-only}.
+
@end ftable
@subheading The @code{-list-target-features} Command
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-17 9:46 ` Yao Qi
@ 2014-02-17 15:04 ` Eli Zaretskii
2014-02-18 2:01 ` Yao Qi
0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2014-02-17 15:04 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> Date: Mon, 17 Feb 2014 17:44:20 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
>
> On 02/14/2014 05:40 PM, Eli Zaretskii wrote:
> > This doesn't really say much about the new option, because what
> > "available children" means is not explained at all. Can you add a
> > sentence about that, or, better, reword this sentence
> >
> > When used, only the available children are displayed.
> >
> > so that the second part explains which children will be displayed and
> > which won't?
>
> IMO, "available children" is not a new term in GDB. We've already had
> some in doc:
>
> Note that this number is not completely reliable for a dynamic varobj.
> It will return the current number of children, but more children may
> ^^^^^^^^^^^^
> be available.
> ^^^^^^^^^^^^^
>
> The @samp{numchild} field in other varobj responses is generally not
> valid for a dynamic varobj -- it will show the number of children that
> @value{GDBN} knows about, but because dynamic varobjs lazily
> instantiate their children, this will not reflect the number of
> children which may be available.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
No, that's definitely not the same. Here, the text just says that not
all children are shown, and "available" is used in its usual meaning.
> "available children" means children, whose values are available, or
> value available children. How about replacing "available children"
> with "value available children"? Is it clear?
No, sorry. "Children whose values are available" is not clear at all.
Can you explain to me what makes the value "available", or what
prevents it from becoming available? Then I will suggest a suitable
wording.
> The number of children of the varobj. This number is not necessarily
> reliable for a dynamic varobj. Instead, you must examine the
> -@samp{has_more} attribute.
> +@samp{has_more} attribute. If the @samp{--available-children-only}
> +option was specified, this number is not reliable either.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"this number is not reliable for any varobj".
Please also make similar corrections in other places.
The rest of the patch is OK, so we are only left with describing what
are the "value available children".
Thanks.
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-17 15:04 ` Eli Zaretskii
@ 2014-02-18 2:01 ` Yao Qi
2014-02-18 5:11 ` Eli Zaretskii
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-18 2:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 02/17/2014 11:04 PM, Eli Zaretskii wrote:
> No, sorry. "Children whose values are available" is not clear at all.
>
> Can you explain to me what makes the value "available", or what
> prevents it from becoming available? Then I will suggest a suitable
> wording.
OK, thanks in advance.
When GDB reads from trace frames, if the variables are collected and
saved in trace frames, GDB is able to show the valid values of these
variables. We call "values of these variables are available". OTOH,
if the variables are not collected, "their values are unavailable".
For example, in a traceframe, field a is collected but field b is not.
As a result, value of field a is available, and value of field b is
unavailable.
struct foo
{
int a; /* Collected */
int b; /* Uncollected */
};
Going to MI/varobj world, everything is structured as a tree, and each
tree node is about certain value. foo.a and foo.b are the children of
foo in MI/varobj, so foo.a is an available child of foo, but foo.b
isn't.
The concept of available and unavailable can be illustrated by GDB
accessing trace frames, but the concept itself is quite independent and
can be applied to other situations.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-18 2:01 ` Yao Qi
@ 2014-02-18 5:11 ` Eli Zaretskii
2014-02-18 7:14 ` Yao Qi
0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2014-02-18 5:11 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> Date: Tue, 18 Feb 2014 09:59:20 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
>
> When GDB reads from trace frames, if the variables are collected and
> saved in trace frames, GDB is able to show the valid values of these
> variables. We call "values of these variables are available". OTOH,
> if the variables are not collected, "their values are unavailable".
>
> For example, in a traceframe, field a is collected but field b is not.
> As a result, value of field a is available, and value of field b is
> unavailable.
>
> struct foo
> {
> int a; /* Collected */
> int b; /* Uncollected */
> };
>
> Going to MI/varobj world, everything is structured as a tree, and each
> tree node is about certain value. foo.a and foo.b are the children of
> foo in MI/varobj, so foo.a is an available child of foo, but foo.b
> isn't.
>
> The concept of available and unavailable can be illustrated by GDB
> accessing trace frames, but the concept itself is quite independent and
> can be applied to other situations.
OK, thanks. I think I understand. I suggest, instead of this:
If the @samp{--available-children-only} option is specified, then only
value available or collected children of the varobj are considered.
to say this:
If the @samp{--available-children-only} option is specified, then
@value(GDBN) considers only those children of the varobj whose
values were collected.
And in general, use "collected values" and "children whose values were
collected" or "children with collected values" in other places.
Does this correctly capture your intent?
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-18 5:11 ` Eli Zaretskii
@ 2014-02-18 7:14 ` Yao Qi
2014-02-18 15:07 ` Eli Zaretskii
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-02-18 7:14 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 02/18/2014 01:11 PM, Eli Zaretskii wrote:
> OK, thanks. I think I understand. I suggest, instead of this:
>
> If the @samp{--available-children-only} option is specified, then only
> value available or collected children of the varobj are considered.
>
> to say this:
>
> If the @samp{--available-children-only} option is specified, then
> @value(GDBN) considers only those children of the varobj whose
> values were collected.
>
> And in general, use "collected values" and "children whose values were
> collected" or "children with collected values" in other places.
>
> Does this correctly capture your intent?
Yes, that is good to me. Here is the updated one.
--
Yao (é½å°§)
gdb/doc:
2014-02-18 Pedro Alves <pedro@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* gdb.texinfo (GDB/MI Variable Objects): Update doc of MI
commands "-var-create", "-var-info-num-children" and
"-var-list-children".
(GDB/MI Support Commands): Document the new
"available-children-only" entry in the output of the
"-list-features" command.
gdb:
2014-02-18 Yao Qi <yao@codesourcery.com>
* NEWS: Mention new option "--available-children-only".
---
gdb/NEWS | 7 +++++++
gdb/doc/gdb.texinfo | 51 +++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7fc3669..06d7634 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,13 @@ PowerPC64 GNU/Linux little-endian powerpc64le-*-linux*
** The commands -break-passcount and -break-commands accept convenience
variable as breakpoint number.
+ ** The commands -var-create, -var-info-num-children and
+ -var-list-children now accept an option
+ "--available-children-only". When used, only the children with
+ collected values are displayed. Support for this feature can be
+ verified using the "-list-features" command, which should contain
+ "available-children-only".
+
*** Changes in GDB 7.7
* Improved support for process record-replay and reverse debugging on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 035573e..87cb26f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32504,7 +32504,7 @@ may work differently in future versions of @value{GDBN}.
@subsubheading Synopsis
@smallexample
- -var-create @{@var{name} | "-"@}
+ -var-create [--available-children-only] @{@var{name} | "-"@}
@{@var{frame-addr} | "*" | "@@"@} @var{expression}
@end smallexample
@@ -32526,6 +32526,13 @@ object must be created.
@var{expression} is any expression valid on the current language set (must not
begin with a @samp{*}), or one of the following:
+If the @samp{--available-children-only} option is specified, then
+@value(GDBN) considers only those children of the varobj whose
+values were collected.
+
+This affects the @samp{numchild} and @samp{has_more} attributes in the
+output result.
+
@itemize @bullet
@item
@samp{*@var{addr}}, where @var{addr} is the address of a memory cell
@@ -32557,7 +32564,8 @@ The name of the varobj.
@item numchild
The number of children of the varobj. This number is not necessarily
reliable for a dynamic varobj. Instead, you must examine the
-@samp{has_more} attribute.
+@samp{has_more} attribute. If the @samp{--available-children-only}
+option was specified, this number is not reliable for any varobj.
@item value
The varobj's scalar value. For a varobj whose type is some sort of
@@ -32577,7 +32585,9 @@ thread's identifier.
@item has_more
For a dynamic varobj, this indicates whether there appear to be any
-children available. For a non-dynamic varobj, this will be 0.
+children available. When the @samp{--available-children-only} option
+is specified, this indicates whether there appear to be more children
+that haven't been listed yet. For the other cases, this will be 0.
@item dynamic
This attribute will be present and have the value @samp{1} if the
@@ -32663,7 +32673,7 @@ Returns the format used to display the value of the object @var{name}.
@subsubheading Synopsis
@smallexample
- -var-info-num-children @var{name}
+ -var-info-num-children [--available-children-only] @var{name}
@end smallexample
Returns the number of children of a variable object @var{name}:
@@ -32672,10 +32682,21 @@ Returns the number of children of a variable object @var{name}:
numchild=@var{n}
@end smallexample
+If the @samp{--available-children-only} option is specified, then
+@value(GDBN) considers only those children of the varobj whose
+values were collected.
+
Note that this number is not completely reliable for a dynamic varobj.
+When the @samp{--available-children-only} option is specified, this
+number is not reliable for any varobj.
It will return the current number of children, but more children may
be available.
+If you use any of the commands @code{-var-create},
+@code{-var-list-children}, or @code{-var-info-num-children} with the
+@samp{--available-children-only} option followed by any of these
+commands without that option, or vice versa, the list of children is
+reinitialized.
@subheading The @code{-var-list-children} Command
@findex -var-list-children
@@ -32683,7 +32704,7 @@ be available.
@subsubheading Synopsis
@smallexample
- -var-list-children [@var{print-values}] @var{name} [@var{from} @var{to}]
+ -var-list-children [--available-children-only] [@var{print-values}] @var{name} [@var{from} @var{to}]
@end smallexample
@anchor{-var-list-children}
@@ -32712,6 +32733,16 @@ then the front end could call @code{-var-set-update-range} with a
different range to ensure that future updates are restricted to just
the visible items.
+If the @samp{--available-children-only} option is specified, then
+@value(GDBN) considers only those children of the varobj whose
+values were collected.
+
+If you use any of the commands @code{-var-create},
+@code{-var-list-children}, or @code{-var-info-num-children} with the
+@samp{--available-children-only} option followed by any of these
+commands without that option, or vice versa, the list of children is
+reinitialized.
+
For each child the following results are returned:
@table @var
@@ -33022,7 +33053,10 @@ hold the new type.
@item new_num_children
For a dynamic varobj, if the number of children changed, or if the
-type changed, this will be the new number of children.
+type changed, this will be the new number of children. Similarly
+for a non-dynamic varobj that was created as result of the
+@samp{--available-children-only} option to the @code{-var-create},
+@code{-var-list-children} or @code{-var-info-num-children} commands.
The @samp{numchild} field in other varobj responses is generally not
valid for a dynamic varobj -- it will show the number of children that
@@ -35296,6 +35330,11 @@ records, produced when trying to execute an undefined @sc{gdb/mi} command
@item exec-run-start-option
Indicates that the @code{-exec-run} command supports the @option{--start}
option (@pxref{GDB/MI Program Execution}).
+@item available-children-only
+Indicates that the @code{-var-create}, @code{-var-info-num-children}
+and @code{-var-list-children} commands support the
+@option{--available-children-only}.
+
@end ftable
@subheading The @code{-list-target-features} Command
--
1.7.7.6
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH 12/12] NEWS and Doc on --available-children-only
2014-02-18 7:14 ` Yao Qi
@ 2014-02-18 15:07 ` Eli Zaretskii
0 siblings, 0 replies; 50+ messages in thread
From: Eli Zaretskii @ 2014-02-18 15:07 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> Date: Tue, 18 Feb 2014 15:12:43 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
>
> On 02/18/2014 01:11 PM, Eli Zaretskii wrote:
> > OK, thanks. I think I understand. I suggest, instead of this:
> >
> > If the @samp{--available-children-only} option is specified, then only
> > value available or collected children of the varobj are considered.
> >
> > to say this:
> >
> > If the @samp{--available-children-only} option is specified, then
> > @value(GDBN) considers only those children of the varobj whose
> > values were collected.
> >
> > And in general, use "collected values" and "children whose values were
> > collected" or "children with collected values" in other places.
> >
> > Does this correctly capture your intent?
>
> Yes, that is good to me. Here is the updated one.
This version is OK. Thanks for persevering.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC 00/12 V2] Visit varobj available children only in MI
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (11 preceding siblings ...)
2014-02-14 8:46 ` [PATCH 12/12] NEWS and Doc on --available-children-only Yao Qi
@ 2014-03-18 1:36 ` Yao Qi
2014-04-04 2:00 ` Yao Qi
2014-06-12 7:37 ` Yao Qi
13 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-03-18 1:36 UTC (permalink / raw)
To: gdb-patches
On 02/14/2014 04:44 PM, Yao Qi wrote:
> This is the V2 of this patch series, addressing Keith's review comments
> to V1 (reviewed in Jan 2014). The general description of this series
> can be found https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
>
> Changes in V2 are:
>
> - Include NEWS and doc.
> - Fix typos, grammatical mistakes, and changelog entries.
> - Update copyright year from "2013" to "2013-2014" for new files.
> - Use -list-features to return "available-children-only" (#7).
> - Update config/djgpp/fnchange.lst for new tests.
> - Fix a missing cleanup (#2).
> - Call varobj_delete_iter in free_variable (#3).
>
> Regression tested on x86_64-linux.
>
Ping. https://sourceware.org/ml/gdb-patches/2014-02/msg00489.html
Can anyone have a look at this patch? Patch 11/12 still depends on
"[PATCH] Accept convenience variable in commands -break-passcount and
-break-commands", but it doesn't matter much for reviewing this patch
series.
IMO, patches #1 - #6 are refactor to existing code on dynamic varobj,
and can go in regardless of the decision of the rest (MI option
--available-children-only stuff). WDYT?
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [RFC 00/12 V2] Visit varobj available children only in MI
2014-03-18 1:36 ` [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
@ 2014-04-04 2:00 ` Yao Qi
2014-04-17 1:12 ` ping : " Yao Qi
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-04-04 2:00 UTC (permalink / raw)
To: gdb-patches
On 03/18/2014 09:34 AM, Yao Qi wrote:
> On 02/14/2014 04:44 PM, Yao Qi wrote:
>> This is the V2 of this patch series, addressing Keith's review comments
>> to V1 (reviewed in Jan 2014). The general description of this series
>> can be found https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
>>
>> Changes in V2 are:
>>
>> - Include NEWS and doc.
>> - Fix typos, grammatical mistakes, and changelog entries.
>> - Update copyright year from "2013" to "2013-2014" for new files.
>> - Use -list-features to return "available-children-only" (#7).
>> - Update config/djgpp/fnchange.lst for new tests.
>> - Fix a missing cleanup (#2).
>> - Call varobj_delete_iter in free_variable (#3).
>>
>> Regression tested on x86_64-linux.
>>
>
> Ping. https://sourceware.org/ml/gdb-patches/2014-02/msg00489.html
>
> Can anyone have a look at this patch? Patch 11/12 still depends on
> "[PATCH] Accept convenience variable in commands -break-passcount and
> -break-commands", but it doesn't matter much for reviewing this patch
> series.
>
> IMO, patches #1 - #6 are refactor to existing code on dynamic varobj,
> and can go in regardless of the decision of the rest (MI option
> --available-children-only stuff). WDYT?
>
Ping.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 50+ messages in thread
* ping : [RFC 00/12 V2] Visit varobj available children only in MI
2014-04-04 2:00 ` Yao Qi
@ 2014-04-17 1:12 ` Yao Qi
2014-04-21 16:20 ` Joel Brobecker
0 siblings, 1 reply; 50+ messages in thread
From: Yao Qi @ 2014-04-17 1:12 UTC (permalink / raw)
To: gdb-patches
On 04/04/2014 09:57 AM, Yao Qi wrote:
> On 03/18/2014 09:34 AM, Yao Qi wrote:
>> On 02/14/2014 04:44 PM, Yao Qi wrote:
>>> This is the V2 of this patch series, addressing Keith's review comments
>>> to V1 (reviewed in Jan 2014). The general description of this series
>>> can be found https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
>>>
>>> Changes in V2 are:
>>>
>>> - Include NEWS and doc.
>>> - Fix typos, grammatical mistakes, and changelog entries.
>>> - Update copyright year from "2013" to "2013-2014" for new files.
>>> - Use -list-features to return "available-children-only" (#7).
>>> - Update config/djgpp/fnchange.lst for new tests.
>>> - Fix a missing cleanup (#2).
>>> - Call varobj_delete_iter in free_variable (#3).
>>>
>>> Regression tested on x86_64-linux.
>>>
>>
>> Ping. https://sourceware.org/ml/gdb-patches/2014-02/msg00489.html
>>
>> Can anyone have a look at this patch? Patch 11/12 still depends on
>> "[PATCH] Accept convenience variable in commands -break-passcount and
>> -break-commands", but it doesn't matter much for reviewing this patch
>> series.
>>
>> IMO, patches #1 - #6 are refactor to existing code on dynamic varobj,
>> and can go in regardless of the decision of the rest (MI option
>> --available-children-only stuff). WDYT?
>>
>
> Ping.
>
Ping?
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: ping : [RFC 00/12 V2] Visit varobj available children only in MI
2014-04-17 1:12 ` ping : " Yao Qi
@ 2014-04-21 16:20 ` Joel Brobecker
2014-04-22 2:54 ` Yao Qi
0 siblings, 1 reply; 50+ messages in thread
From: Joel Brobecker @ 2014-04-21 16:20 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> >>> The general description of this series
> >>> can be found https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
I wanted to see if I could help with this patch series, but I wasn't
able to understand what the problem was, and why this new feature
is needed. The description just goes too quickly for me, sorry. Since
Pedro wrote the original patch series, he probably knows more about it.
--
Joel
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: ping : [RFC 00/12 V2] Visit varobj available children only in MI
2014-04-21 16:20 ` Joel Brobecker
@ 2014-04-22 2:54 ` Yao Qi
0 siblings, 0 replies; 50+ messages in thread
From: Yao Qi @ 2014-04-22 2:54 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 04/22/2014 12:20 AM, Joel Brobecker wrote:
>>>>> The general description of this series
>>>>> can be found https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
>
> I wanted to see if I could help with this patch series, but I wasn't
> able to understand what the problem was, and why this new feature
> is needed. The description just goes too quickly for me, sorry. Since
> Pedro wrote the original patch series, he probably knows more about it.
>
Joel, thanks for your feedback on this patch series. If the whole
change is hard to be justified, probably we can get patch 01~04 and 10
reviewed at first. Patch 01~04 are refactor to existing python/varobj
stuff, and make the interface clearer.
Keith is looking at this patch series...
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [RFC 00/12 V2] Visit varobj available children only in MI
2014-02-14 8:46 [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
` (12 preceding siblings ...)
2014-03-18 1:36 ` [RFC 00/12 V2] Visit varobj available children only in MI Yao Qi
@ 2014-06-12 7:37 ` Yao Qi
13 siblings, 0 replies; 50+ messages in thread
From: Yao Qi @ 2014-06-12 7:37 UTC (permalink / raw)
To: gdb-patches
On 02/14/2014 04:44 PM, Yao Qi wrote:
> This is the V2 of this patch series, addressing Keith's review comments
> to V1 (reviewed in Jan 2014). The general description of this series
> can be found https://sourceware.org/ml/gdb-patches/2013-11/msg00739.html
>
> Changes in V2 are:
>
> - Include NEWS and doc.
> - Fix typos, grammatical mistakes, and changelog entries.
> - Update copyright year from "2013" to "2013-2014" for new files.
> - Use -list-features to return "available-children-only" (#7).
> - Update config/djgpp/fnchange.lst for new tests.
> - Fix a missing cleanup (#2).
> - Call varobj_delete_iter in free_variable (#3).
>
> Regression tested on x86_64-linux.
Since 7_8 branch is created, I pushed patches 1 to 6 (reviewed by Tom)
in to trunk.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 50+ messages in thread