This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors
- From: Jeff Law <law at redhat dot com>
- To: Michael Matz <matz at suse dot de>
- Cc: Trevor Saunders <tsaunders at mozilla dot com>, Richard Biener <richard dot guenther at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 18 Sep 2013 22:22:45 -0600
- Subject: Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors
- Authentication-results: sourceware.org; auth=none
- References: <1378264562-30803-1-git-send-email-tsaunders at mozilla dot com> <CAFiYyc2WMqt7b=riv_3+LUDQ=+OjycgEqK+L8afjPcaVzU9Wag at mail dot gmail dot com> <20130904145911 dot GC17620 at tsaunders-iceball dot corp dot tor1 dot mozilla dot com> <522759C8 dot 5040802 at redhat dot com> <20130911000350 dot GA28492 at tsaunders-iceball dot corp dot tor1 dot mozilla dot com> <52389CB1 dot 60504 at redhat dot com> <5239126A dot 6010702 at redhat dot com> <alpine dot LNX dot 2 dot 00 dot 1309181640400 dot 9949 at wotan dot suse dot de> <5239D985 dot 4080205 at redhat dot com> <alpine dot LNX dot 2 dot 00 dot 1309181849550 dot 9949 at wotan dot suse dot de>
On 09/18/2013 11:17 AM, Michael Matz wrote:
Well, as Ian pointed out, it is in our recommended style guidelines and
you'll find uses in the vec class as well. It's well established at
this point and I see no compelling reason to go back unless you can
convince the project as a whole to change the C++ guidelines.
On Wed, 18 Sep 2013, Jeff Law wrote:
On 09/18/2013 10:24 AM, Michael Matz wrote:
I'm irritated by the member name uglification (e.g. equiv_stack_ with
trailing underscore). I know that's a certain style to mark private
members, but I think it's a bad style (like prefixing variable names with
their type), and before it sets a precedent in GCCs c++ coding style I'd
like this to be changed, like in the below.
We're already using the trailing underscore idiom for private objects
moving into classes (see the pass class).
I know, and I don't like it there either.
I don't see anything in Trevor's work that requires jumping through
hoops. It's pretty simple stuff. And again, as Ian pointed out, our
established guidelines for C++ usage encourage this behaviour.
I'd also like us to not use member privatization in our classes, but
that's not in the patch, but if we could agree on that it would be nice.
Member privatization is quite natural. What specifically do you not like
about the practice?
That was conditional on "when we need to jump through hoops", but for
constistency it'd make sense to avoid it everywhere.
(I know that Ian agreed to that mail, but somehow the mailing list
archives don't have that!?)
It makes it very clear to the reader that we're dealing with objects
that belong to a class instance rather than direct access to an auto or
static. That can be important.
Regstrapped on x86-64-linux, okay?
Obviously any ChangeLog, formatting and such can go in. However, the
trailing underscore should stay given it's already established practice
and has several nice benefits.
What's the benefit of reading and writing such noisy lines? :
*out_mode = mode_;
mode_ = GET_MODE_WIDER_MODE (mode_);
The uglification merely makes code harder to write and read, it should be
used in cases where you _don't_ want developers to write such names.
I feel it makes it harder in some ways and easier in others.
Given it's recommended by our C++ guidelines which were discussed at
length, I'm going to explicitly NAK your patch. If you want to re-open
the guidelines for C++ usage, then that's fine with me and if we as a
project change the guidelines to disallow such things, then that will be
the time to remove the trailing underscores, private members, etc.
FWIW, I have worked on large C++ codebases that were a free-for-all and
found them *amazingly* painful. The restricted set allowed for GCC is
actually quite reasonable IMHO, particularly for projects where the main
body of code is evolving from a pure C base.