Use is/is not to check for None in python.

Message ID 20210607225044.486370-1-lsix@lancelotsix.com
State New
Headers show
Series
  • Use is/is not to check for None in python.
Related show

Commit Message

Lancelot SIX via Gdb-patches June 7, 2021, 10:50 p.m.
While reviewing a patch sent to the mailing list, I noticed there are few
places where python code checks if a variable is 'None' or not by using the
comparison operators '==' and '!='.  PEP8[1], which is used as coding standard
in GDB coding standards, recommends using 'is' / 'is not' when comparing to a
singleton such as 'None'.

This patch proposes to change the instances of '== None' by 'is None' and
'!= None' by 'is not None'.

[1] https://www.python.org/dev/peps/pep-0008/

gdb/doc/ChangeLog:

	* python.texi (Writing a Pretty-Printer): Use 'is None' instead of
	'== None'.

gdb/ChangeLog:

	* python/lib/gdb/FrameDecorator.py (FrameDecorator): Use 'is None' instead of
	'== None'.
	(FrameVars): Use 'is not None' instead of '!= None'.
	* python/lib/gdb/command/frame_filters.py (SetFrameFilterPriority): Use 'is None'
	instead of '== None' and 'is not None' instead of '!= None'.

gdb/testsuite/ChangeLog:

	* gdb.base/premature-dummy-frame-removal.py (TestUnwinder): Use
	'is None' instead of '== None' and 'is not None' instead of
	'!= None'.
	* gdb.python/py-frame-args.py (lookup_function): Same.
	* gdb.python/py-framefilter-invalidarg.py (Reverse_Function): Same.
	* gdb.python/py-framefilter.py (Reverse_Function): Same.
	* gdb.python/py-nested-maps.py (lookup_function): Same.
	* gdb.python/py-objfile-script-gdb.py (lookup_function): Same.
	* gdb.python/py-prettyprint.py (lookup_function): Same.
	* gdb.python/py-section-script.py (lookup_function): Same.
	* gdb.python/py-unwind-inline.py (dummy_unwinder): Same.
	* gdb.python/python.exp: Same.
	* gdb.rust/pp.py (lookup_function): Same.
---
 gdb/doc/python.texi                                    |  2 +-
 gdb/python/lib/gdb/FrameDecorator.py                   | 10 +++++-----
 gdb/python/lib/gdb/command/frame_filters.py            |  4 ++--
 .../gdb.base/premature-dummy-frame-removal.py          |  4 ++--
 gdb/testsuite/gdb.python/py-frame-args.py              |  2 +-
 gdb/testsuite/gdb.python/py-framefilter-invalidarg.py  |  2 +-
 gdb/testsuite/gdb.python/py-framefilter.py             |  2 +-
 gdb/testsuite/gdb.python/py-nested-maps.py             |  4 ++--
 gdb/testsuite/gdb.python/py-objfile-script-gdb.py      |  2 +-
 gdb/testsuite/gdb.python/py-prettyprint.py             |  4 ++--
 gdb/testsuite/gdb.python/py-section-script.py          |  2 +-
 gdb/testsuite/gdb.python/py-unwind-inline.py           |  2 +-
 gdb/testsuite/gdb.python/python.exp                    |  2 +-
 gdb/testsuite/gdb.rust/pp.py                           |  2 +-
 14 files changed, 22 insertions(+), 22 deletions(-)

-- 
2.31.1

Comments

Lancelot SIX via Gdb-patches June 8, 2021, 1:12 a.m. | #1
On 07 Jun 2021 23:50, Lancelot SIX via Gdb-patches wrote:
> While reviewing a patch sent to the mailing list, I noticed there are few

> places where python code checks if a variable is 'None' or not by using the

> comparison operators '==' and '!='.  PEP8[1], which is used as coding standard

> in GDB coding standards, recommends using 'is' / 'is not' when comparing to a

> singleton such as 'None'.


this is correct, so all the changes look fine.  but i wonder if we couldn't
make many more pythonic by treating them as bools.  for example:

> --- a/gdb/testsuite/gdb.python/py-framefilter-invalidarg.py

