Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: Matthieu Longo <matthieu.longo@arm.com>
Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
Subject: Re: [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T
Date: Fri, 06 Mar 2026 09:47:10 -0700	[thread overview]
Message-ID: <871phwzz01.fsf@tromey.com> (raw)
In-Reply-To: <f040685d-2c3b-4e66-a0a3-fea1accdf0d1@arm.com> (Matthieu Longo's message of "Fri, 6 Mar 2026 11:37:28 +0000")

>>>>> Matthieu Longo <matthieu.longo@arm.com> writes:

> After a deeper examination, I am not sure that your series helps much for this patch.

> * Case 1: return value from the registry.

> PyObject *result = (PyObject *) cfpy_inferior_corefile_data_key.get (inf);
>   if (result != nullptr)
>     return gdbpy_ref<>::new_reference (result);

> If I had gdbpy_borrowed_ref available, I could replace the code above by something like:
> gdbpy_borrowed_ref result = cfpy_inferior_corefile_data_key.get (inf);
> if (result != nullptr)
>   return result; /* assuming that gdbpy_ref<> would have a constructor taking gdbpy_borrowed_ref. */

Ok, I see.  Sorry about that.

Does the registry need to return a properly-typed one?
Perhaps it can just always return a gdbpy_ref<>.

> * Case 2: registration of the new object in the registry and return the value.

> gdbpy_ref<corefile_object> object
>     (PyObject_New (corefile_object, &corefile_object_type));
> ...
> cfpy_inferior_corefile_data_key.set (inf, object.get ());
> return gdbpy_ref<>::new_reference (object.release ());

Here the release stuff is no longer needed as upcasting works fine:

cfpy_inferior_corefile_data_key.set (...);
return object;

> However, I am getting further from my original objective, which was only flattening the code for upcoming changes.
> All those improvements seem to me out of scope of my original patch.

Yeah.  It's fine to take small steps and not do too much.
So if it's convenient just go ahead; but if you don't mind add a bit of
explanation to the commit message.

thanks,
Tom

  parent reply	other threads:[~2026-03-06 16:47 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-03 16:16 [PATCH v2 0/9] gdb: more fixes for Python limited C API support Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents Matthieu Longo
2026-03-03 18:09   ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code Matthieu Longo
2026-03-03 18:16   ` Tom Tromey
2026-03-04 16:30     ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 3/9] gdb: switch bytes object helpers to Python limited API equivalents Matthieu Longo
2026-03-03 18:03   ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 4/9] gdb: add new helpers for retrieving a type's fully qualified name Matthieu Longo
2026-03-03 18:59   ` Tom Tromey
2026-03-06 17:49     ` Matthieu Longo
2026-03-06 19:45       ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 5/9] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T Matthieu Longo
2026-03-03 18:18   ` Tom Tromey
2026-03-04 16:56     ` Matthieu Longo
2026-03-04 18:55       ` Tom Tromey
2026-03-06 11:37     ` Matthieu Longo
2026-03-06 11:43       ` Matthieu Longo
2026-03-06 16:47       ` Tom Tromey [this message]
2026-03-09 11:38         ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 6/9] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
2026-03-03 18:22   ` Tom Tromey
2026-03-09 11:41     ` Matthieu Longo
2026-03-03 18:22   ` Tom Tromey
2026-03-03 16:16 ` [PATCH v2 7/9] gdb/python: accept gdbpy_ref in init helpers and return bool Matthieu Longo
2026-03-03 18:24   ` Tom Tromey
2026-03-09 13:25     ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 8/9] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper Matthieu Longo
2026-03-03 18:30   ` Tom Tromey
2026-03-06 12:03     ` Matthieu Longo
2026-03-03 16:16 ` [PATCH v2 9/9] gdb/python: add accessor helpers for __dict__ in Python extension objects Matthieu Longo
2026-03-03 19:02   ` Tom Tromey
2026-03-06 14:33     ` Matthieu Longo
2026-03-06 16:04       ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871phwzz01.fsf@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    --cc=matthieu.longo@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox