Re: [PATCH] bound_registers.py: Add support for Python 3

[Luis Machado <lgustavo@codesourcery.com>]

On 11/26/2016 10:27 AM, Jonah Graham wrote:
> On 23 November 2016 at 23:09, Jonah Graham <jonah@kichwacoders.com> wrote:
>> On 23 November 2016 at 23:04, Luis Machado <lgustavo@codesourcery.com> wrote:
>>> On 11/23/2016 04:39 PM, Jonah Graham wrote:
>>>>
>>>> On 23 November 2016 at 16:59, Luis Machado <lgustavo@codesourcery.com>
>>>> wrote:
>>>>>
>>>>> On 11/20/2016 02:45 PM, Jonah Graham wrote:
>>>>>>
>>>>>>
>>>>>> gdb/Changelog:
>>>>>>
>>>>>>         * python/lib/gdb/printer/bound_registers.py: Add support
>>>>>>         for Python 3.
>>>>>> ---
>>>>>>  gdb/ChangeLog                                 | 5 +++++
>>>>>>  gdb/python/lib/gdb/printer/bound_registers.py | 5 +++++
>>>>>>  2 files changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>>>>>> index 3797e8b..1923888 100644
>>>>>> --- a/gdb/ChangeLog
>>>>>> +++ b/gdb/ChangeLog
>>>>>> @@ -1,3 +1,8 @@
>>>>>> +2016-11-20  Jonah Graham  <jonah@kichwacoders.com>
>>>>>> +
>>>>>> +       * python/lib/gdb/printer/bound_registers.py: Add support
>>>>>> +       for Python 3.
>>>>>> +
>>>>>>  2016-11-19  Joel Brobecker  <brobecker@adacore.com>
>>>>>>
>>>>>>         * contrib/ari/gdb_ari.sh: Add detection of printf_vma and
>>>>>> diff --git a/gdb/python/lib/gdb/printer/bound_registers.py
>>>>>> b/gdb/python/lib/gdb/printer/bound_registers.py
>>>>>> index 9ff94aa..e91cc19 100644
>>>>>> --- a/gdb/python/lib/gdb/printer/bound_registers.py
>>>>>> +++ b/gdb/python/lib/gdb/printer/bound_registers.py
>>>>>> @@ -16,6 +16,11 @@
>>>>>>
>>>>>>  import gdb.printing
>>>>>>
>>>>>> +if sys.version_info[0] > 2:
>>>>>> +    # Python 3 removed basestring and long
>>>>>> +    basestring = str
>>>>>> +    long = int
>>>>>> +
>>>>>>  class MpxBound128Printer:
>>>>>>      """Adds size field to a mpx __gdb_builtin_type_bound128 type."""
>>>>>>
>>>>>>
>>>>>
>>>>> Since this should apply to every python module, maybe this should be
>>>>> defined
>>>>> inside gdb/python/lib/gdb/__init__.py?
>>>>>
>>>>> I'm assuming it gets included by module "gdb" and therefore "long" is
>>>>> defined to be "int" for Python versions > 3.
>>>>>
>>>>> Then we can remove that code from gdb/python/lib/gdb/printing.py (and
>>>>> from
>>>>> other places setting this type of version-specific adjustment).
>>>>>
>>>>> Does that make sense?
>>>>
>>>>
>>>> Hi Luis,
>>>>
>>>> I am not sure I understand how to do it in __init__.py as the modules
>>>> that wanted to be both Python 2 and Python 3 compatible would still
>>>> have to have special code. AFAIK an __init__.py cannot change the
>>>> value of long in other py files. So python/lib/gdb/printing.py and
>>>> python/lib/gdb/printer/bound_registers.py would still need special
>>>> case code locally, even if that special case was an import for a
>>>> definition of long.
>>>
>>>
>>> That was the point i wasn't sure about (not a Python expert). I see
>>> conditional code executing if the version is > 3, but i wasn't sure if it
>>> would take effect globally when a module is included.
>>>
>>> In that case, i suppose there is not much that can be done other than
>>> duplicate the code.
>>>
>>
>> There is no way I know of, as a reference
>> http://python3porting.com/differences.html#long recommends doing it
>> (roughly) the way already done in python/lib/gdb/printing.py and
>> copied to python/lib/gdb/printer/bound_registers.py
>>
>> Thanks for the review,
>> Jonah
>
> Hi,
>
> Is there more I am supposed to do to get this patch into GDB? If so,
> please advise and I would be happy to.

One of the maintainers needs to OK it, which shouldn't that that long. 
Given the size of the patch, i don't there's anything else that should 
be done code-wise.