> > My suggestions: > > > > * When it is appropriate to use a child class with virtual > functions, > > the virtual functions should all be declared as protected in the > > parent class. > > > > At first reading, I thought you meant "all virtual functions should > be > > protected", but I think you meant "if a child ADDS a virtual function > > that the parent doesn't have, ...", but in that case I don't see why > the > > parent needs a declaration at all. This should be clarified. > > I did mean that all virtual functions should be protected. I tried to > clarify the wiki page.
Is "do not introduce new virtual functions in a child class" another way to phrase this? > > * Namespaces > > > > In general, I don't like namespaces, unless you're defining a > package. > > Class names create their own name spaces, globals should be global, > etc. > > Saying "everything should be in a namespace" is IMHO an inappropriate > > restriction. Indicate where namespaces are appropriate, leave it at > > that. > > The proposal doesn't say that everything should be in a namespace; it > says that namespaces are permitted. I continue to think that is > appropriate. I agree that globals should be global. However, GCC is > a big package which can be naturally divided into modules. For > example, GIMPLE code should not normally call RTL functions. Putting > the RTL functions in a namespace would make it clear when something is > reaching across a module boundary. Of course there are other ways to > do this as well, but my point is that "global is global" is not > necessary a well-defined concept in a big program. Some names are > global to parts of the program but not to others. I agree. In my experience, people can often lean too heavily on namespaces to help them avoid silent collisions of implementation that would otherwise be avoided by more carefully monitoring and maintaining header file dependencies. > > * When a method refers to a non-static data member, it should always > > qualify the reference with this->. > > > > I'm very opposed to this. To me, it makes the code less readable > > because it lets the author write code that's hard to understand at a > > larger scope. I would forbid explicit references to "this" except to > > pass it unadorned as a parameter to some other function. If it's not > > clear where method call data comes from, write clearer code! > > > > If you're implementing a class, it should be clear you're > implementing a > > class, and implied that data/methods come from the class unless > > specified otherwise (which you'd have to do anyway, like foo- > >reg_p()). > > If it's not clear, you've not properly encapsulated the class and you > > need to redesign it so it *is* properly contained and easy to > > understand. > > Our current code always uses the equivalent of an explicit this > pointer, except that it is called something else. Therefore I would > argue that this is not new. I don't see why using an explicit this-> > makes code harder to understand in a larger scope; the explicit this-> > makes it very clear where data is coming from. > > The biggest need for this-> is when calling methods in the current > class if the current class happens to be in a template. The C++ > template namespace lookup rules are confusing. The explicit this-> > makes them clear. While it's true that most code is not in a > template, it's possible that a template will be added later. For methods, I understand and agree. Does this still hold up for non-static data members which by previous guidelines are highly recommended to be private, though? If not, would the recommendation for always qualifying the reference just to be for consistency? I'm leaning toward DJ's opinion on this one, mainly because it is somewhat duplicative. Also, requiring data members to end in an underscore expresses the scope already. but I can see where you're coming from as well. > > * use of existing templates, e.g., from the standard library, is > fine > > > > I'd prefer avoiding pulling in STL stuff at first. I find STL to be > > quite a "culture shock" relative to C. When used I'd limit it to one > > STL object at a time - avoid containers of lists of vectors of > strings, > > etc. > > As noted earlier I think we do want to use some STL classes. I agree with Mark's earlier declaration that it is relatively straight-forward, low-hanging fruit to replace VEC_* with std::vector. (I had started trying this replacement myself last year, but the CXX build on trunk without any changes never did bootstrap and pass the testsuite correctly for me on Ubuntu 9.10, so I couldn't easily test my changes.) > > * Local variables should be declared where they're first used, if > their > > use is contiguous and localized. If they're ubiquitous (like a > return > > value temporary) or reused later (like "i" for for-loop iterators), > > declare them at the beginning of the function. > > Thanks, I added text along these lines. Agreed. > > * Use C-style comments for multi-line comments, and C++-style > comments > > for single-line comments. > > I'm not sure i agree with this, because I don't see anything wrong > with multi-line C++-style comments. I'm with Ian on this one. Is there a reason for this, other than one's personal tool preference for editing code may make C-style multi-line comments easier to add/remove? I've seen arguments both ways on that front, some people strongly preferring C++-style comments as it was easier for them to read code since they also strongly preferred absolutely no syntax highlighting (or color of any kind) in their terminals/editors. In other (more sane) cases, an editor of choice has a bug in one of its extensions that needed fixing, and we fixed said tool appropriately. Since it is so subjective, personal editing tool preference shouldn't dictate (much) of a coding style guide, IMO. One thing I disagree with in the wiki is the complete disallowance of multiple inheritance. When I contracted at Google, the team I worked with worked hard to make allowances for multiple inheritance of pure virtual classes (aka pure interfaces) to break dependencies by increasing encapsulation, and improve unit-testability (and re-use) by loosening coupling : http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Multiple_Inheritance This was a necessity to have the composability required to comply with the Interface Segregation Principle, while avoiding duplication like parallel inheritance hierarchies. The danger, of course, would be if the virtual dtors in multiply-inherited pure virtual classes needed to be called, then RTTI would be required. There is already a guideline for disallowing dtors, which helps avoid that issue altogether. Even so, a new warning might be helpful to avoid the issue altogether. A variation of Item 14 in -Weffc++ as a new warning could be used. Even without an new warning, I think the GCC C++ style guide should match Google's when it comes to inheritance. (Except for maybe suggesting the "Interface" suffix, which we often found impaired the fluency of the code.)
