Re: [PATCH] [Python] Add a boolean parameter "python resolve-operators"

[Doug Evans <dje at google dot com>]

Siva Chandra writes:
 > The attached patch adds a boolean parameter "python
 > resolve-operators". It is implemented using GDB's Python API. Python
 > operators in GDB Python scripts will be resolved to overloaded C++
 > operators (if they exist) only if this parameter is "on".
 >
 > gdb/ChangeLog:
 >
 > 2015-03-07  Siva Chandra Reddy  <sivachandra@google.com>
 >
 >         * NEWS (New commands): Add entries for
 >         "set python resolve-operators" and
 >         "show python resolve-operators".
 >         * data-directory/Makefile.in (PYTHON_FILE_LIST): Add
 >         gdb/parameters/__init__.py and
 >         gdb/parameters/resolve_operators.py.
 >         * python/lib/gdb/__init__.py (packages): Add 'parameters'.
 >         * python/lib/gdb/parameters/__init__.py: New file.
 >         * python/lib/gdb/parameters/resolve_operators.py: New file.
 >         * python/py-value.c (resolve_to_overloaded_operators): New
 >         function.
 >         (valpy_binop): Invoke value_x_binop only if resolving Python
 >         operators to C++ operators is enabled.
 >
 > gdb/testsuite/ChangeLog:
 >
 > 2015-03-07  Siva Chandra Reddy  <sivachandra@google.com>
 >
 >         * gdb.python/py-resolve-operators.cc: New file.
 >         * gdb.python/py-resolve-operators.exp: New file.

Hi.

This will also need a doc patch.

It's interesting to implement the gdb parameter in python,
but it'd be simpler to do in C (see below),
so let's go with C.

Plus python parameters currently have the bug that they always
print something when set. IWBN to fix that before we start
implementing gdb parameters in python.

 > diff --git a/gdb/NEWS b/gdb/NEWS
 > index 49dc0e6..0e1a94a 100644
 > --- a/gdb/NEWS
 > +++ b/gdb/NEWS
 > @@ -28,6 +28,14 @@ record btrace bts
 >  record bts
 >    Start branch trace recording using Branch Trace Store (BTS) format.
 >
 > +show python resolve-operators
 > +  Show whether resolving Python operators (in Python scripts) to
 > +  overloaded C++ operators is enabled/disabled.
 > +
 > +set python resolve-operators
 > +  Enable/disable resolving Python operators (in Python scripts) to
 > +  overloaded C++ operators.
 > +
 >  * New options
 >
 >  set max-completions
 > diff --git a/gdb/data-directory/Makefile.in  
b/gdb/data-directory/Makefile.in
 > index c01b86d..eda541e 100644
 > --- a/gdb/data-directory/Makefile.in
 > +++ b/gdb/data-directory/Makefile.in
 > @@ -74,6 +74,8 @@ PYTHON_FILE_LIST = \
 >  	gdb/function/__init__.py \
 >  	gdb/function/caller_is.py \
 >  	gdb/function/strfns.py \
 > +	gdb/parameters/__init__.py \
 > +	gdb/parameters/resolve_operators.py \
 >  	gdb/printer/__init__.py \
 >  	gdb/printer/bound_registers.py
 >
 > diff --git a/gdb/python/lib/gdb/__init__.py  
b/gdb/python/lib/gdb/__init__.py
 > index 92b06f2..3152b00 100644
 > --- a/gdb/python/lib/gdb/__init__.py
 > +++ b/gdb/python/lib/gdb/__init__.py
 > @@ -82,6 +82,7 @@ PYTHONDIR = os.path.dirname(os.path.dirname(__file__))
 >  packages = [
 >      'function',
 >      'command',
 > +    'parameters',
 >      'printer'
 >  ]
 >
 > diff --git a/gdb/python/lib/gdb/parameters/__init__.py  
b/gdb/python/lib/gdb/parameters/__init__.py
 > new file mode 100644
 > index 0000000..97b8f7e
 > --- /dev/null
 > +++ b/gdb/python/lib/gdb/parameters/__init__.py
 > @@ -0,0 +1,14 @@
 > +# Copyright (C) 2015 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/>.
 > diff --git a/gdb/python/lib/gdb/parameters/resolve_operators.py  
