* Re: [PATCH] MI: Allow non-raw varobj evaluation
2018-01-25 11:11 ` Leszek Swirski via gdb-patches
@ 2018-02-02 4:27 ` Simon Marchi
2018-02-02 9:01 ` Leszek Swirski via gdb-patches
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2018-02-02 4:27 UTC (permalink / raw)
To: Leszek Swirski; +Cc: gdb-patches
On 2018-01-25 06:11 AM, Leszek Swirski via gdb-patches wrote:
> On Thu, Jan 25, 2018 at 2:53 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>
>> Could you give an example to reproduce this problem? I tried with a vector of vector, but -var-evaluate-expression just prints "{...}":
>>
>> -var-evaluate-expression var1
>> ^done,value="{...}"
>>
>> so I am not sure what problem you are after.
>
> Hi Simon,
>
> So, this is a bit of an edge case, which I encountered with the
> chromium pretty printers. Effectively, it's when a pretty printer
> returns another object in its to_string, rather than a string.
>
> Consider the following code:
>
> struct Foo { int val; };
> struct Wrapper { Foo foo; };
>
> int main() {
> Wrapper w;
> w.foo.val = 23;
> }
>
> and this pretty printer file:
>
> import gdb.printing
>
> class FooPrinter:
> def __init__(self, val):
> self.val = val
> def to_string(self):
> return "Foo" + str(self.val["val"])
>
> class WrapperPrinter:
> def __init__(self, val):
> self.val = val
> def to_string(self):
> return self.val["foo"]
>
> test_printer = gdb.printing.RegexpCollectionPrettyPrinter("test")
> test_printer.add_printer('Foo', '^Foo$', FooPrinter)
> test_printer.add_printer('Wrapper', '^Wrapper$', WrapperPrinter)
>
> gdb.printing.register_pretty_printer(None, test_printer)
>
> Setting a breakpoint at the end of the function, we call the following commands:
>
> -enable-pretty-printing
> ^done
>
> -var-create var_w @ w
> ^done,name="var_w",numchild="0",value="{val =
> 23}",type="Wrapper",dynamic="1",has_more="0"
> -var-create var_w_foo @ w.foo
> ^done,name="var_w_foo",numchild="0",value="Foo23",type="Foo",dynamic="1",has_more="0"
>
> -var-evaluate-expression var_w
> ^done,value="{val = 23}"
> -var-evaluate-expression var_w_foo
> ^done,value="Foo23"
>
> -data-evaluate-expression w
> ^done,value="Foo23"
> -data-evaluate-expression w.foo
> ^done,value="Foo23"
>
> So, in the -var-evaluate-expression var_w case, we print the "raw"
> value of w.foo, while in the -data-evaluate-expression w case, we
> print the pretty printed w.foo value. After my patch, all of the above
> print "Foo23".
>
> Note that this isn't encountered very often, probably because it
> disappears if I replace `return self.val["foo"]` with `return
> str(self.val["foo"])`. But, it does feel like the wrong behaviour.
Thanks for the explanation, the problem is now very clear, and I also think the current
behavior is not the right one. It's documented that to_string can return a gdb.Value,
and there's no reason why we shouldn't apply pretty printers to this value. I put your
explanation in an updated patch below.
Also, from what I understand we don't have a test for this, so I converted your example
to a test case. If you are fine, I would push the following patch.
From f52e7d0ce13095db5e73466a55fe6752de46a5ee Mon Sep 17 00:00:00 2001
From: Leszek Swirski via gdb-patches <gdb-patches@sourceware.org>
Date: Thu, 1 Feb 2018 23:23:28 -0500
Subject: [PATCH] MI: Allow non-raw varobj evaluation
Make the MI variable object expression evaluation, with the
-var-evaluate-expression command, recursively call pretty printers, to
match the output of normal expression printing.
Consider the following code:
struct Foo { int val; };
struct Wrapper { Foo foo; };
int main() {
Wrapper w;
w.foo.val = 23;
}
and this pretty printer file:
import gdb.printing
class FooPrinter:
def __init__(self, val):
self.val = val
def to_string(self):
return "Foo" + str(self.val["val"])
class WrapperPrinter:
def __init__(self, val):
self.val = val
def to_string(self):
return self.val["foo"]
test_printer = gdb.printing.RegexpCollectionPrettyPrinter("test")
test_printer.add_printer('Foo', '^Foo$', FooPrinter)
test_printer.add_printer('Wrapper', '^Wrapper$', WrapperPrinter)
gdb.printing.register_pretty_printer(None, test_printer)
Setting a breakpoint at the end of the function, we call the following commands:
-enable-pretty-printing
^done
-var-create var_w @ w
^done,name="var_w",numchild="0",value="{val = 23}",type="Wrapper",dynamic="1",has_more="0"
-var-create var_w_foo @ w.foo
^done,name="var_w_foo",numchild="0",value="Foo23",type="Foo",dynamic="1",has_more="0"
-var-evaluate-expression var_w
^done,value="{val = 23}"
-var-evaluate-expression var_w_foo
^done,value="Foo23"
-data-evaluate-expression w
^done,value="Foo23"
-data-evaluate-expression w.foo
^done,value="Foo23"
So, in the -var-evaluate-expression var_w case, we print the "raw" value
of w.foo, while in the -data-evaluate-expression w case, we print the
pretty printed w.foo value. After this patch, all of the above print
"Foo23".
gdb/ChangeLog:
* varobj.c (varobj_formatted_print_options): Allow recursive
pretty printing if pretty printing is enabled.
gdb/testsuite/ChangeLog:
* gdb.python/py-prettyprint.c
(struct to_string_returns_value_inner,
struct to_string_returns_value_wrapper): New.
(main): Add tsrvw variable.
* gdb.python/py-prettyprint.py (ToStringReturnsValueInner,
ToStringReturnsValueWrapper): New classes.
(register_pretty_printers): Register new pretty-printers.
* gdb.python/py-prettyprint.exp (run_lang_tests): Test printing
recursive pretty printer.
* gdb.python/py-mi.exp: Likewise.
---
gdb/ChangeLog | 5 +++++
gdb/testsuite/ChangeLog | 14 ++++++++++++++
gdb/testsuite/gdb.python/py-mi.exp | 10 ++++++++++
gdb/testsuite/gdb.python/py-prettyprint.c | 11 +++++++++++
gdb/testsuite/gdb.python/py-prettyprint.exp | 4 ++++
gdb/testsuite/gdb.python/py-prettyprint.py | 26 +++++++++++++++++++++++++-
gdb/varobj.c | 2 +-
7 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7081f9ec53..3e384b5c29 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-01-24 Leszek Swirski <leszeks@google.com>
+
+ * varobj.c (varobj_formatted_print_options): Allow recursive
+ pretty printing if pretty printing is enabled.
+
2018-02-01 Leszek Swirski <leszeks@google.com>
* c-exp.y (lex_one_token, classify_name, yylex): Don't classify
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1558ed18fe..0251584448 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,17 @@
+2018-02-01 Simon Marchi <simon.marchi@polymtl.ca>
+ Leszek Swirski <leszeks@google.com>
+
+ * gdb.python/py-prettyprint.c
+ (struct to_string_returns_value_inner,
+ struct to_string_returns_value_wrapper): New.
+ (main): Add tsrvw variable.
+ * gdb.python/py-prettyprint.py (ToStringReturnsValueInner,
+ ToStringReturnsValueWrapper): New classes.
+ (register_pretty_printers): Register new pretty-printers.
+ * gdb.python/py-prettyprint.exp (run_lang_tests): Test printing
+ recursive pretty printer.
+ * gdb.python/py-mi.exp: Likewise.
+
2018-02-01 Leszek Swirski <leszeks@google.com>
* gdb.cp/filename.cc, gdb.cp/filename.exp: Test that member
diff --git a/gdb/testsuite/gdb.python/py-mi.exp b/gdb/testsuite/gdb.python/py-mi.exp
index bbe1266724..f9829478f0 100644
--- a/gdb/testsuite/gdb.python/py-mi.exp
+++ b/gdb/testsuite/gdb.python/py-mi.exp
@@ -295,6 +295,16 @@ mi_gdb_test "-var-evaluate-expression me" \
mi_create_dynamic_varobj children_as_list children_as_list 1 \
"printer whose children are returned as a list"
+# Test that when a pretty-printer returns a gdb.Value in its to_string, we call
+# the pretty-printer of that value too.
+mi_create_varobj_checked tsrvw tsrvw \
+ "struct to_string_returns_value_wrapper" \
+ "create tsrvw varobj"
+mi_check_varobj_value tsrvw "Inner to_string 1989" "check tsrvw varobj value"
+mi_gdb_test "-data-evaluate-expression tsrvw" \
+ "\\^done,value=\"Inner to_string 1989\"" \
+ "check tsrvw expression value"
+
# Regression test for bug 14741.
mi_continue_to_line \
[gdb_get_line_number {breakpoint bug 14741} ${srcfile}] \
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.c b/gdb/testsuite/gdb.python/py-prettyprint.c
index fc0d829e04..35ef5e0756 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.c
+++ b/gdb/testsuite/gdb.python/py-prettyprint.c
@@ -112,6 +112,16 @@ class Fake
};
#endif
+struct to_string_returns_value_inner
+{
+ int val;
+};
+
+struct to_string_returns_value_wrapper
+{
+ struct to_string_returns_value_inner inner;
+};
+
struct substruct {
int a;
int b;
@@ -284,6 +294,7 @@ main ()
struct lazystring estring, estring2, estring3;
struct hint_error hint_error;
struct children_as_list children_as_list;
+ struct to_string_returns_value_wrapper tsrvw = { { 1989 } };
nstype.elements = narray;
nstype.len = 0;
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.exp b/gdb/testsuite/gdb.python/py-prettyprint.exp
index 5df6f69158..2671cb8471 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.exp
+++ b/gdb/testsuite/gdb.python/py-prettyprint.exp
@@ -64,6 +64,10 @@ proc run_lang_tests {exefile lang} {
gdb_test "print arraystruct" " = {$nl *y = 7, *$nl *x = { a=<23> b=<$hex>, a=<24> b=<$hex>} *$nl *}"
+ # Test that when a pretty-printer returns a gdb.Value in its to_string, we
+ # call the pretty-printer of that value too.
+ gdb_test "print tsrvw" " = Inner to_string 1989"
+
if {$lang == "c++"} {
gdb_test "print cps" "= a=<8> b=<$hex>"
gdb_test "print cpss" " = {$nl *zss = 9, *$nl *s = a=<10> b=<$hex>$nl}"
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.py b/gdb/testsuite/gdb.python/py-prettyprint.py
index ea6caaea43..7357f05cc9 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.py
+++ b/gdb/testsuite/gdb.python/py-prettyprint.py
@@ -84,6 +84,25 @@ class NoStringContainerPrinter (object):
def children(self):
return _iterator_except (self.val['elements'], self.val['len'])
+# See ToStringReturnsValueWrapper.
+class ToStringReturnsValueInner:
+
+ def __init__(self, val):
+ self.val = val
+
+ def to_string(self):
+ return 'Inner to_string {}'.format(int(self.val['val']))
+
+# Test a printer that returns a gdb.Value in its to_string. That gdb.Value
+# also has its own pretty-printer.
+class ToStringReturnsValueWrapper:
+
+ def __init__(self, val):
+ self.val = val
+
+ def to_string(self):
+ return self.val['inner']
+
class pp_s (object):
def __init__(self, val):
self.val = val
@@ -316,7 +335,12 @@ def register_pretty_printers ():
pretty_printers_dict[re.compile ('^string_repr$')] = string_print
pretty_printers_dict[re.compile ('^container$')] = ContainerPrinter
pretty_printers_dict[re.compile ('^justchildren$')] = NoStringContainerPrinter
-
+
+ pretty_printers_dict[re.compile ('^struct to_string_returns_value_inner$')] = ToStringReturnsValueInner
+ pretty_printers_dict[re.compile ('^to_string_returns_value_inner$')] = ToStringReturnsValueInner
+ pretty_printers_dict[re.compile ('^struct to_string_returns_value_wrapper$')] = ToStringReturnsValueWrapper
+ pretty_printers_dict[re.compile ('^to_string_returns_value_wrapper$')] = ToStringReturnsValueWrapper
+
pretty_printers_dict[re.compile ('^struct ns$')] = pp_ns
pretty_printers_dict[re.compile ('^ns$')] = pp_ns
diff --git a/gdb/varobj.c b/gdb/varobj.c
index b6a2d8f369..f23243f3b7 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2274,7 +2274,7 @@ varobj_formatted_print_options (struct value_print_options *opts,
{
get_formatted_print_options (opts, format_code[(int) format]);
opts->deref_ref = 0;
- opts->raw = 1;
+ opts->raw = !pretty_printing;
}
std::string
--
2.16.1
^ permalink raw reply [flat|nested] 6+ messages in thread