* [RFA] Updated path expression computations for varobj trees
@ 2011-12-16 23:43 Keith Seitz
2011-12-20 15:25 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2011-12-16 23:43 UTC (permalink / raw)
To: gp >> "gdb-patches@sourceware.org ml"
[-- Attachment #1: Type: text/plain, Size: 980 bytes --]
Hi,
Revisions on my 10586 patch has exposed a couple of shortcomings with my
recent varobj tree testing infrastructure's handling of path expressions.
I am proposing the attached patch which updates comments and fixes a
bunch of path expression problems. The most notable change for this is
an assumption that all varobjs are compound types unless the type is
recognized as a simple type.
With this patch, I can now test path expressions in my 10586 patch which
I had previously omitted.
Comments/questions/concerns?
Keith
testsuite/ChangeLog
2011-12-16 Keith Seitz <keiths@redhat.com>
* lib/mi-support.exp: Expand comments about PATH_EXPR.
(varobj_tree::get_path_expr): Assume that all varobjs are
compound unless they are known simple types.
Adjust path expressions based on parent type, path parent type,
and tree language.
(varobj_tree::walk_tree): Add LANGUAGE parameter and save it into
the root varobj.
(mi_walk_varobj_tree): Add LANGUAGE parameter.
[-- Attachment #2: varobj_walk_tree_update.patch --]
[-- Type: text/x-patch, Size: 5313 bytes --]
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index e1845f6..1f2b79c 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2012,7 +2012,7 @@ proc mi_get_features {} {
# }
# }
#
-# mi_walk_varobj_tree $tree
+# mi_walk_varobj_tree c++ $tree
#
# If you'd prefer to walk the tree using your own callback,
# simply pass the name of the callback to mi_walk_varobj_tree.
@@ -2038,6 +2038,9 @@ proc mi_get_features {} {
# type - the type of this variable (type="type" in the output
# of -var-list-children, or the special tag "anonymous"
# path_expr - the "-var-info-path-expression" for this variable
+# NOTE: This member cannot be used reliably with typedefs.
+# Use with caution!
+# See notes inside get_path_expr for more.
# parent - the variable name of the parent varobj
# children - a list of children variable names (which are the
# names Tcl arrays, not object names)
@@ -2084,7 +2087,8 @@ namespace eval ::varobj_tree {
}
# The default callback used by mi_walk_varobj_tree. This callback
- # simply checks all of VAR's children.
+ # simply checks all of VAR's children. It specifically does not test
+ # path expressions, since that is very problematic.
#
# This procedure may be used in custom callbacks.
proc test_children_callback {variable_name} {
@@ -2154,20 +2158,82 @@ namespace eval ::varobj_tree {
# parent varobj whose variable name is given by PARENT_VARIABLE.
proc get_path_expr {parent_variable name type} {
upvar #0 $parent_variable parent
+ upvar #0 $parent_variable path_parent
# If TYPE is "", this is one of the CPLUS_FAKE_CHILD varobjs,
- # which has no path expression
- if {[string length $type] == 0} {
+ # which has no path expression. Likewsise for anonymous structs
+ # and unions.
+ if {[string length $type] == 0 \
+ || [string compare $type "anonymous"] == 0} {
return ""
}
# Find the path parent variable.
while {![is_path_expr_parent $parent_variable]} {
- set parent_variable $parent(parent)
- upvar #0 $parent_variable parent
- }
+ set parent_variable $path_parent(parent)
+ upvar #0 $parent_variable path_parent
+ }
+
+ # This is where things get difficult. We do not actually know
+ # the real type for variables defined via typedefs, so we don't actually
+ # know whether the parent is a structure/union or not.
+ #
+ # So we assume everything that isn't a simple type is a compound type.
+ set stars ""
+ regexp {\*+} $parent(type) stars
+ set is_compound 1
+ set delim {[\ \*&]+}
+ switch -regexp $parent(type) \
+ bool$delim - \
+ char$delim - \
+ char16_t$delim - \
+ char32_t$delim - \
+ double$delim - \
+ float$delim - \
+ int$delim - \
+ long$delim - \
+ short$delim - \
+ signed$delim - \
+ unsigned$delim - \
+ void$delim - \
+ wchar_t$delim {
+ set is_compound 0
+ } \
+ \
+ default {
+ # We assume everything is a compound type except pointers
+ # to structures, which are automatically dereferenced by
+ # varobj. All other pointers are simple types.
+ if {[string length $stars] > 1} {
+ set is_compound 0
+ }
+ }
+
+ if {[string index $parent(type) end] == "\]"} {
+ # Parent is an array.
+ return "($path_parent(path_expr))\[$name\]"
+ } elseif {$is_compound} {
+ # Parent is a structure or union or a pointer to one.
+ if {[string length $stars]} {
+ set join "->"
+ } else {
+ set join "."
+ }
- return "(($parent(path_expr)).$name)"
+ global root
+
+ # To make matters even more hideous, varobj.c has slightly different
+ # path expressions for C and C++.
+ set path_expr "($path_parent(path_expr))$join$name"
+ if {[string compare -nocase $root(language) "c"] == 0} {
+ return $path_expr
+ } else {
+ return "($path_expr)"
+ }
+ } else {
+ # Parent is a pointer.
+ return "*($path_parent(path_expr))"
+ }
}
# Process the CHILDREN (a list of varobj_tree elements) of the variable
@@ -2208,7 +2274,7 @@ namespace eval ::varobj_tree {
# The main procedure to call the given CALLBACK on the elements of the
# given varobj TREE. See detailed explanation above.
- proc walk_tree {tree callback} {
+ proc walk_tree {language tree callback} {
global root
if {[llength $tree] < 3} {
@@ -2216,6 +2282,7 @@ namespace eval ::varobj_tree {
}
# Create root node and process the tree.
+ array set root [list language $language]
array set root [list obj_name "root"]
array set root [list display_name "root"]
array set root [list type "root"]
@@ -2259,7 +2326,8 @@ proc mi_varobj_tree_test_children_callback {variable} {
# Walk the variable object tree given by TREE, calling the specified
# CALLBACK. By default this uses mi_varobj_tree_test_children_callback.
-proc mi_walk_varobj_tree {tree {callback \
- mi_varobj_tree_test_children_callback}} {
- ::varobj_tree::walk_tree $tree $callback
+proc mi_walk_varobj_tree {language tree \
+ {callback \
+ mi_varobj_tree_test_children_callback}} {
+ ::varobj_tree::walk_tree $language $tree $callback
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Updated path expression computations for varobj trees
2011-12-16 23:43 [RFA] Updated path expression computations for varobj trees Keith Seitz
@ 2011-12-20 15:25 ` Tom Tromey
2011-12-20 17:48 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2011-12-20 15:25 UTC (permalink / raw)
To: Keith Seitz; +Cc: gp >> "gdb-patches@sourceware.org ml"
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> * lib/mi-support.exp: Expand comments about PATH_EXPR.
Keith> (varobj_tree::get_path_expr): Assume that all varobjs are
Keith> compound unless they are known simple types.
Keith> Adjust path expressions based on parent type, path parent type,
Keith> and tree language.
Keith> (varobj_tree::walk_tree): Add LANGUAGE parameter and save it into
Keith> the root varobj.
Keith> (mi_walk_varobj_tree): Add LANGUAGE parameter.
This is ok.
I'm not super fond of the type regexp thing, but I suppose we can always
enhance it more later if need be.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Updated path expression computations for varobj trees
2011-12-20 15:25 ` Tom Tromey
@ 2011-12-20 17:48 ` Keith Seitz
2012-01-05 20:05 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2011-12-20 17:48 UTC (permalink / raw)
To: Tom Tromey; +Cc: gp >> "gdb-patches@sourceware.org ml"
On 12/20/2011 07:23 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz<keiths@redhat.com> writes:
>
> Keith> * lib/mi-support.exp: Expand comments about PATH_EXPR.
> Keith> (varobj_tree::get_path_expr): Assume that all varobjs are
> Keith> compound unless they are known simple types.
> Keith> Adjust path expressions based on parent type, path parent type,
> Keith> and tree language.
> Keith> (varobj_tree::walk_tree): Add LANGUAGE parameter and save it into
> Keith> the root varobj.
> Keith> (mi_walk_varobj_tree): Add LANGUAGE parameter.
>
> This is ok.
>
> I'm not super fond of the type regexp thing, but I suppose we can always
> enhance it more later if need be.
I am not very fond of it, either, but let me see if I can fiddle with it
one more time. I've thought of something that might work.
I'll send a note to the list one way or the other.
Thank you for taking a look.
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Updated path expression computations for varobj trees
2011-12-20 17:48 ` Keith Seitz
@ 2012-01-05 20:05 ` Keith Seitz
2012-01-12 21:30 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2012-01-05 20:05 UTC (permalink / raw)
To: Tom Tromey; +Cc: gp >> "gdb-patches@sourceware.org ml"
[-- Attachment #1: Type: text/plain, Size: 720 bytes --]
On 12/20/2011 07:23 AM, Tom Tromey wrote:
> I'm not super fond of the type regexp thing, but I suppose we can always
> enhance it more later if need be.
Ok, well, after fiddling with this far too much, I've (almost
embarrassingly) whittled this regexp to:
+ if {[string index $name 0] == "*"} {
+ set is_compound 0
+ }
In other words, we assume that the construct is a compound type unless
the child's name begins with "*", which means the parent must be a pointer.
We may encounter difficulties with this, too, mind you, but I believe
this is a cheaper and easier condition to deal with than the regexp.
What do you think?
Keith
[Re-attaching whole patch for clarity. ChangeLog is still the same.]
[-- Attachment #2: varobj_walk_tree_update-2.patch --]
[-- Type: text/x-patch, Size: 4795 bytes --]
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index e1845f6..4693775 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2012,7 +2012,7 @@ proc mi_get_features {} {
# }
# }
#
-# mi_walk_varobj_tree $tree
+# mi_walk_varobj_tree c++ $tree
#
# If you'd prefer to walk the tree using your own callback,
# simply pass the name of the callback to mi_walk_varobj_tree.
@@ -2038,6 +2038,9 @@ proc mi_get_features {} {
# type - the type of this variable (type="type" in the output
# of -var-list-children, or the special tag "anonymous"
# path_expr - the "-var-info-path-expression" for this variable
+# NOTE: This member cannot be used reliably with typedefs.
+# Use with caution!
+# See notes inside get_path_expr for more.
# parent - the variable name of the parent varobj
# children - a list of children variable names (which are the
# names Tcl arrays, not object names)
@@ -2084,7 +2087,8 @@ namespace eval ::varobj_tree {
}
# The default callback used by mi_walk_varobj_tree. This callback
- # simply checks all of VAR's children.
+ # simply checks all of VAR's children. It specifically does not test
+ # path expressions, since that is very problematic.
#
# This procedure may be used in custom callbacks.
proc test_children_callback {variable_name} {
@@ -2154,20 +2158,59 @@ namespace eval ::varobj_tree {
# parent varobj whose variable name is given by PARENT_VARIABLE.
proc get_path_expr {parent_variable name type} {
upvar #0 $parent_variable parent
+ upvar #0 $parent_variable path_parent
# If TYPE is "", this is one of the CPLUS_FAKE_CHILD varobjs,
- # which has no path expression
- if {[string length $type] == 0} {
+ # which has no path expression. Likewsise for anonymous structs
+ # and unions.
+ if {[string length $type] == 0 \
+ || [string compare $type "anonymous"] == 0} {
return ""
}
# Find the path parent variable.
while {![is_path_expr_parent $parent_variable]} {
- set parent_variable $parent(parent)
- upvar #0 $parent_variable parent
- }
+ set parent_variable $path_parent(parent)
+ upvar #0 $parent_variable path_parent
+ }
+
+ # This is where things get difficult. We do not actually know
+ # the real type for variables defined via typedefs, so we don't actually
+ # know whether the parent is a structure/union or not.
+ #
+ # So we assume everything that isn't a simple type is a compound type.
+ set stars ""
+ regexp {\*+} $parent(type) stars
+ set is_compound 1
+ if {[string index $name 0] == "*"} {
+ set is_compound 0
+ }
+
+ if {[string index $parent(type) end] == "\]"} {
+ # Parent is an array.
+ return "($path_parent(path_expr))\[$name\]"
+ } elseif {$is_compound} {
+ # Parent is a structure or union or a pointer to one.
+ if {[string length $stars]} {
+ set join "->"
+ } else {
+ set join "."
+ }
+
+ global root
- return "(($parent(path_expr)).$name)"
+ # To make matters even more hideous, varobj.c has slightly different
+ # path expressions for C and C++.
+ set path_expr "($path_parent(path_expr))$join$name"
+ if {[string compare -nocase $root(language) "c"] == 0} {
+ return $path_expr
+ } else {
+ return "($path_expr)"
+ }
+ } else {
+ # Parent is a pointer.
+ return "*($path_parent(path_expr))"
+ }
}
# Process the CHILDREN (a list of varobj_tree elements) of the variable
@@ -2208,7 +2251,7 @@ namespace eval ::varobj_tree {
# The main procedure to call the given CALLBACK on the elements of the
# given varobj TREE. See detailed explanation above.
- proc walk_tree {tree callback} {
+ proc walk_tree {language tree callback} {
global root
if {[llength $tree] < 3} {
@@ -2216,6 +2259,7 @@ namespace eval ::varobj_tree {
}
# Create root node and process the tree.
+ array set root [list language $language]
array set root [list obj_name "root"]
array set root [list display_name "root"]
array set root [list type "root"]
@@ -2259,7 +2303,8 @@ proc mi_varobj_tree_test_children_callback {variable} {
# Walk the variable object tree given by TREE, calling the specified
# CALLBACK. By default this uses mi_varobj_tree_test_children_callback.
-proc mi_walk_varobj_tree {tree {callback \
- mi_varobj_tree_test_children_callback}} {
- ::varobj_tree::walk_tree $tree $callback
+proc mi_walk_varobj_tree {language tree \
+ {callback \
+ mi_varobj_tree_test_children_callback}} {
+ ::varobj_tree::walk_tree $language $tree $callback
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Updated path expression computations for varobj trees
2012-01-05 20:05 ` Keith Seitz
@ 2012-01-12 21:30 ` Tom Tromey
2012-01-12 22:33 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2012-01-12 21:30 UTC (permalink / raw)
To: Keith Seitz; +Cc: gp >> "gdb-patches@sourceware.org ml"
>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:
Keith> In other words, we assume that the construct is a compound type unless
Keith> the child's name begins with "*", which means the parent must be a
Keith> pointer.
Keith> We may encounter difficulties with this, too, mind you, but I believe
Keith> this is a cheaper and easier condition to deal with than the regexp.
Me too.
If the issue is typedefs, we also have the option of requiring a certain
naming convention for code using this part of the test suite.
Keith> What do you think?
It is fine by me.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Updated path expression computations for varobj trees
2012-01-12 21:30 ` Tom Tromey
@ 2012-01-12 22:33 ` Keith Seitz
0 siblings, 0 replies; 6+ messages in thread
From: Keith Seitz @ 2012-01-12 22:33 UTC (permalink / raw)
To: gp >> "gdb-patches@sourceware.org ml"
On 01/12/2012 01:21 PM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz<keiths@redhat.com> writes:
> Keith> What do you think?
>
> It is fine by me.
Ok, I've committed this. I will now retest and commit the patch for 10586.
Thank you for taking a look at this.
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-12 22:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-16 23:43 [RFA] Updated path expression computations for varobj trees Keith Seitz
2011-12-20 15:25 ` Tom Tromey
2011-12-20 17:48 ` Keith Seitz
2012-01-05 20:05 ` Keith Seitz
2012-01-12 21:30 ` Tom Tromey
2012-01-12 22:33 ` Keith Seitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).