> +++ b/gdb/testsuite/gdb.python/py-framefilter-invalidarg.py

> @@ -28,7 +28,7 @@ class Reverse_Function(FrameDecorator):

>  

>      def function(self):

>          fname = str(self.fobj.function())

> -        if fname == None or fname == "":

> +        if fname is None or fname == "":


this is simply:
	if not fname:
-mike
Lancelot SIX via Gdb-patches June 8, 2021, 4:25 p.m. | #2
On 2021-06-07 9:12 p.m., Mike Frysinger via Gdb-patches wrote:
> On 07 Jun 2021 23:50, Lancelot SIX via Gdb-patches wrote:

>> While reviewing a patch sent to the mailing list, I noticed there are few

>> places where python code checks if a variable is 'None' or not by using the

>> comparison operators '==' and '!='.  PEP8[1], which is used as coding standard

>> in GDB coding standards, recommends using 'is' / 'is not' when comparing to a

>> singleton such as 'None'.

> 

> this is correct, so all the changes look fine.  but i wonder if we couldn't

> make many more pythonic by treating them as bools.  for example:


The patch LGTM but I also agree with Mike.  So Lancelot, if you want to
merge it as-is, that's fine.  If you want to update it as suggested by
Mike, that's fine too.

Simon
Lancelot SIX via Gdb-patches June 8, 2021, 11:01 p.m. | #3
On Tue, Jun 08, 2021 at 12:25:37PM -0400, Simon Marchi wrote:
> On 2021-06-07 9:12 p.m., Mike Frysinger via Gdb-patches wrote:

> > On 07 Jun 2021 23:50, Lancelot SIX via Gdb-patches wrote:

> >> While reviewing a patch sent to the mailing list, I noticed there are few

> >> places where python code checks if a variable is 'None' or not by using the

> >> comparison operators '==' and '!='.  PEP8[1], which is used as coding standard

> >> in GDB coding standards, recommends using 'is' / 'is not' when comparing to a

> >> singleton such as 'None'.

> > 

> > this is correct, so all the changes look fine.  but i wonder if we couldn't

> > make many more pythonic by treating them as bools.  for example:

> 

> The patch LGTM but I also agree with Mike.  So Lancelot, if you want to

> merge it as-is, that's fine.  If you want to update it as suggested by

> Mike, that's fine too.

> 

> Simon

> 


Hi,

Thanks.

I have pushd the patch with minor modifications for the case Mike
pointed out.

After looking again, the pattern was:

    fname = str(self.fobj.function())
	  if fname is None or fname == "":
		    return None

but in this situation fname can not be None.  Worst case scenario, if
`self.fobj.function()` returns None, then `fname == 'None'`, so the
`is None` check is useless anyway.

For the rest, of the changes, I kept 'is None' / 'is not None'.

Lancelot.

Patch

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 7b7f0692b3a..ab934a8c012 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1661,7 +1661,7 @@  example above might be written.
 @smallexample
 def str_lookup_function(val):
     lookup_tag = val.type.tag
-    if lookup_tag == None:
+    if lookup_tag is None:
         return None
     regex = re.compile("^std::basic_string<char,.*>$")
     if regex.match(lookup_tag):
diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index 1f4b2cab6ec..69a0a9a4d7f 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -117,7 +117,7 @@  class FrameDecorator(object):
         # address.  If GDB detects an integer value from this function
         # it will attempt to find the function name from minimal
         # symbols via its own internal functions.
-        if func == None:
+        if func is None:
             pc = frame.pc()
             return pc
 
@@ -270,7 +270,7 @@  class FrameVars(object):
         except RuntimeError:
             block = None
 
-        while block != None:
+        while block is not None:
             if block.is_global or block.is_static:
                 break
             for sym in block:
@@ -295,12 +295,12 @@  class FrameVars(object):
         except RuntimeError:
             block = None
 
-        while block != None:
-            if block.function != None:
+        while block is not None:
+            if block.function is not None:
                 break
             block = block.superblock
 
-        if block != None:
+        if block is not None:
             for sym in block:
                 if not sym.is_argument:
                     continue