b/gdb/python/lib/gdb/parameters/resolve_operators.py
 > new file mode 100644
 > index 0000000..7f67458
 > --- /dev/null
 > +++ b/gdb/python/lib/gdb/parameters/resolve_operators.py
 > @@ -0,0 +1,43 @@
 > +# Copyright (C) 2015 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/>.
 > +
 > +
 > +import gdb
 > +
 > +
 > +class ResolveOperators(gdb.Parameter):
 > +    def __init__(self):
 > +        self.set_doc = ('Enable/disable resolving Python operators to '
 > +                        'overloaded C++ operators.\n\n'
 > +                        'Usage:\n'
 > +                        '    set python resolve-operators <on|off>\n')
 > +        self.show_doc = ('Show whether Python operators will be  
resolved '
 > +                         'to overloaded C++ operators.\n\n'
 > +                         'Usage:\n'
 > +                         '    show python resolve-operators\n')
 > +        gdb.Parameter.__init__(self, 'python resolve-operators',
 > +                               gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
 > +        self.value = True
 > +
 > +    def get_set_string(self):
 > +        return ('Resolving Python operators to overloaded C++ operators  
is ' +
 > +                'set to "%s".' % ('on' if self.value else 'off'))
 > +
 > +    def get_show_string(self, svalue):
 > +        return ('Resolving Python operators to overloaded C++ operators  
is ' +
 > +                '"%s".' % ('on' if self.value else 'off'))
 > +
 > +
 > +ResolveOperators()
 > diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
 > index 5a13777..015bf8a 100644
 > --- a/gdb/python/py-value.c
 > +++ b/gdb/python/py-value.c
 > @@ -27,6 +27,7 @@
 >  #include "expression.h"
 >  #include "cp-abi.h"
 >  #include "python.h"
 > +#include "cli/cli-decode.h"
 >
 >  #include "python-internal.h"
 >
 > @@ -920,6 +921,23 @@ enum valpy_opcode
 >  #define STRIP_REFERENCE(TYPE) \
 >    ((TYPE_CODE (TYPE) == TYPE_CODE_REF) ? (TYPE_TARGET_TYPE (TYPE)) :  
(TYPE))
 >
 > +/* */

missing function comment

 > +
 > +static int
 > +resolve_to_overloaded_operators (void)
 > +{
 > +  const char *show_cmd = "show python resolve-operators";
 > +  struct cmd_list_element *alias, *prefix, *cmd;
 > +  int found = -1;
 > +  volatile struct gdb_exception except;
 > +
 > +  found = lookup_cmd_composition (show_cmd, &alias, &prefix, &cmd);
 > +  if (!found || !cmd->var || (cmd->var_type != var_boolean))

All of these are really internal errors.
They can happen when running gdb from the build dir and not
providing -D, or from a bad gdb installation.
It doesn't feel right to have a silent failure here though.
OTOH, we could just implement the parameter in C in this file
and then this issue goes away.

 > +    return 0;
 > +
 > +  return *((int *) cmd->var);
 > +}
 > +
 >  /* Returns a value object which is the result of applying the operation
 >     specified by OPCODE to the given arguments.  Returns NULL on error,  
with
 >     a python exception set.  */
 > @@ -1038,7 +1056,8 @@ valpy_binop (enum valpy_opcode opcode, PyObject  
*self, PyObject *other)
 >
 >        if (!handled)
 >  	{
 > -	  if (binop_user_defined_p (op, arg1, arg2))
 > +	  if (binop_user_defined_p (op, arg1, arg2)
 > +	      && resolve_to_overloaded_operators ())
 >  	    res_val = value_x_binop (arg1, arg2, op, OP_NULL, EVAL_NORMAL);
 >  	  else
 >  	    res_val = value_binop (arg1, arg2, op);
 > diff --git a/gdb/testsuite/gdb.python/py-resolve-operators.cc  
b/gdb/testsuite/gdb.python/py-resolve-operators.cc
 > new file mode 100644
 > index 0000000..b4630a7
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.python/py-resolve-operators.cc
 > @@ -0,0 +1,38 @@
 > +/* This testcase is part of GDB, the GNU debugger.
 > +
 > +   Copyright 2015 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/>.  */
 > +
 > +class A
 > +{
 > +public:
 > +  int operator+ (int i);
 > +
 > +  int a;
 > +};
 > +
 > +int
 > +A::operator+ (int i)
 > +{
 > +  return a + i;
 > +}
 > +
 > +int
 > +main (void)
 > +{
 > +  A a = { 10 };
 > +
 > +  return a + -10;  /*  Break here. */

extra space after /*

 > +}
 > diff --git a/gdb/testsuite/gdb.python/py-resolve-operators.exp  
b/gdb/testsuite/gdb.python/py-resolve-operators.exp
 > new file mode 100644
 > index 0000000..9d03352
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.python/py-resolve-operators.exp
 > @@ -0,0 +1,50 @@
 > +# Copyright 2015 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/>.
 > +
 > +# This file is part of the GDB testsuite.  It tests the debug methods
 > +# feature in the Python extension language.
 > +
 > +load_lib gdb-python.exp
 > +
 > +if { [skip_cplus_tests] } { continue }
 > +
 > +standard_testfile py-resolve-operators.cc

standard_testfile .cc

 > +
 > +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]}  
{
 > +    return -1
 > +}
 > +
 > +# Skip all tests if Python scripting is not enabled.
 > +if { [skip_python_tests] } { continue }
 > +
 > +if ![runto_main] {
 > +   return -1
 > +}
 > +
 > +gdb_breakpoint [gdb_get_line_number "Break here."]
 > +gdb_continue_to_breakpoint "Break here" ".*Break here.*"
 > +
 > +gdb_test_no_output "python a = gdb.parse_and_eval('a')" "make_a"
 > +
 > +gdb_test "show python resolve-operators" ".*on.*" "test_show_with_on"
 > +gdb_test "python print(a + 113)" "123" "print_a_with_on"
 > +
 > +gdb_test "set python resolve-operators off" ".*off.*" "test_set_to_off"
 > +gdb_test "python print(a + 113)" \
 > +    ".*gdb\.error: Argument to arithmetic operation not a number or  
boolean.*" \
 > +    "print_a_with_off"
 > +
 > +gdb_test "set python resolve-operators on" ".*on.*" "test_set_to_on"
 > +gdb_test "python print(a + 113)" "123" "print_a_with_on_again"