Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors

[Richard Biener <richard dot guenther at gmail dot com>]

Jeff Law <law@redhat.com> wrote:
>On 09/18/2013 11:17 AM, Michael Matz wrote:
>> Hi,
>>
>> 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.
>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.
>
>>
>>>> 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?
>>
>> http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00302.html
>>
>> 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!?)
>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.
>
>
>>
>>>> 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_);
>>        count_++;
>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.
>

There is a language specific way, too. Just qualify accesses with this-> that also avoids all the interesting name-lookup issues with dependent names.

Richard.

>>
>> 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.
>
>
>Jeff