Hi, On Thu, Jun 25, 2015 at 07:28:45PM +0100, Richard Sandiford wrote: > Sorry in advance for inviting a bikeshed discussion >, but while making > the hashing changes that I just committed, I noticed that the C++ification > has been done in a variety of different styles. I ended up having to follow > the "do what the surrounding code does" principle that some code bases have, > but to me that's always seemed like an admission of failure. One of the > strengths of the GCC code base was always that it was written in a very > consistent style. Regardless of what you think of that style (I personally > like it, but I know others don't at all), it was always easy to work on > a new area of the compiler without having to learn how the surrounding code > preferred to format things. It would be a shame if we lost that in the > rush to make everything "more C++".
Agreed. > > The three main inconsistencies I saw were: > > (1) Should inline member functions be implemented inside the class or outside > the class? If inside, should they be formatted like this: https://gcc.gnu.org/codingconventions.html has a big section on C++ (note that there is also a similar document on wiki which IIRC often contradicts this one). Unfortunately, in this case, it gives two sightly contradicting guidelines. On one hand, section https://gcc.gnu.org/codingconventions.html#Cxx_Inlining says "Constructors and destructors, even those with empty bodies, are often much larger than programmers expect. Prefer non-inline versions unless you have evidence that the inline version is smaller or has a significant performance impact. " on the other, https://gcc.gnu.org/codingconventions.html#Member_Form explicitely states: "Define all members outside the class definition. That is, there are no function bodies or member initializers inside the class definition. " Since the former seems to be specific to constructors and destructors, it probably overrides the latter. So in my book, you can have inline constructors and destructors if they are extremely small, such as empty, but nothing else should be inline. > > void > foo (args...) > { > ...; > } > > or like this: > > void > foo (args...) > { > ...; > } > > (both have been used). > > The coding standard is pretty clear about this one: > > Define all members outside the class definition. That is, there > are no function bodies or member initializers inside the class > definition. > > But in-class definitions have become very common. Do we want > to revisit this? Or do we just need more awareness of what the > rule is supposed to be? > > [Personally I like the rule. The danger with in-class definitions > is that it becomes very hard to see the interface at a glance. > It obviously makes things more verbose though.] > > (2) Is there supposed to be a space before a template parameter list? > I.e. is it: > Examples in that document have them but I could not quickly find any rules enforcing this. I personally do not like them, but will comply to whichever case prevails. > foo<bar> > > or: > > foo <bar> > > ? Both are widely used. > > The current coding conventions don't say explicitly, but all the > examples use the second style. It's also more in keeping with > convention for function parameters. On the other hand, it could > be argued that the space in: > > foo <bar, frob>::thing > > makes the binding confusing and looks silly compared to: > > foo<bar, frob>::thing > > But there again, the second one might look like two unrelated > blobs at first glance. > > (3) Do we allow non-const references to be passed and returned by > non-operator functions? Some review comments have pushed back > on that, but some uses have crept in. I think it was just me pushing back and I am still a bit unhappy about them, but other people voicing opinions though they were perfectly fine. I believe the main reason for using them is that if you pass a pointer to a vector to a function, the function then has to access the elements with (*vec_arg)[i] which is super-ugly but there you go. But perhaps this is fixable with some template operator overload? (Of course, if someone then had an array of vectors, accessing the individual elements would be pain). Anyway, with a reference you are fine. > > [IMO non-const references are too easy to misread as normal > parameters.] Yes, IMHO too. Thanks, Martin
