[gdb/python] Add gdbpy_handle_gdb_exception

I've recently committed two patches:
- commit 2f8cd40c37a ("[gdb/python] Use GDB_PY_HANDLE_EXCEPTION more often")
- commit fbf8e4c35c2 ("[gdb/python] Use GDB_PY_SET_HANDLE_EXCEPTION more often")
which use the macros GDB_PY_HANDLE_EXCEPTION and GDB_PY_SET_HANDLE_EXCEPTION
more often, with the goal of making things more consistent.

Having done that, I wondered if a better approach could be possible.

Consider GDB_PY_HANDLE_EXCEPTION:
...
 /* Use this in a 'catch' block to convert the exception to a Python
    exception and return nullptr.  */
 #define GDB_PY_HANDLE_EXCEPTION(Exception)	\
   do {						\
     gdbpy_convert_exception (Exception);	\
     return nullptr;				\
   } while (0)
...

The macro nicely codifies how python handles exceptions:
- setting an error condition using some PyErr_Set* variant, and
- returning a value implying that something went wrong
presumably with the goal that using the macro will mean not accidentally:
- forgetting to return on error, or
- returning the wrong value on error.

The problems are that:
- the macro hides control flow, specifically the return statement, and
- the macro hides the return value.

For example, when reading somewhere:
...
  catch (const gdb_exception &except)
    {
      GDB_PY_HANDLE_EXCEPTION (except);
    }
...
in order to understand what this does, you have to know that the macro
returns, and that it returns nullptr.

Add a template gdbpy_handle_gdb_exception:
...
template<typename T>
[[nodiscard]] T
gdbpy_handle_gdb_exception (T val, const gdb_exception &e)
{
  gdbpy_convert_exception (e);
  return val;
}
...
which can be used instead:
...
  catch (const gdb_exception &except)
    {
      return gdbpy_handle_gdb_exception (nullptr, except);
    }
...

[ Initially I tried this:
...
template<auto val>
[[nodiscard]] auto
gdbpy_handle_gdb_exception (const gdb_exception &e)
{
  gdbpy_convert_exception (e);
  return val;
}
...
with which the usage is slightly better looking:
...
  catch (const gdb_exception &except)
    {
      return gdbpy_handle_gdb_exception<nullptr> (except);
    }
...
but I ran into trouble with older gcc compilers. ]

While still a single statement, we now have it clear:
- that the statement returns,
- what value the statement returns.

[ FWIW, this could also be handled by say:
...
-      GDB_PY_HANDLE_EXCEPTION (except);
+      GDB_PY_HANDLE_EXCEPTION_AND_RETURN_VAL (except, nullptr);
...
but I still didn't find the fact that it returns easy to spot.

Alternatively, this is the simplest form we could use:
...
      return gdbpy_convert_exception (e), nullptr;
...
but the pairing would not necessarily survive a copy/paste/edit cycle. ]

Also note how making the value explicit makes it easier to check for
consistency:
...
  catch (const gdb_exception &except)
    {
      return gdbpy_handle_gdb_exception (-1, except);
    }

  if (PyErr_Occurred ())
    return -1;
...
given that we do use the explicit constants almost everywhere else.

Compared to using GDB_PY_HANDLE_EXCEPTION, there is the burden now to specify
the return value, but I assume that this will be generally copy-pasted and
therefore present no problem.

Also, there's no longer a guarantee that there's an immediate return, but I
assume that nodiscard making sure that the return value is not silently
ignored is sufficient mitigation.

For now, re-implement GDB_PY_HANDLE_EXCEPTION and GDB_PY_SET_HANDLE_EXCEPTION
in terms of gdbpy_handle_gdb_exception.

Follow-up patches will eliminate the macros.

No functional changes.

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 82680cdac0a84317831250ffef25b6e9e741f013..759e305d40d067865d5613b78d1cc7f7c2698533 100644 (file)
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -934,19 +934,17 @@ private:
 
 /* Use this in a 'catch' block to convert the exception to a Python
    exception and return nullptr.  */
-#define GDB_PY_HANDLE_EXCEPTION(Exception)     \
-  do {                                         \
-    gdbpy_convert_exception (Exception);       \
-    return nullptr;                            \
+#define GDB_PY_HANDLE_EXCEPTION(Exception)                             \
+  do {                                                                 \
+    return gdbpy_handle_gdb_exception (nullptr, Exception);            \
   } while (0)
 
 /* Use this in a 'catch' block to convert the exception to a Python
    exception and return -1.  */
-#define GDB_PY_SET_HANDLE_EXCEPTION(Exception)                         \
-    do {                                                               \
-      gdbpy_convert_exception (Exception);                             \
-      return -1;                                                       \
-    } while (0)
+#define GDB_PY_SET_HANDLE_EXCEPTION(Exception)                 \
+  do {                                                         \
+    return gdbpy_handle_gdb_exception (-1, Exception); \
+  } while (0)
 
 int gdbpy_print_python_errors_p (void);
 void gdbpy_print_stack (void);
@@ -1013,6 +1011,18 @@ extern PyObject *gdbpy_gdberror_exc;
 extern void gdbpy_convert_exception (const struct gdb_exception &)
     CPYCHECKER_SETS_EXCEPTION;
 
+ /* Use this in a 'catch' block to convert the exception E to a Python
+    exception and return value VAL to signal that an exception occurred.
+    Typically at the use site, that value will be returned immediately.  */
+
+template<typename T>
+[[nodiscard]] T
+gdbpy_handle_gdb_exception (T val, const gdb_exception &e)
+{
+  gdbpy_convert_exception (e);
+  return val;
+}
+
 int get_addr_from_python (PyObject *obj, CORE_ADDR *addr)
     CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;