From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42250 invoked by alias); 6 Jun 2018 14:34:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 42228 invoked by uid 89); 6 Jun 2018 14:34:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=no version=3.3.2 spammy=JIT X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Jun 2018 14:34:48 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 169CB401EF1A for ; Wed, 6 Jun 2018 14:34:47 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9286A1C725; Wed, 6 Jun 2018 14:34:46 +0000 (UTC) Subject: Re: [PATCH 4/8] Add a C++ wrapper for GCC C plug-in To: Keith Seitz , gdb-patches@sourceware.org References: <20180503184153.31183-1-keiths@redhat.com> <20180503184153.31183-5-keiths@redhat.com> From: Pedro Alves Message-ID: <20f5dd67-b19f-f2f9-919e-1c30cf9a8a5d@redhat.com> Date: Wed, 06 Jun 2018 14:34:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180503184153.31183-5-keiths@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-06/txt/msg00139.txt.bz2 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