On Thu, Sep 19, 2013 at 7:24 PM, Trevor Saunders <tsaund...@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.