Re: [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object

[Simon Marchi <simark@simark.ca>]

On 2026-02-20 20:58, Simon Marchi wrote:
> 
> 
> On 2026-02-20 16:03, Tom Tromey wrote:
>> This changes symtab_and_line_to_sal_object to return a gdbpy_ref<>,
>> using the type system to convey that a new reference is always
>> returned.
> 
> I noted some stylistic suggestions below.  It's a bit subjective so it's
> up to you really.  I suppose that the same patterns happen in all the
> patches.
> 
>> @@ -560,7 +560,7 @@ pspy_find_pc_line (PyObject *o, PyObject *args)
>>        return gdbpy_handle_gdb_exception (nullptr, except);
>>      }
>>  
>> -  return result;
>> +  return result.release ();
> 
> In these cases I think I would return directly from the
> symtab_and_line_to_sal_object call:
> 
>     return symtab_and_line_to_sal_object (sal).release ();
> 
>> diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
>> index 2dca0083277..820542932ba 100644
>> --- a/gdb/python/py-symtab.c
>> +++ b/gdb/python/py-symtab.c
>> @@ -390,7 +390,7 @@ symtab_to_symtab_object (struct symtab *symtab)
>>  
>>  /* Create a new symtab and line (gdb.Symtab_and_line) object
>>     that encapsulates the symtab_and_line structure from GDB.  */
>> -PyObject *
>> +gdbpy_ref<>
>>  symtab_and_line_to_sal_object (struct symtab_and_line sal)
>>  {
>>    sal_object *sal_obj;
>> @@ -399,7 +399,7 @@ symtab_and_line_to_sal_object (struct symtab_and_line sal)
>>    if (sal_obj != nullptr)
>>      set_sal (sal_obj, sal);
>>  
>> -  return (PyObject *) sal_obj;
>> +  return gdbpy_ref<> (sal_obj);
> 
> I think you could be even more pedantic and change sal_obj to be of type
> gdbpy_ref<>.  This way, the ref is managed from the very edge of the
> Python API.

Regarding this, I thought this would work, but it doesn't:

  gdbpy_ref<sal_object> sal_obj (PyObject_New (sal_object, &sal_object_type));
  if (sal_obj != nullptr)
    set_sal (sal_obj.get (), sal);

  return sal_obj;

I don't know if there is a magic way to make gdbpy_ref<sal_object>
implicitly convertible to gdbpy_ref<>.

I tried changing the return type of symtab_and_line_to_sal_object to be
gdbpy_ref<sal_object> instead.  I don't think it would be a bad thing,
it would convey what type of object it is exactly.  But this would require moving the
definition of struct sal_object in some header file, otherwise the other
files don't know that `sal_object *` is convertible to `PyObject *`,
when they try to pass the released value to the Python API.

Anyway, what you have here is fine, it is definitely an improvement over
the current state.

Simon