diff --git a/gdb/python/lib/gdb/command/frame_filters.py b/gdb/python/lib/gdb/command/frame_filters.py
index 91e8ca2d43c..97488ca2c3d 100644
--- a/gdb/python/lib/gdb/command/frame_filters.py
+++ b/gdb/python/lib/gdb/command/frame_filters.py
@@ -372,7 +372,7 @@  class SetFrameFilterPriority(gdb.Command):
 
     def invoke(self, arg, from_tty):
         command_tuple = self._parse_pri_arg(arg)
-        if command_tuple != None:
+        if command_tuple is not None:
             self._set_filter_priority(command_tuple)
 
 
@@ -453,7 +453,7 @@  class ShowFrameFilterPriority(gdb.Command):
 
     def invoke(self, arg, from_tty):
         command_tuple = self._parse_pri_arg(arg)
-        if command_tuple == None:
+        if command_tuple is None:
             return
         filter_name = command_tuple[1]
         list_name = command_tuple[0]
diff --git a/gdb/testsuite/gdb.base/premature-dummy-frame-removal.py b/gdb/testsuite/gdb.base/premature-dummy-frame-removal.py
index 31936658788..fbad6d447d3 100644
--- a/gdb/testsuite/gdb.base/premature-dummy-frame-removal.py
+++ b/gdb/testsuite/gdb.base/premature-dummy-frame-removal.py
@@ -46,10 +46,10 @@  class TestUnwinder(Unwinder):
         sp = pending_frame.read_register(sp_desc)
 
         block = gdb.block_for_pc(int(pc))
-        if block == None:
+        if block is None:
             return None
         func = block.function
-        if func == None:
+        if func is None:
             return None
         if str(func) != "break_bt_here":
             return None
diff --git a/gdb/testsuite/gdb.python/py-frame-args.py b/gdb/testsuite/gdb.python/py-frame-args.py
index 32d7204a5bd..59dd14d0b56 100644
--- a/gdb/testsuite/gdb.python/py-frame-args.py
+++ b/gdb/testsuite/gdb.python/py-frame-args.py
@@ -53,7 +53,7 @@  def lookup_function(val):
 
     # Get the type name.
     typename = type.tag
-    if typename == None:
+    if typename is None:
         return None
 
     # Iterate over local dictionary of types to determine
diff --git a/gdb/testsuite/gdb.python/py-framefilter-invalidarg.py b/gdb/testsuite/gdb.python/py-framefilter-invalidarg.py
index 609ff80a88f..1a4925ba092 100644
--- a/gdb/testsuite/gdb.python/py-framefilter-invalidarg.py
+++ b/gdb/testsuite/gdb.python/py-framefilter-invalidarg.py
@@ -28,7 +28,7 @@  class Reverse_Function(FrameDecorator):
 
     def function(self):
         fname = str(self.fobj.function())
-        if fname == None or fname == "":
+        if fname is None or fname == "":
             return None
         if fname == "end_func":
             extra = self.fobj.inferior_frame().read_var("str").string()
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index ce5a35d17a9..9e90c8ba5d0 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -28,7 +28,7 @@  class Reverse_Function(FrameDecorator):
 
     def function(self):
         fname = str(self.fobj.function())
-        if fname == None or fname == "":
+        if fname is None or fname == "":
             return None
         if fname == "end_func":
             extra = self.fobj.inferior_frame().read_var("str").string()
diff --git a/gdb/testsuite/gdb.python/py-nested-maps.py b/gdb/testsuite/gdb.python/py-nested-maps.py
index 163fc865866..2848347c56a 100644
--- a/gdb/testsuite/gdb.python/py-nested-maps.py
+++ b/gdb/testsuite/gdb.python/py-nested-maps.py
@@ -88,7 +88,7 @@  def lookup_function(val):
     # Get the type name.
     typename = type.tag
 
-    if typename == None:
+    if typename is None:
         return None
 
     # Iterate over local dictionary of types to determine
@@ -109,7 +109,7 @@  def lookup_typedefs_function(val):
     # Get the type.
     type = val.type
 
