From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id 6428A3857C60 for ; Sun, 27 Dec 2020 06:20:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6428A3857C60 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 126A4116AA9; Sun, 27 Dec 2020 01:20:32 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id VBo0RC32YLeg; Sun, 27 Dec 2020 01:20:32 -0500 (EST) Received: from float.home (localhost.localdomain [127.0.0.1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id A7822116AA5; Sun, 27 Dec 2020 01:20:31 -0500 (EST) Received: by float.home (Postfix, from userid 1000) id CA15DA1608; Sun, 27 Dec 2020 10:20:26 +0400 (+04) Date: Sun, 27 Dec 2020 10:20:26 +0400 From: Joel Brobecker To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Pass void_context_p to parse_expression Message-ID: <20201227062026.GC1103585@adacore.com> References: <20201218151218.3078714-1-tromey@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201218151218.3078714-1-tromey@adacore.com> X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 27 Dec 2020 06:20:34 -0000 Hi Tom, On Fri, Dec 18, 2020 at 08:12:18AM -0700, Tom Tromey wrote: > An earlier patch pointed out that nothing in GDB sets void_context_p > when parsing an expression. This patch fixes this omission. > > "print" and "call" differ in that the former will print a value that > has void type, while the latter will not. AdaCore has had a patch for > a long time that uses this distinction to help with overload > resolution. In particular, in a "call" context, a procedure will be > chosen, while in a "print" context, a zero-argument function will be > chosen instead. > > Regression tested on x86-64 Fedora 32. > > gdb/ChangeLog > 2020-12-18 Tom Tromey > > * parse.c (parse_expression): Add void_context_p parameter. Use > parse_exp_in_context. > * printcmd.c (print_command_1): Change voidprint to bool. Pass to > parse_expression. > (print_command, call_command): Update. > * expression.h (parse_expression): Add void_context_p parameter. > > gdb/testsuite/ChangeLog > 2020-12-18 Tom Tromey > > * gdb.ada/voidctx/pck.adb: New file. > * gdb.ada/voidctx/pck.ads: New file. > * gdb.ada/voidctx/voidctx.adb: New file. > * gdb.ada/voidctx.exp: New file. Thanks for the patch. It looks good to me. In looking at the testcase, it inspired me to modify the testcase we have in house to use counters to keep track of which function gets called, when it gets called and how many times. What do you think of doing the same here? Thanks again! > --- > gdb/ChangeLog | 9 +++++++ > gdb/expression.h | 3 ++- > gdb/parse.c | 15 ++++++++--- > gdb/printcmd.c | 10 ++++--- > gdb/testsuite/ChangeLog | 7 +++++ > gdb/testsuite/gdb.ada/voidctx.exp | 33 +++++++++++++++++++++++ > gdb/testsuite/gdb.ada/voidctx/pck.adb | 26 ++++++++++++++++++ > gdb/testsuite/gdb.ada/voidctx/pck.ads | 19 +++++++++++++ > gdb/testsuite/gdb.ada/voidctx/voidctx.adb | 31 +++++++++++++++++++++ > 9 files changed, 144 insertions(+), 9 deletions(-) > create mode 100644 gdb/testsuite/gdb.ada/voidctx.exp > create mode 100644 gdb/testsuite/gdb.ada/voidctx/pck.adb > create mode 100644 gdb/testsuite/gdb.ada/voidctx/pck.ads > create mode 100644 gdb/testsuite/gdb.ada/voidctx/voidctx.adb > > diff --git a/gdb/expression.h b/gdb/expression.h > index c07111c316f..2abf2d280d6 100644 > --- a/gdb/expression.h > +++ b/gdb/expression.h > @@ -120,7 +120,8 @@ typedef std::unique_ptr expression_up; > > class innermost_block_tracker; > extern expression_up parse_expression (const char *, > - innermost_block_tracker * = nullptr); > + innermost_block_tracker * = nullptr, > + bool void_context_p = false); > > extern expression_up parse_expression_with_language (const char *string, > enum language lang); > diff --git a/gdb/parse.c b/gdb/parse.c > index 80a67c6c707..1d98a6d9e0c 100644 > --- a/gdb/parse.c > +++ b/gdb/parse.c > @@ -1158,13 +1158,20 @@ parse_exp_in_context (const char **stringptr, CORE_ADDR pc, > return result; > } > > -/* Parse STRING as an expression, and complain if this fails > - to use up all of the contents of STRING. */ > +/* Parse STRING as an expression, and complain if this fails to use up > + all of the contents of STRING. TRACKER, if non-null, will be > + updated by the parser. VOID_CONTEXT_P should be true to indicate > + that the expression may be expected to return a value with void > + type. Parsers are free to ignore this, or to use it to help with > + overload resolution decisions. */ > > expression_up > -parse_expression (const char *string, innermost_block_tracker *tracker) > +parse_expression (const char *string, innermost_block_tracker *tracker, > + bool void_context_p) > { > - expression_up exp = parse_exp_1 (&string, 0, 0, 0, tracker); > + expression_up exp = parse_exp_in_context (&string, 0, nullptr, 0, > + void_context_p, nullptr, > + tracker, nullptr); > if (*string) > error (_("Junk after end of expression.")); > return exp; > diff --git a/gdb/printcmd.c b/gdb/printcmd.c > index de083a2a768..2de5f32cc7c 100644 > --- a/gdb/printcmd.c > +++ b/gdb/printcmd.c > @@ -1206,7 +1206,7 @@ print_value (value *val, const value_print_options &opts) > /* Implementation of the "print" and "call" commands. */ > > static void > -print_command_1 (const char *args, int voidprint) > +print_command_1 (const char *args, bool voidprint) > { > struct value *val; > value_print_options print_opts; > @@ -1223,7 +1223,9 @@ print_command_1 (const char *args, int voidprint) > > if (exp != nullptr && *exp) > { > - expression_up expr = parse_expression (exp); > + /* VOIDPRINT is true to indicate that we do want to print a void > + value, so invert it for parse_expression. */ > + expression_up expr = parse_expression (exp, nullptr, !voidprint); > val = evaluate_expression (expr.get ()); > } > else > @@ -1321,14 +1323,14 @@ print_command_completer (struct cmd_list_element *ignore, > static void > print_command (const char *exp, int from_tty) > { > - print_command_1 (exp, 1); > + print_command_1 (exp, true); > } > > /* Same as print, except it doesn't print void results. */ > static void > call_command (const char *exp, int from_tty) > { > - print_command_1 (exp, 0); > + print_command_1 (exp, false); > } > > /* Implementation of the "output" command. */ > diff --git a/gdb/testsuite/gdb.ada/voidctx.exp b/gdb/testsuite/gdb.ada/voidctx.exp > new file mode 100644 > index 00000000000..7e38cad0f7e > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/voidctx.exp > @@ -0,0 +1,33 @@ > +# Copyright 2020 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 . > + > +load_lib "ada.exp" > + > +if { [skip_ada_tests] } { return -1 } > + > +standard_ada_testfile voidctx > + > +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } { > + return -1 > +} > + > +clean_restart ${testfile} > + > +set bp_location [gdb_get_line_number "STOP" ${testdir}/voidctx.adb] > +runto "voidctx.adb:$bp_location" > + > +gdb_test "print DoSomething" " = 42" > +gdb_test_no_output "call DoSomething" > + > diff --git a/gdb/testsuite/gdb.ada/voidctx/pck.adb b/gdb/testsuite/gdb.ada/voidctx/pck.adb > new file mode 100644 > index 00000000000..5b8bdfe6a29 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/voidctx/pck.adb > @@ -0,0 +1,26 @@ > +-- Copyright 2020 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 . > + > +package body Pck is > + function DoSomething return Integer is > + begin > + return 42; > + end; > + > + procedure DoSomething is > + begin > + null; > + end; > +end Pck; > diff --git a/gdb/testsuite/gdb.ada/voidctx/pck.ads b/gdb/testsuite/gdb.ada/voidctx/pck.ads > new file mode 100644 > index 00000000000..71d891a9bea > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/voidctx/pck.ads > @@ -0,0 +1,19 @@ > +-- Copyright 2020 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 . > + > +package Pck is > + function DoSomething return Integer; > + procedure DoSomething; > +end Pck; > diff --git a/gdb/testsuite/gdb.ada/voidctx/voidctx.adb b/gdb/testsuite/gdb.ada/voidctx/voidctx.adb > new file mode 100644 > index 00000000000..fcf7af73f05 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/voidctx/voidctx.adb > @@ -0,0 +1,31 @@ > +-- Copyright 2020 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 . > + > +with Pck; use Pck; > +procedure Voidctx is > + > + function DoSomething return Integer is > + begin > + return 42; > + end; > + > + procedure DoSomething is > + begin > + null; > + end; > + > +begin > + null; -- STOP > +end Voidctx; > -- > 2.26.2 -- Joel