Re: [PATCH 1/4] Change evpy_emit_event to accept a gdbpy_opt_borrowed_ref

[Simon Marchi <simark@simark.ca>]

On 8/8/26 5:47 PM, Tom Tromey wrote:
> This changes evpy_emit_event to accept a gdbpy_opt_borrowed_ref.  An
> "opt" type is used because one caller explicitly passes NULL, meaning
> that the event should be emitted without a payload object.
> 
> This patch is a small step toward using the safety API in event
> generation, though for the time being it mainly allows the removal of
> calls to "get".
> ---
>  gdb/python/py-breakpoint.c      | 5 ++---
>  gdb/python/py-connection.c      | 2 +-
>  gdb/python/py-continueevent.c   | 2 +-
>  gdb/python/py-corefile.c        | 2 +-
>  gdb/python/py-event.c           | 2 +-
>  gdb/python/py-event.h           | 7 ++++++-
>  gdb/python/py-exitedevent.c     | 2 +-
>  gdb/python/py-inferior.c        | 8 ++++----
>  gdb/python/py-infevents.c       | 6 +++---
>  gdb/python/py-newobjfileevent.c | 6 +++---
>  gdb/python/py-progspace.c       | 4 ++--
>  gdb/python/py-stopevent.c       | 2 +-
>  gdb/python/py-threadevent.c     | 2 +-
>  gdb/python/py-tui.c             | 2 +-
>  gdb/python/python.c             | 2 +-
>  15 files changed, 29 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index ecb42cee5f9..b628e14ae4e 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -1283,8 +1283,7 @@ gdbpy_breakpoint_created (struct breakpoint *bp)
>  
>    if (!evregpy_no_listeners_p (gdb_py_events.breakpoint_created))
>      {
> -      if (evpy_emit_event ((PyObject *) newbp,
> -			   gdb_py_events.breakpoint_created) < 0)
> +      if (evpy_emit_event (newbp, gdb_py_events.breakpoint_created) < 0)

Unrelated, but those two if could be collapsed into one with &&.

Also unrelated: I would change evregpy_no_listeners_p to
evregpy_has_listeners_p, because double-negative is confusing.

I can make following patches to change those.

>  	gdbpy_print_stack ();
>      }
>  }
> @@ -1312,7 +1311,7 @@ gdbpy_breakpoint_deleted (struct breakpoint *b)
>  
>  	  if (!evregpy_no_listeners_p (gdb_py_events.breakpoint_deleted))
>  	    {
> -	      if (evpy_emit_event ((PyObject *) bp_obj.get (),
> +	      if (evpy_emit_event (bp_obj,
>  				   gdb_py_events.breakpoint_deleted) < 0)
>  		gdbpy_print_stack ();
>  	    }
> diff --git a/gdb/python/py-connection.c b/gdb/python/py-connection.c
> index bc738669b79..99bed238b6c 100644
> --- a/gdb/python/py-connection.c
> +++ b/gdb/python/py-connection.c
> @@ -146,7 +146,7 @@ emit_connection_event (process_stratum_target *target,
>    if (evpy_add_attribute (event_obj.get (), "connection", conn.get ()) < 0)
>      return -1;
>  
> -  return evpy_emit_event (event_obj.get (), registry);
> +  return evpy_emit_event (event_obj, registry);
>  }
>  
>  /* Callback for the connection_removed observer.  */
> diff --git a/gdb/python/py-continueevent.c b/gdb/python/py-continueevent.c
> index 15404157062..06d4dabadd9 100644
> --- a/gdb/python/py-continueevent.c
> +++ b/gdb/python/py-continueevent.c
> @@ -51,6 +51,6 @@ emit_continue_event (ptid_t ptid)
>  
>    gdbpy_ref<> event = create_continue_event_object (ptid);
>    if (event != NULL)
> -    return evpy_emit_event (event.get (), gdb_py_events.cont);
> +    return evpy_emit_event (event, gdb_py_events.cont);
>    return -1;
>  }
> diff --git a/gdb/python/py-corefile.c b/gdb/python/py-corefile.c
> index fc5b4889fdc..542bc05f560 100644
> --- a/gdb/python/py-corefile.c
> +++ b/gdb/python/py-corefile.c
> @@ -346,7 +346,7 @@ emit_corefile_changed_event (inferior *inf)
>  			     inf_obj.get ()) < 0)
>      return -1;
>  
> -  return evpy_emit_event (event_obj.get (), gdb_py_events.corefile_changed);
> +  return evpy_emit_event (event_obj, gdb_py_events.corefile_changed);
>  }
>  
>  /* Callback from gdb::observers::core_file_changed.  The core file for
> diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
> index a7aa46dcb6f..6dd6f3bf356 100644
> --- a/gdb/python/py-event.c
> +++ b/gdb/python/py-event.c
> @@ -62,7 +62,7 @@ gdbpy_initialize_event ()
>     returns 0 if emit is successful -1 otherwise.  */
>  
>  int
> -evpy_emit_event (PyObject *event,
> +evpy_emit_event (gdbpy_opt_borrowed_ref<> event,
>  		 eventregistry_object *registry)
>  {
>    Py_ssize_t i;

Copying a review comment from Claude that seems important:

  - gdb/python/py-event.c:88: PyObject_CallFunctionObjArgs (func, event,
    NULL) — event is now a gdbpy_opt_borrowed_ref<> and sits in the ...
    of a true variadic C function. Variadic arguments get default
    argument promotions only; the user-defined operator T * is never
    applied, so the class object is passed, not a PyObject *. It
    compiles silently (reduced case, g++ -std=c++17 -Wall -Wextra: no
    diagnostic) and happens to work only because a trivially-copyable
    one-pointer struct is passed like a pointer on the usual ABIs.
    Introduce a local PyObject *ev = event; and pass that.

Otherwise, it also noted:

  - gdb/python/py-breakpoint.c:1309 and :1340: both calls now fit on one line.

Simon