-    if type == None or type.name == None or type.code != gdb.TYPE_CODE_TYPEDEF:
+    if type is None or type.name is None or type.code != gdb.TYPE_CODE_TYPEDEF:
         return None
 
     # Iterate over local dictionary of typedef types to determine if a
diff --git a/gdb/testsuite/gdb.python/py-objfile-script-gdb.py b/gdb/testsuite/gdb.python/py-objfile-script-gdb.py
index 88372e41a19..45d036ccf0b 100644
--- a/gdb/testsuite/gdb.python/py-objfile-script-gdb.py
+++ b/gdb/testsuite/gdb.python/py-objfile-script-gdb.py
@@ -42,7 +42,7 @@  def lookup_function(val):
     # Get the type name.
     typename = type.tag
 
-    if typename == None:
+    if typename is None:
         return None
 
     # Iterate over local dictionary of types to determine
diff --git a/gdb/testsuite/gdb.python/py-prettyprint.py b/gdb/testsuite/gdb.python/py-prettyprint.py
index 84dbc3ba3a4..89ffc0f8d18 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint.py
+++ b/gdb/testsuite/gdb.python/py-prettyprint.py
@@ -314,7 +314,7 @@  def lookup_function(val):
     # Get the type name.
     typename = type.tag
 
-    if typename == None:
+    if typename is None:
         return None
 
     # Iterate over local dictionary of types to determine
@@ -344,7 +344,7 @@  def lookup_typedefs_function(val):
     # Get the type.
     type = val.type
 
-    if type == None or type.name == None or type.code != gdb.TYPE_CODE_TYPEDEF:
+    if type is None or type.name is None or type.code != gdb.TYPE_CODE_TYPEDEF:
         return None
 
     # Iterate over local dictionary of typedef types to determine if a
diff --git a/gdb/testsuite/gdb.python/py-section-script.py b/gdb/testsuite/gdb.python/py-section-script.py
index aac70a0a8f5..8562e52a5c3 100644
--- a/gdb/testsuite/gdb.python/py-section-script.py
+++ b/gdb/testsuite/gdb.python/py-section-script.py
@@ -42,7 +42,7 @@  def lookup_function(val):
     # Get the type name.
     typename = type.tag
 
-    if typename == None:
+    if typename is None:
         return None
 
     # Iterate over local dictionary of types to determine
diff --git a/gdb/testsuite/gdb.python/py-unwind-inline.py b/gdb/testsuite/gdb.python/py-unwind-inline.py
index 3042472bff5..2892f41f27e 100644
--- a/gdb/testsuite/gdb.python/py-unwind-inline.py
+++ b/gdb/testsuite/gdb.python/py-unwind-inline.py
@@ -45,7 +45,7 @@  class dummy_unwinder(Unwinder):
     def get_regs(self, pending_frame):
         """Return a list of register names that should be read.  Only
         gathers the list once, then caches the result."""
-        if self.regs != None:
+        if self.regs is not None:
             return self.regs
 
         # Collect the names of all registers to read.
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index c7d879efe63..d9fd60f3dd4 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -192,7 +192,7 @@  gdb_test "python print (a)" ".*aliases -- User-defined aliases of other commands
 # Test PR 12212, using InfThread.selected_thread() when no inferior is
 # loaded.
 gdb_py_test_silent_cmd "python nothread = gdb.selected_thread()" "Attempt to aquire thread with no inferior" 1
-gdb_test "python print (nothread == None)" "True" "ensure that no threads are returned"
+gdb_test "python print (nothread is None)" "True" "ensure that no threads are returned"
 
 gdb_test_multiline "register atexit function" \
     "python" "" \
diff --git a/gdb/testsuite/gdb.rust/pp.py b/gdb/testsuite/gdb.rust/pp.py
index 57c8cc3fbf3..a78e30065ba 100644
--- a/gdb/testsuite/gdb.rust/pp.py
+++ b/gdb/testsuite/gdb.rust/pp.py
@@ -38,7 +38,7 @@  def lookup_function(val):
     # Get the type name.
     typename = type.tag
 
-    if typename == None:
+    if typename is None:
         return None
 
     if typename == "pp::Inner":