Re: [PATCH 1/5] gdb/python: handle saving user registers in a frame unwinder

[Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>]

On Mon, Jun 07, 2021 at 01:20:33PM -0400, Simon Marchi wrote:
> On 2021-06-07 1:07 p.m., Lancelot SIX via Gdb-patches wrote:
> > Hi,
> > 
> > I just have a minor stylistic remark in the python code in the test:
> > 
> >> […]
> >> +
> >> +    def __call__(self, pending_frame):
> >> +        pc_desc = pending_frame.architecture().registers().find("pc")
> >> +        pc = pending_frame.read_register(pc_desc)
> >> +
> >> +        sp_desc = pending_frame.architecture().registers().find("sp")
> >> +        sp = pending_frame.read_register(sp_desc)
> >> +
> >> +        block = gdb.block_for_pc(int(pc))
> >> +        if block == None:
> > 
> > When looking for None, it is usually prefered to use 'is None' instead
> > of '== None'.  The result is the same unless there is a strange overload
> > of __eq__.
> > 
> > This pattern can also be seen in patch 3 and 4 of your series (patch 4
> > using both '==' and 'is' to check for None).
> 
> I agree, that's the convention in Python.  It is not in our coding
> standards, but I suggest using flake8 to check the Python code, it
> reports this (and much more):

Hi,

Actually, this is mentioned in the PEP-8[1][2], which states in the
“Programming Recommandations” section:

    Comparisons to singletons like None should always be done with is or
		is not, never the equality operators.

This leads me to an annex question. Given that I still lack a lot of
experience with the overall codebase, I tend to pick this kind of small
stylistic details more easily than design and logic problems.  I do not
always point out those I see when I read the ML, but I can totally
understand those isolated stylistic comments can be considered as noise.
If so, please let me know!

> 
> 
>     $ flake8 testsuite/gdb.python/py-unwind-user-regs.py
>     testsuite/gdb.python/py-unwind-user-regs.py:52:18: E711 comparison to None should be 'if cond is None:'
>     testsuite/gdb.python/py-unwind-user-regs.py:55:17: E711 comparison to None should be 'if cond is None:'

I am currently running the testsuite against a patch that fixes those I
found. I’ll try to post it later tonight.

Lancelot.

> 
> Simon

[1] https://www.python.org/dev/peps/pep-0008/#programming-recommendations
[2] https://sourceware.org/gdb/wiki/Internals%20GDB-Python-Coding-Standards

-- 
Lancelot SIX