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

[Jonah Graham <jonah@kichwacoders.com>]

On 26 November 2016 at 21:26, Luis Machado <lgustavo@codesourcery.com> wrote:
> 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.
>

Ping on this patch. Please let me know if there is anything else I can
do to get it in.