From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19495 invoked by alias); 4 Sep 2013 13:22:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 19486 invoked by uid 89); 4 Sep 2013 13:22:22 -0000 Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Sep 2013 13:22:22 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RDNS_NONE autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from relay1.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 510E5A531D; Wed, 4 Sep 2013 15:22:19 +0200 (CEST) Date: Wed, 04 Sep 2013 13:22:00 -0000 From: Michael Matz To: David Malcolm Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 0/6] Convert gimple to a C++ class hierarchy In-Reply-To: <1378257127.18117.41.camel@surprise> Message-ID: References: <1377793216-22549-1-git-send-email-dmalcolm@redhat.com> <1377890482.29222.32.camel@surprise> <1378257127.18117.41.camel@surprise> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-IsSubscribed: yes X-SW-Source: 2013-09/txt/msg00205.txt.bz2 Hi, On Tue, 3 Sep 2013, David Malcolm wrote: > > I can't really say I find this shorter, easier to read, more > > expressive or even safer than what was there before. And the > > repetition for adding the helpers for const and non-const types > > all the time doesn't make it better. > Part of this is the verbose struct names. I mentioned getting rid of > the "_statement" part of the typenames, I think I'll do that. Yep, IMO makes sense. > The other part is that the accessor functions become redundant, and that > you'd be able to do the cast once, and then use all of the various > fields of a gimple_whatever, bypassing the getters/setters. Well, you can do that today with unions too, it's just not prevalent style; but you could do: if (gimple_has_mem_ops (g)) { struct gimple_statement_with_memory_ops_base *gm = &g->gsmembase; gm->vuse = ...; } Obviously the naming of the struct here also is a bit excessive. Using accessors has one large advantage over accessing the fields directly, you can change the semantics of them. One reason why the above style isn't used. But if that is true one of your reasons doing the change (downcast once, access fields directly, obsoleting the accessors) becomes moot, because we don't _want_ to access the fields directly. That is, until you add member functions doing the accesses, which has its own problems (of stylistic nature, because then we'd have a very weird and clumsy mix in GCC sources of some data structures having member function accessors and others using the traditional C style). Hmm. After some nights sleeping over this, I'm oscillating again between not liking the change and being indifferent; as in, I do see some of the advantages you mentioned but I don't regard them outweighing the disadvantages. Ciao, Michael.