This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 4/8] Add a C++ wrapper for GCC C plug-in
- From: Pedro Alves <palves at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>, gdb-patches at sourceware dot org
- Date: Wed, 6 Jun 2018 15:34:45 +0100
- Subject: Re: [PATCH 4/8] Add a C++ wrapper for GCC C plug-in
- References: <20180503184153.31183-1-keiths@redhat.com> <20180503184153.31183-5-keiths@redhat.com>
On 05/03/2018 07:41 PM, Keith Seitz wrote:
> This patch introduces a new class which wraps the GCC C compile plug-in.
> It is a little unfortunate that this all happened in between the time that
> GCC moved to C++ and GDB moved to C++, leaving us with an ABI promise to
> support a C-like interface.
It's easier to keep an ABI promise in C anyway, so it's not uncommon
for C++ projects to expose a C plugin interface and then provide wrappers
for multiple languages. That's how the GCC JIT interface is exported
for example.
> The hope is to isolate GDB from some of this
> should it change in the future.
Not so sure it would be a good idea to change it, IMHO.
>
> Broadly, what this does is replace calls like:
>
> C_CTX (context)->c_ops->operation (C_CTX (context), ...);
>
> with calls that now look like:
>
> context->c_plugin->operation (...);
This LGTM.
I do wonder whether c_plugin needs to be a pointer though?
Couldn't it be an object directly? Like:
context->c_plugin.operation (...);
I see that the next patch switches this to a method named
plugin(), and then I wonder if that method can return a reference
instead of a pointer (because IIUC, you can never have a
NULL plugin).
TBC, even if you agree, I don't think it's worth it to go
over the pain of to redoing the series because of this.
I'd be totally fine with changing it on top of the series.
> @@ -206,22 +197,20 @@ convert_enum (struct compile_c_instance *context, struct type *type)
> {
> gcc_type int_type, result;
> int i;
> - struct gcc_c_context *ctx = C_CTX (context);
> + const gcc_c_plugin *plugin = context->c_plugin;
>
> - int_type = ctx->c_ops->int_type_v0 (ctx,
> - TYPE_UNSIGNED (type),
> - TYPE_LENGTH (type));
> + int_type = plugin->int_type_v0 (TYPE_UNSIGNED (type),
> + TYPE_LENGTH (type));
Alignment looks odd here, but maybe it's just in the email client.
Thanks,
Pedro Alves