From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129426 invoked by alias); 2 Feb 2018 04:27:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 129413 invoked by uid 89); 2 Feb 2018 04:27:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=ns, 1989, Fake, 2846 X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Feb 2018 04:27:03 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w124Qtfn012140 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 1 Feb 2018 23:27:00 -0500 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 60EB51E48F; Thu, 1 Feb 2018 23:26:55 -0500 (EST) Subject: Re: [PATCH] MI: Allow non-raw varobj evaluation To: Leszek Swirski Cc: gdb-patches@sourceware.org References: <20180124173223.213808-1-leszeks@google.com> <9bc195e52123f5d878778ebebd074ee4@polymtl.ca> From: Simon Marchi Message-ID: Date: Fri, 02 Feb 2018 04:27:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 2 Feb 2018 04:26:55 +0000 X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg00030.txt.bz2 On 2018-01-25 06:11 AM, Leszek Swirski via gdb-patches wrote: > On Thu, Jan 25, 2018 at 2:53 AM, Simon Marchi 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 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 + + * varobj.c (varobj_formatted_print_options): Allow recursive + pretty printing if pretty printing is enabled. + 2018-02-01 Leszek Swirski * 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 + Leszek Swirski + + * 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 * 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