From patchwork Thu Nov 20 13:12:54 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 412703 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5638B1400E2 for ; Fri, 21 Nov 2014 00:13:05 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=MZBpU0Q8/pSZyp8yci iSOF2bBJrZ3migrXHsxf8YPNtixIEv32DBQbivcgG9K9kHi64MwlGqXViIFzCuZc 7145E2dqS4zcBgc2OxiWJ3UU1DCYCeyoTfM3hNEnoATm1u4Tq4iMMjCHw7rWTwr9 4yz2r+/k70weEmvIc7Pkv3FCQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=LWz6pPOIVrW0dIJ1dpcIgyzg UEQ=; b=fPymzsjEBxEosnf+3NN/fqUnTttUkFfxpG4s5kFEzE2WqE1eXSljwQXi zSul656fE4cdpTjFGGqX+VuRCWqnJ/xsaPlFivXgM8yVbEM9bvDsocqltdDNTUXf m9p4n13FT+eZEp/V73ON4CeLOPj9ZTkNHSJ9gJ9xW3OL0FHuvoI= Received: (qmail 315 invoked by alias); 20 Nov 2014 13:12:58 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 306 invoked by uid 89); 20 Nov 2014 13:12:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f50.google.com Received: from mail-oi0-f50.google.com (HELO mail-oi0-f50.google.com) (209.85.218.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 20 Nov 2014 13:12:56 +0000 Received: by mail-oi0-f50.google.com with SMTP id a141so1973461oig.37 for ; Thu, 20 Nov 2014 05:12:54 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.182.51.136 with SMTP id k8mr18546obo.69.1416489174227; Thu, 20 Nov 2014 05:12:54 -0800 (PST) Received: by 10.76.101.132 with HTTP; Thu, 20 Nov 2014 05:12:54 -0800 (PST) In-Reply-To: <1416435844.3562.78.camel@surprise> References: <1416420732.3562.49.camel@surprise> <546CEEDF.10007@redhat.com> <8C1B6C93-EAA6-4F10-84BB-743499104758@gmail.com> <546D0724.3080006@redhat.com> <1416435844.3562.78.camel@surprise> Date: Thu, 20 Nov 2014 14:12:54 +0100 Message-ID: Subject: Re: "gimple-classes-v2-option-3" git branch committed to svn trunk as r217787 From: Richard Biener To: David Malcolm Cc: Andrew MacLeod , GCC Patches X-IsSubscribed: yes On Wed, Nov 19, 2014 at 11:24 PM, David Malcolm wrote: > On Wed, 2014-11-19 at 22:36 +0100, Richard Biener wrote: >> On November 19, 2014 10:09:56 PM CET, Andrew MacLeod wrote: >> >On 11/19/2014 03:43 PM, Richard Biener wrote: >> >> On November 19, 2014 8:26:23 PM CET, Andrew MacLeod >> > wrote: >> >>> On 11/19/2014 01:12 PM, David Malcolm wrote: >> >>> >> >>>> (A) could become: >> >>>> >> >>>> greturn *stmt = gsi->as_a_greturn (); >> >>>> >> >>>> (B) could become: >> >>>> >> >>>> stmt = gsi->dyn_cast (); >> >>>> if (!stmt) >> >>>> or: >> >>>> >> >>>> stmt = gsi->dyn_cast_gcall (); >> >>>> if (!stmt) >> >>>> >> >>>> or maybe: >> >>>> >> >>>> stmt = gsi->is_a_gcall (); >> >>>> if (!stmt) >> >>>> >> >>>> An earlier version of my patches had casting methods within the >> >>>> gimple_statement_base class, which were rejected; the above >> >proposals >> >>>> would instead put them within gimple_stmt_iterator. >> >>>> >> >>> I would like all gsi routines to be consistent, not a mix of >> >functions >> >>> and methods. >> >>> so something like >> >>> >> >>> stmt = gsi_as_call (gsi); >> >>> stmt = gsi_dyn_call (gsi); >> >>> >> >>> or we need to change gsi_stmt() and friends into methods and access >> >>> them >> >>> as gsi->stmt ().. which is possibly better, but that much more >> >>> intrusive again (another 2000+ locations). >> >>> If we switched to methods everywhere for gsi, I'd prefer something >> >like >> >>> gsi->as_a_call () >> >>> gsi->is_a_call () >> >>> gsi->dyn_cast_call () >> >>> >> >>> I think its more readable... and it removes a dependency on the >> >>> implementation.. so if we ever decide to change the name of 'gcall' >> >>> down >> >>> the road to using a namespace, and make it gimple::call or whatever, >> >we >> >>> >> >>> wont have to change every single gsi-> location which has a >> >templated >> >>> use of the type. >> >>> >> >>> I'm also think this sort of thing could probably wait until next >> >stage >> >>> 1.. >> >>> >> >>> my 2 cents... >> >> Why not as_a (*gsi)? It would >> >> Add operator* to gsi of course. >> >> >> >> Richard. >> >> >> >> >> > >> >I could live with that form too. >> > >> >we often have an instance of gimple_stmt_iterator rather than a pointer >> > >> >to it, so wouldn't "operator gimple *()" to implicitly call gsi_stmt() >> > >> >when needed work better? (or "operator gimple ()" before the next >> >change) .. >> >> Not sure. The * matches how iterators work in STL... >> >> Note that for the cases where we pass a pointer to an iterator we can change those to use references to avoid writing **gsi. >> >> Richard. >> >> >Andrew > > I had a go at adding an operator * to gimple_stmt_iterator, using it > everywhere that we do an as_a<> or dyn_cast<> on the result of a > gsi_stmt, to abbreviate the gsi_stmt call down to one character. > > Patch attached; only lightly smoketested; am posting it for the sake of > discussion. Looks good. Note that gcall *call = as_a (*iter); probably not possible in 100% of all cases (where we sometimes pass NULL as the iterator pointer) but in most. > I don't think this API will make the non-C++-fans happier; I think the > objection to the work I just merged is that it's adding more C++ than > those people are comfortable with. How so? It's already super-ugly in those views. We decided to get C++. Now we have it. Now please make it AT LEAST CONSISTENT. > So although the attached patch makes things shorter (good), it's taking > things in a "more C++" direction (questionable). I'd hoped to paper > over the C++ somewhat. > > I suspect that any API which requires the of < > characters within the > implementation of a gimple pass to mean a template is going to give > those less happy with C++ a visceral "ugh" reaction. I wonder if > there's a way to spell these things that's concise and which doesn't > involve <> ? Only if you drop as_a/is_a/dyn_cast everywhere. Richard. diff --git a/gcc/asan.c b/gcc/asan.c index be28ede..d06d60c 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1902,7 +1902,7 @@ instrument_builtin_call (gimple_stmt_iterator *iter) return false; bool iter_advanced_p = false; - gcall *call = as_a (gsi_stmt (*iter)); + gcall *call = as_a (**iter); should be "fixed" by making instrument_builtin_call take a reference to the iterator so the above becomes