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

[Richard Biener]

On Thu, Sep 19, 2013 at 7:24 PM, Trevor Saunders <tsaunders@mozilla.com> wrote:
> On Thu, Sep 19, 2013 at 03:23:21PM +0200, Michael Matz wrote:
>> > I don't see anything in Trevor's work that requires jumping through
>> > hoops.
>>
>> Me neither, from that perspective it's okay.  It's merely that I doubt the
>> value of any syntactic privatization like it's implemented in C++, you can
>> #define it away, hence the compiler can't make use of that information for
>
> no, it can't make use of it if someone does something crazy like #define
> it away which is atleast a little tricky because of the ':'.  I believe
> clang does infact make use of private to find unused fields (maybe it
> does something else, but I can't imagine what that would be).
>
>> code generation, and the cognitive value for the developer ("hey I
>> shouldn't look at this member from outside") is dubious, as that probably
>> is a general rule, no direct data member access from non-members (although
>> I have problems with that too).
>
> The value is that when you read code you *know* that something is only
> used in certain places instead of hoping that is true.
>
>> And I think the fact that Trevor made one data member non-private to
>> access it from a non-member function (move_computations_dom_walker::todo)
>> just underlines my point: private is useless and gets in the way.
>
> It certainly shows a case where that's true, but it doesn't really show
> that's always true.
>
>> > > 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.
>>
>> this->x.
>>
>> From the wiki it seems that was dicussed (on the wiki, not the mailing
>> list) and rejected by Lawrence on the grounds of indroducing too long
>> lines.  I agree with that, but I don't agree that therefore members should
>> be named foo_.
>
> this-> also has the disadvantage that you always have to rember it, and
> fundimentally doesn't help you know where a member could possibly be
> used.

Sure, there is no way to syntactically force the use of this-> - that's a
disadvantage (though we've had private -Wxxx warnings which we could
add for this).  The extra advantage of this-> is that it makes name-lookup
unambiguous to those not 100% familiar with it (and who really is ...)

Richard.

> Trev
>
>>
>> > Given it's recommended by our C++ guidelines which were discussed at
>> > length, I'm going to explicitly NAK your patch.
>>
>> Hmmkay.
>>
>> > FWIW, I have worked on large C++ codebases
>>
>> Me too.
>>
>> > that were a free-for-all and found them *amazingly* painful.
>>
>> I don't think any of my mails about style can be interpreted as advocating
>> free-for-all.
>>
>> > 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.
>>
>> Funnily it's the small things that weren't much discussed (probably
>> because they are deemed not very important) in the convention that give
>> me a hard time, nits such as these syntactic uglifications.  The larger
>> things indeed mostly are okayish.
>>
>>
>> Ciao,
>> Michael.