diff mbox series

[v2,5/8] gdb: add gdbarch_up

Message ID 20230105200237.987771-6-simon.marchi@polymtl.ca
State Committed
Commit d246d904adf3e338c731c123219a8246281002e2
Headers
Series Initial support for ROCm platform (AMD GPU) debugging |

Commit Message

Simon Marchi Jan. 5, 2023, 8:02 p.m. UTC 
  From: Simon Marchi <simon.marchi@efficios.com>

Add a gdbarch_up unique pointer type, that calls gdbarch_free on
deletion.  This is used in the ROCm support patch at the end of this
series.

Change-Id: I4b808892d35d69a590ce83180f41afd91705b2c8
Approved-By: Andrew Burgess <aburgess@redhat.com>
---
 gdb/gdbarch.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Tom Tromey Jan. 5, 2023, 8:31 p.m. UTC | #1 
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> From: Simon Marchi <simon.marchi@efficios.com>
Simon> Add a gdbarch_up unique pointer type, that calls gdbarch_free on
Simon> deletion.  This is used in the ROCm support patch at the end of this
Simon> series.

gdbarch_free is just a wrapper around delete.
What if this typedef just used the default deleter instead?
Moving gdbarch to just use new/delete always looks pretty easy.

Tom
Simon Marchi Jan. 5, 2023, 8:36 p.m. UTC | #2 
On 1/5/23 15:31, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@efficios.com>
> Simon> Add a gdbarch_up unique pointer type, that calls gdbarch_free on
> Simon> deletion.  This is used in the ROCm support patch at the end of this
> Simon> series.
> 
> gdbarch_free is just a wrapper around delete.
> What if this typedef just used the default deleter instead?
> Moving gdbarch to just use new/delete always looks pretty easy.
> 
> Tom

I think it would involve moving the struct gdbarch definition from gdbarch.c
to gdbarch-gen.h, right?

Simon
Tom Tromey Jan. 5, 2023, 8:41 p.m. UTC | #3 
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> On 1/5/23 15:31, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>> 
Simon> From: Simon Marchi <simon.marchi@efficios.com>
Simon> Add a gdbarch_up unique pointer type, that calls gdbarch_free on
Simon> deletion.  This is used in the ROCm support patch at the end of this
Simon> series.
>> 
>> gdbarch_free is just a wrapper around delete.
>> What if this typedef just used the default deleter instead?
>> Moving gdbarch to just use new/delete always looks pretty easy.
>> 
>> Tom

Simon> I think it would involve moving the struct gdbarch definition from gdbarch.c
Simon> to gdbarch-gen.h, right?

Ugh, yeah, never mind.  It would be nice, maybe, to convert all this so
the accessors are inlined and whatnot, but it's a much bigger job.

This looks ok to me.

Tom
Simon Marchi Jan. 6, 2023, 12:48 a.m. UTC | #4 
On 1/5/23 15:41, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> On 1/5/23 15:31, Tom Tromey wrote:
>>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>
> Simon> From: Simon Marchi <simon.marchi@efficios.com>
> Simon> Add a gdbarch_up unique pointer type, that calls gdbarch_free on
> Simon> deletion.  This is used in the ROCm support patch at the end of this
> Simon> series.
>>>
>>> gdbarch_free is just a wrapper around delete.
>>> What if this typedef just used the default deleter instead?
>>> Moving gdbarch to just use new/delete always looks pretty easy.
>>>
>>> Tom
> 
> Simon> I think it would involve moving the struct gdbarch definition from gdbarch.c
> Simon> to gdbarch-gen.h, right?
> 
> Ugh, yeah, never mind.  It would be nice, maybe, to convert all this so
> the accessors are inlined and whatnot, but it's a much bigger job.

I think it would be nice to C++-ify a bit (while keeping the same
principle used today.  Make all the function pointer fields private, and
change all the gdbarch_* free functions to be methods instead.  So a
call like this:

  gdbarch_do_something (arch, 1, "hello");

would become

  arch->do_something (1, "hello");

... which would still go through the function pointer underneath.

Simon
diff mbox series

Patch

diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index f4efd8c0bc78..f0399c2fa888 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -306,6 +306,14 @@  extern struct gdbarch *gdbarch_alloc (const struct gdbarch_info *info,
 
 extern void gdbarch_free (struct gdbarch *);
 
+struct gdbarch_deleter
+{
+  void operator() (gdbarch *arch) const
+  { gdbarch_free (arch); }
+};
+
+using gdbarch_up = std::unique_ptr<gdbarch, gdbarch_deleter>;
+
 /* Get the obstack owned by ARCH.  */
 
 extern obstack *gdbarch_obstack (gdbarch *arch);