This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]