]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
[gdb/python] Reimplement F405 fix
authorTom de Vries <tdevries@suse.de>
Mon, 2 Jun 2025 04:44:59 +0000 (06:44 +0200)
committerTom de Vries <tdevries@suse.de>
Mon, 2 Jun 2025 04:44:59 +0000 (06:44 +0200)
At commit 34b0776fd73^, flake8 reports the following F405 warnings:
...
$ pre-commit run flake8 --file gdb/python/lib/gdb/__init__.py
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

F405 'flush' may be undefined, or defined from star imports: _gdb
F405 'write' may be undefined, or defined from star imports: _gdb
F405 'STDOUT' may be undefined, or defined from star imports: _gdb
F405 'STDERR' may be undefined, or defined from star imports: _gdb
  ...
F405 'selected_inferior' may be undefined, or defined from star imports: _gdb
F405 'execute' may be undefined, or defined from star imports: _gdb
F405 'parameter' may be undefined, or defined from star imports: _gdb
...

The F405s are addressed by commit 34b0776fd73 ('Suppress some "undefined"
warnings from flake8').

The problem indicated by the first F405 is that the use of flush here:
...
class _GdbFile(object):
     ...
    def flush(self):
        flush(stream=self.stream)
...
cannot be verified by flake8.  It concludes that either, flush is undefined,
or it is defined by this "star import":
...
from _gdb import *  # noqa: F401,F403
...

In this particular case, indeed flush is defined by the star import.

This can be addressed by simply adding:
...
        flush(stream=self.stream)  # noqa: F405
...
but that has only effect for flake8, so other analyzers may report the same
problem.

The commit 34b0776fd73 addresses it instead by adding an "import _gdb" and
adding a "_gdb." prefix:
...
        _gdb.flush(stream=self.stream)
...

This introduces a second way to specify _gdb names, but the first one still
remains, and occasionally someone will use the first one, which then requires
fixing once flake8 is run [1].

While this works to silence the warnings, there is a problem: if a developer
makes a typo:
...
        _gdb.flash(stream=self.stream)
...
this is not detected by flake8.

This matters because although the python import already complains:
...
$ gdb -q -batch -ex "python import gdb"
Exception ignored in: <gdb._GdbFile object at 0x7f6186d4d7f0>
Traceback (most recent call last):
  File "__init__.py", line 63, in flush
    _gdb.flash(stream=self.stream)
AttributeError: module '_gdb' has no attribute 'flash'
...
that doesn't trigger if the code is hidden behind some control flow:
...
if _var_mostly_false:
    flash(stream=self.stream)
...

Instead, fix the F405s by reverting commit 34b0776fd73 and adding a second
import of _gdb alongside the star import which lists the names used locally:
...
 from _gdb import *  # noqa: F401,F403
+from _gdb import (
+    STDERR,
+    STDOUT,
+    Command,
+    execute,
+    flush,
+    parameter,
+    selected_inferior,
+    write,
+)
...

This gives the following warnings for the flash typo:
...
31:1: F401 '_gdb.flush' imported but unused
70:5: F811 redefinition of unused 'flush' from line 31
71:9: F405 'flash' may be undefined, or defined from star imports: _gdb
...

The benefits of this approach compared to the previous one are that:
- the typo is noticed, and
- when using a new name, the F405 fix needs to be done once (by adding it to
  the explicit import list), while previously the fix had to be applied to
  each use (by adding the "_gdb." prefix).

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>
[1] Commit 475799b692e ("Fix some pre-commit nits in gdb/__init__.py")

gdb/python/lib/gdb/__init__.py

index 03eb426b66243b4b16f9dffc1f50b8df6e524897..cedd897ab0f2727e0ec63f87e3c944660e18f428 100644 (file)
@@ -21,10 +21,20 @@ import traceback
 from contextlib import contextmanager
 from importlib import reload
 
-import _gdb
-
+# The star import imports _gdb names.  When the names are used locally, they
+# trigger F405 warnings unless added to the explicit import list.
 # Note that two indicators are needed here to silence flake8.
 from _gdb import *  # noqa: F401,F403
+from _gdb import (
+    STDERR,
+    STDOUT,
+    Command,
+    execute,
+    flush,
+    parameter,
+    selected_inferior,
+    write,
+)
 
 # isort: split
 
@@ -55,14 +65,14 @@ class _GdbFile(object):
             self.write(line)
 
     def flush(self):
-        _gdb.flush(stream=self.stream)
+        flush(stream=self.stream)
 
     def write(self, s):
-        _gdb.write(s, stream=self.stream)
+        write(s, stream=self.stream)
 
 
-sys.stdout = _GdbFile(_gdb.STDOUT)
-sys.stderr = _GdbFile(_gdb.STDERR)
+sys.stdout = _GdbFile(STDOUT)
+sys.stderr = _GdbFile(STDERR)
 
 # Default prompt hook does nothing.
 prompt_hook = None
@@ -184,7 +194,7 @@ def GdbSetPythonDirectory(dir):
 
 def current_progspace():
     "Return the current Progspace."
-    return _gdb.selected_inferior().progspace
+    return selected_inferior().progspace
 
 
 def objfiles():
@@ -221,14 +231,14 @@ def set_parameter(name, value):
             value = "on"
         else:
             value = "off"
-    _gdb.execute("set " + name + " " + str(value), to_string=True)
+    execute("set " + name + " " + str(value), to_string=True)
 
 
 @contextmanager
 def with_parameter(name, value):
     """Temporarily set the GDB parameter NAME to VALUE.
     Note that this is a context manager."""
-    old_value = _gdb.parameter(name)
+    old_value = parameter(name)
     set_parameter(name, value)
     try:
         # Nothing that useful to return.
@@ -406,7 +416,7 @@ class ParameterPrefix:
     # __doc__ string of its own, then sub-classes will inherit that __doc__
     # string, and GDB will not understand that it needs to generate one.
 
-    class _PrefixCommand(_gdb.Command):
+    class _PrefixCommand(Command):
         """A gdb.Command used to implement both the set and show prefixes.
 
         This documentation string is not used as the prefix command