Mikhail Maltsev <malts...@gmail.com> writes: > On 06/25/2015 09:28 PM, 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. > Perhaps one disappointing exception is mixed space/tabs indentation. It > is often inconsistent (i.e. some parts use space-only indentation).
Yeah. Been hitting that recently too. > That's rather annoying because changes in such line trigger warnings in > the checker script. I hope that I'll find some free time to address this > problem (I mean, I'll write a script which will fix indentation in the > GCC source tree, but will only change the lines affected by a given > patch. So that we will have not just check_GNU_style, but also a kind of > fix_GNU_style :) ). The problem is that if you have a paragraph of documentation or a fairly long block of code, all consistently indented with spaces, diffs start to look strange if one or two newer lines are correctly formatted. Saying "do what the surrounding code does" -- i.e. don't change the indentation of just one line if the line above or below will still use spaces -- is still admitting failure though... > Of course the "ideal" solution would be to rewrite the whole repository > (will full history), but that would break many things like MD5 sums of > released versions and commit hashes in GIT mirror :(. There was at least one mass removal of trailing whitespace, which was a bit controversial at the time, but got rid of another similar problem. >> (2) Is there supposed to be a space before a template parameter list? >> I.e. is it: >> >> foo<bar> >> >> or: >> >> foo <bar> >> >> ? Both are widely used. > There is a wiki page about coding conventions > https://gcc.gnu.org/wiki/CppConventions and it mentions that the latter > form should be used, but for some reason that rule did not get into > official guide. OK. >> If this message does generate any discussion, I'm happy to write up >> the result in the coding conventions and try to make the code base >> consistent with it. > My personal wishlist w.r.t. clarifications in coding conventions. I > would be glad if someone explained this at least briefly, I could then > document it and submit a patch for web site: > > 1. GCC has some common naming conventions. For example there are macros > and functions ending with "_P" (or "_p"), they check for some property > of their arguments (and my guess is that "p" stands for "predicate", but > I still don't know for sure). Yeah. This is a LISP-ism. There are lots of is_* functions these days too. I was never sure how much foo_p was preferred over is_foo for new code, although I do tend to stick to foo_p. > There is also an "_r" suffix, which is > used in callbacks for various traversals (reentrant? recursive?), Not sure. > foo/foo_1 pairs (wrappermain logic?). Yeah. > It would be nice to know the correct and intended use of such > suffixes/prefixes/etc. > > 2. Is this C legacy or a convention: > typedef struct some_name > { > ... > } some_name_t; It's legacy. It's also bad practice to use "_t" in POSIX-compatible code. Maybe we should get rid of the existing cases? There are also uses of: typedef struct some_name_def { ... } *some_name; to abstract away the pointer. Never been a big fan of that, but it would be pointless to rewrite it all :-) > 3. Provided that we have a definition of "struct other_name", which is > correct: "void foo (struct other_name *)" or "void foo (other_name *)"? Think we should strongly prefer the second now. "struct foo" wasn't so much of a coding convention as being forced by C. clang warns by default about "struct foo" being used if foo was declared or defined as "class foo", due to a MSC++ bug that caused it to reject that combination. We don't care about that version of MSC++, but people do report excessive warnings with clang. Maybe it would be too invasive for merging and backporting to remove all the redundant "struct"s, although I was allowed to remove the "enum" from "enum machine_mode". (Of course, more "enum machine_mode"s have crept in since then, because people are so used to the old ways. We're only going to get true consistency with some kind of bootstrap-time checking.) > 4. "int" is the universal type for induction variables and lots of other > stuff in GCC. Isn't it better to use "unsigned" where semantics does not > imply negative values? "int" can be more efficient because overflow is undefined. If you have: for (unsigned int i = n1; i != n2; ++i) a[i] = ...; then the compiler has to consider the possibility that n1 could be greater than n2 and i could wrap around. With "int" it can assume that the accesses to a[] are at increasing indices. I think the right choice here depends on what you're iterating over. > 5. In general: which is better: to support consistency with surrounding > code or to use the new conventions? Particularly, the rule about > declaring variables at the place of first use (in a function where all > other variables are declared in the beginning)? That too isn't so much a convention choice as something that was forced by C. I think declaring new variables at the point of first use makes sense even if there are other variables in the function that are declared at the top. There's also no escaping it if you need to use a non-default constructor and the argument values you need aren't available at the top of the function. I think there are going to be cases where we couldn't be consistent with the surrounding code. Thanks, Richard
