19 ms·

Google C++ Style Guide Is No Good

77 points by eyakubovich 8y ago

jchw 8y ago
>We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes. This is presented without justification as if it must be true. But is it? My view is that you absolutely should have different worlds. You can then limit the number of people and the amount of code using foot gun prone constructs to a very small minority, and instead provide higher level abstractions. I'd absolutely hate to see code that looks like Boost on the business layer of an application.
cma 8y ago
> Exceptions vs error codes debate is much like space-vs-tabs so I will sit this one out. GSG forbids exceptions on the ground of “Google’s existing code is not exception-tolerant”. I’m not sure what that refers to. Read "Exceptional C++". It isn't worth trying to support them. It is an exercise in masochism. It isn't at all like tabs-vs-spaces.
- hendzen 8y ago
  it really isn't that hard if you practice RAII properly. I've used C++ exceptions on several large C++ codebases and not run in to major issues.
  - cma 8y ago
    Template libraries that have to deal with stuff that could throw vs couldn't end up with lots of nearr duplicate code, twice as much needed test coverage, and/or less efficiency if you just cover the could-throw case. And you will still probably get it wrong. Exceptions can work ok in a GCed language, but I haven't seen them work well otherwise. Maybe it is possible, but in C++ it is a huge trap and isn't worth the extra care that will have to go into everything to avoid really bad problems.
    - quotemstr 8y ago
      > Template libraries that have to deal with stuff that could throw vs couldn't end up with lots of nearr duplicate code, twice as much needed test coverage, and/or less efficiency if you just cover the could-throw case. And you will still probably get it wrong. Highly-generic container libraries constitute a tiny minority of all code, and even in this kind of code (of which I've written plenty), the tiny local optimizations that we can do if we, say, know we have a nothrow move constructor do not constitute anything near a 2x code size penalty. You should be grateful that C++ lets you specialize code for can-fail and cannot-fail cases. Try doing that in a sloppy-exceptions language like Java. > Exceptions can work ok in a GCed language, but I haven't seen them work well otherwise. I have hundreds of thousands of lines of my own C++ code that say otherwise. BTW: ever hear of this weird startup called "Facebook"? All of their C++ code uses exceptions. Nevertheless, contra certain prominent C++ influencers, rivers have not flowed with blood, locusts have failed to eat the crops, and the firstborn of the nation are safe in their beds.
    - jcelerier 8y ago
      > Template libraries that have to deal with stuff that could throw vs couldn't end up with lots of nearr duplicate code, twice as much needed test coverage, and/or less efficiency if you just cover the could-throw case. And you will still probably get it wrong. what are you referring to ? I have never seen template libraries having to do anything particular wrt to exceptions. If you want to throw just throw, that's the point.
      - microtherion 8y ago
        Something like a std::vector constructor/assignment operator can get rather messy if the value type constructor can throw.
  - wyldfire 8y ago
    Unwinding can be perilous across library boundaries, this is IMO its greatest weakness.
  - stdplaceholder 8y ago
    It results in slower generated code and much larger binaries, neither of which would be acceptable to Google (some of their binaries are already on the brink of being unbuildable, so a bunch of junk to enable exceptions is not possible.)
    - deleted 8y ago
      [deleted]
    - hendzen 8y ago
      the slower generated code is an academic concern. Show me a realistic/non-contrived benchmark where not keeping the stack unwindable actually helps give a useful performance increase. Error codes force people to litter the code with branches and put error handling code in the hot path of the instruction stream.
      - cma 8y ago
        Using 'likely/unlikely' compiler directives, the instructions in the unlikely error branch can be hoisted out to another area and not significantly affect instruction cache. It still is a branch though.
        jcelerier 8y ago
        and exception-based code can have much less branches since you don't need to check every function call for error but instead let the exception propagate from where they can be thrown. e.g. in a hypothetic case where you load a settings file five layers deep, in an error-code-based solution you have potentially 5 branches while in an exception-based solution you only have one
        tpolzer 8y ago
        At the cost of predictable performance though. With current implementations, the exceptional path will be several magnitudes (!) slower for cheap functions.
      - 8y ago
- kazinator 8y ago
  It's not like tabs-vs-spaces because we can trivially reformat large amounts of code from tabs to spaces or vice versa without changing the token stream or AST, let alone the object code.
- nitwit005 8y ago
  It doesn't matter how Google, or anyone else, feels about exceptions. Using them in a code base that isn't exception safe is essentially guaranteed to cause issues, and Google knew that was the current state of things when they wrote that document.
ramenmeal 8y ago
Does it work for them?
- jsolson 8y ago
  Yes; it enables tens of thousands of developers to work on a monorepo with billions of lines of code across over a billion files and generally be able to move between parts of that codebase without surprises. It promotes re-use, and simplifies refactoring useful utilities from one-shots into general libraries. Speaking for myself as an engineer who works in that codebase, I rarely find it gets in my way, and when I've felt exceptions were the clearest/best approach, I've not found it burdensome or difficult to justify them.
kazinator 8y ago
> GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. Google is right here; a purely build-time issue, like avoiding including a header file twice, should be solved without having to resort to nonstandard language features. The generated code doesn't change in any way, so it is gratuitous. Save the language extensions for doing something that is inherently nonportable, like gaining access to some platform feature.
- loeg 8y ago
  Meh. C and C++ should just standardize on #pragma once. Everything supports it. It's trivial for new compilers to implement. There's no reason a stupid preprocessor hack should still be required in order to prevent double-inclusion.
  - tpolzer 8y ago
    You could say that there's no reason we're still using a stupid preprocessor hack of textual inclusion to import interfaces. As usual, the devil lies in the detail. If you want to make pragma once robust you need to checksum files, which in the end will be slower than include guards.
    - deleted 8y ago
      [deleted]
    - kazinator 8y ago
      If you want to make #pragma once robust, you firstly have to specify the requirements rigorously. The requirements can be specified in such a way that checksumming of files is avoided. We can stipulate that exact (as well as inexact) copies of a file are distinct objects with their own identity under #pragma once, and are not mutually excluded. #pragma once can be defined in terms of a reference model whereby it is equivalent to a machine-generated sequence: #ifndef <ident> #define <ident> #endif where the detailed semantics is tied to how the machine generates <ident>. If <ident> is a digest of the absolute path, then references to the same file via different hard or symbolic links look different and do not mutually exclude. If <ident> is produced from the volume and object identifier (like inode number) then different links to the same header will mutually exclude. If <ident> is a content hash, then identical files will mutually exclude (but we need to deal with hash collisions somehow). A much better solution would be to sidestep this whole thing entirely and just allow any file-scope definition in C++ to be repeated in the same translation unit (with some proviso, like that the multiple definitions have to be identical; and that could be enforced with diagnostics). The multiple inclusions of the same material aren't a problem.
rbanffy 8y ago
The Python one is particularly offensive.
jmgao 8y ago
I disagree with a lot of the Google C++ style guide, but I disagree with many of the complaints in the post. > Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. However forward declarations are useful in contexts like the following: ... Yes, those are the cases where it isn't possible to avoid forward declarations, so it's fine to use them? > While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. Globals like this are not problematic and very useful: No, globals like that are very problematic, so much so that there is a clang warning specifically to detect these (-Wexit-time-destructors). exit runs global destructors, so if one thread calls exit while some other thread is still doing stuff, it might destroy an object that's in use, leading to explosions. Also, if you're exiting, why are you paying to deallocate memory when it's all going to be blown away anyway? > The lynchpin of C++ are value-types. Such types should be copyable and moveable and the language automatically generates the necessary constructors and operators by default. “a copyable class should explicitly declare the copy operations, a move-only class should explicitly declare the move operations” – this goes against the language philosophy. It is very non-obvious which constructors will get automatically generated, because it depends on the types of the members. `Foo(Foo&& move) = default;` makes it immediately obvious.
- Chabs 8y ago
  The most egregious thing for me is: > in other words, the reader is expected to understand the semantics of an unknown function without consulting the declaration (or documentation). The idea that the author considers making code as self-documenting as possible at the call site a conceptual mistake is just weird to me.
- gok 8y ago
  > No, globals like that are very problematic Indeed, they're terrible. What I don't understand is why basically every Google-authored project has loads of these (protobuf, gflags, TensorFlow...)
  - fpoling 8y ago
    In Chromium they are so widespread that makes the rule showing that complex code cannot avoid them and defeating the rule.
cwyers 8y ago
I think the author misunderstands the difference between his use of C++ and Google's use, where Google wants to treat thousands of developers are largely exchangeable over billions of lines of code.
- OnlyRepliesToBS 8y ago
  joy
- jghn 8y ago
  One problem is that many people fail to make that distinction and then cargo cult whatever google puts forth for style guides People need to remember that just because Google does something one way does not imply it is correct nor that it is incorrect
axiometry 8y ago
I can't believe we still have developers who are completely against defensive language design/style. Foot guns are real and deadly. The lack of a restrictive style guide in C++ is how you end up with one of these projects: https://news.ycombinator.com/item?id=18554272 https://news.ycombinator.com/item?id=18554272
mav3rick 8y ago
Style guide is not meant for agreement, it's a general guideline to ensure that when developers come and go, the style and readability doesn't undergo a massive change. I've found it very useful. One can nitpick each style guide to death.
saagarjha 8y ago
I don’t particularly like the style guide either, but the reasons presented here aren’t great: > GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. I mean, it’s non-standard. What more do you need? > Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. The example provided is like the one place you would use a forward declaration. I’m sure it’s fine in this case. > Marking the function “inline” lets the compiler make the decision. Not marking it inline is a sure way to prevent inlining, unless Link Time Optimizations are turned on. …for external linkage only. Inside your own code base, the compiler has this authority everywhere.
- jcelerier 8y ago
  > I mean, it’s non-standard. What more do you need? so are most programming languages used in the world. In practice, #pragma once is less trouble than #ifdef ; we had the debate just today in reddit :-) https://www.reddit.com/r/cpp/comments/a14o5q/real_world_problems_with_pragma_once/ https://www.reddit.com/r/cpp/comments/a14o5q/real_world_prob...
  - groby_b 8y ago
    Nobody is proposing a non-standard language, that's the point. Literally the entire reason for having a coding standard is to have a well-defined subset of the language you're using. Meanwhile, the "trouble" the reddit thread comes up with is copy/paste programming for #ifdef. That's a one-time effort of writing a commit hook that checks for that. But no amount of effort will ensure the next compiler you need supports #pragma once
    - jcelerier 8y ago
      > Literally the entire reason for having a coding standard is to have a well-defined subset of the language you're using. that's a perversion of what "standards" means. Standards are originally there to codify existing practice, not to put new practice into existence. "#pragma once" is common enough that it has its own wikipedia page : https://en.wikipedia.org/wiki/Pragma_once https://en.wikipedia.org/wiki/Pragma_once with the list of all supported compilers.
forrestthewoods 8y ago
> At the same time, every feature that went into the language was vetted for being useful. It was deemed that without the said feature, the code was significantly worse Talk about an appeal to authority! Cppcon had an entire talk titled “The Nightmare of Initialization in C++”. C++ initialization is indeed a total clusterfuck. And that’s not a controversial statement! I’m sure each thing was added to solve a problem. But now there are too many things, and that’s the new problem.
haberman 8y ago
There's a lot here, just a few comments: > Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. However forward declarations are useful in contexts like the following [...] Yes, circular data structures are a great reason to use a forward declaration; probably the best one. I think it is a perfect example of why the recommendation against forward declarations is not a prohibition. Please, please do not forward-declare types you do not own. It constraints the way those types can be refactored. More info here: https://abseil.io/about/compatibility#what-users-must-and-must-not-do https://abseil.io/about/compatibility#what-users-must-and-mu... > The rule basically says that global/static variables with non-trivial constructors and destructors are not allowed. While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. Globals like this are not problematic and very useful: A safe alternative to this is to use a static object inside a function. This can serve the same purpose without the gotchas. > Even the inter-translation unit ordering is not a huge problem – Stroustrup discusses it in his D&E book. Experience says otherwise. In the protobuf code-base we have to put a thread-safe initialization check in all of our constructors because we have to statically initialize our default instances, but we can't guarantee these static initializers will run before other static initializers that use them. Static initialization ordering is a real and difficult problem.
stdplaceholder 8y ago
It's ambiguous in the article, and it's not specifically mentioned in the Google C++ Style Guide, but during code review at Google it's likely that a reviewer will object to this code: std::vector<int> v; v.resize(10, 42); That form of std::vector::resize is discouraged because nobody can remember which argument is which. A reviewer would probably prefer: std::vector<int> v(10); std::fill(v.begin(), v.end(), 42); Or even std::vector<int> v{42, 42, 42, 42, 42, 42, 42, 42, 42, 42}; I have some other quibbles with this article * "Not marking it inline is a sure way to prevent inlining" is totally wrong) * The bit about using-directives is also pretty wrong. See https://abseil.io/tips/153 https://abseil.io/tips/153 for why. And the example the author gives is terrible: void foo() { using namespace std::placeholders; std::bind(bar, _1, 2); This doesn't even need a using-directive. void foo() { using std::placeholders::_1; std::bind(bar, _1, 2); It's shorter, even. To summarize, the Google C++ Style Guide is there to assist reviewers in reviewing new code. Contrary to what the author thinks, the guide doesn't prescribe anything. At Google decisions are always left to the reviewer. Read the Abseil libraries to see how the style guide applies and when it is ignored.
- Jyaif 8y ago
  You'd never see `v.resize(10, 42);` because 10 and 42 would be in self-documenting variables, e.g. ``` int kDefaultValue = 42; size_type kExpectedSize = 10; v.resize(kExpectedSize, kDefaultValue); ```
  - wyldfire 8y ago
    ...though in this case it's still the reviewers job to help understand what the code does and if the reviewer doesn't know for sure which arg comes first then they still can't easily check that the code does what's intended.
    - AstralStorm 8y ago
      The guide should then specify some common instances where arguments may be flipped.
cletus 8y ago
Having written C++ at Google using these (strictly enforced) style guidelines, I actually kind of likes Google's subset of C++. It should be noted that any reasonably sized shop uses a subset of C++. It's insanity not to. It's just a question of what to allow and what not to. Google C++ uses a style of error-handling that's akin to how Go handles errors. Many functions return a util::Status (which would be OK or an error) or, when you needed to return a value, return a util::StatusOr<T>. Some people who write Go rail against the verbosity of this. I actually like it because it makes it pretty easy to reason about what your code is doing without the flow control you can get from exceptions. There are definite warts in the codebase. One big one I remember was that one of the early developers (which might've been Craig Silverstein) didn't believe in unsigned types so there are a bunch of size functions that return an int and this has caused no end of problems. I'm no C++ expert so I can't speak to some of the author's gripes (eg pragma-once vs #define/#ifndef) but a lot of the criticisms suggest that the author hasn't worked on large codebases. Take operator overloading. This is fine, in theory, if used judiciously. The problem is engineers differ on what's reasonable and this may lead to unexpected behaviour, particularly when you start factoring in automatic type conversions. You see this in Scala, for example, where people go nuts, basically because they can and for no other reason. There's a certain type of engineer who gets trapped in a mindset where they start considering complexity a virtue. I once got into a discussion with someone where he was shocked that I didn't know what perfect forwarding was. After looking it up I was like "why do I need to know this (unless I'm writing a templated library)?" and the answer is I don't. Google's C++ style guide is largely about avoiding these flights of fancy making it into codebases that other people rely on, may need to debug, etc. And it achieves that goal as I found Google C++ code quite readable on the whole. There's still a lot of code that doesn't, say, use smart pointers, so you can get dereferencing errors if you're not careful or if you don't understand who owns what. And this isn't something that C++11/14/17 smart pointers are perfect at anyway (something I'm quite bullish about for Rust, in comparison). Another example: the complaint about no reference arguments. The point of this is so you can't tell what function arguments might be mutated if you allow non-const reference function arguments whereas with pointers it's obvious. To be clear: foo(c); // could be pass by value or reference (const or non-const) foo(&c); // clearly mutable I also believe the C++11 thing is outdated. Even when I still worked there some features were allowed from C++14 and some code was obviously going to be migrated to the C++14 equivalent once the issues with that were resolved (IIRC std::make_shared had an equivalent).
wutbrodo 8y ago
> I can already hear an argument that there’s a difference between “library writers” and “everyone else”. Maybe GSG should be waived for “library writers” who are expected to be language experts. This is a dangerous world view. We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes. Every developer should be a “library writer” as it is the only way to decompose the code into manageable, testable and reusable units. When I started at Google out of college, I came in writing clever, elegant code, and had to learn that, in large systems that will be touched by lots of people over large periods of time, readability is often more than worth sacrificing the maximally performant, concise, or elegant system. One of the ways to maintain this across a large organization is by limiting the set of available footguns, and granting exceptions when needed (eg, for "library code"). The latter type of code absolutely is bifurcated from generally-accessible and modifiable systems. The cost of doing the latter is increasing the hurdles to requires to modify the code, which already is and should be the case for widely-used library code. This seems obvious enough to me that I'm not sure how the author is misunderstanding it so badly (either that or I am). Now, the author isn't completely wrong: another thing I've noticed is how much cargo cult, "Google does it" behavior there is among startups. Creating such an large-org-safe subset of C++ usage may not be appropriate for every type of organization, and if you don't understand why you're using GSG, it's probably worth figuring out whether it's a good fit. But the author utterly fails to make the claim that it's "no good", or even that it isn't appropriate in plenty of situations.
sys_64738 8y ago
If you find them no good then you haven't worked for a big company. These docs are standards are your complaints have been heard before and shaped the doc.
anthonyvd 8y ago
This article is so full of misconceptions. > Unlike C++ Core Guidelines that try to explain how to use the language effectively, GSG is about forbidding the use of certain features. They aren't meant to accomplish the same thing. The GSG's goal is to standardize both style and feature set across the mono repo so that thousands of engineers can effectively contribute. > We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes. It's a style guide, not an unwavering book of law. Library writers use some features that aren't necessary in non-library code. In fact, some language features are written specifically to the benefit of the implementers. > GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. When the difference is a 2 lines of code that you probably have tooling to generate anyways, why not default to the thing that's standard? > Yes, both of these forward declarations are possible to avoid by type-punning through a void* but it is not a good pattern. The point of "Avoid using fwd-declarations where possible" is exactly that: reason about the code and whether the declaration is necessary. It's not "dogmatically avoid forward declarations". I'm sure no-one at Google is punning through void* for those examples. > Marking the function “inline” lets the compiler make the decision. Not marking it inline is a sure way to prevent inlining, unless Link Time Optimizations are turned on. That's simply not true. The compiler can inline a function if it can infer that it doesn't alter the program's behavior. Generally, tools make good decisions. Override them when you've measured that something else is better. > Library code should always be placed in a namespace. Top level/application code has questionable value being placed in a namespace. It's easier to be diligent about placing everything in a namespace because application code doesn't always remain so. Especially not in a gigantic mono repo like Google's. > GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants? I mean, you declare your constants in the header and define them in the implementation file. First example I can find in Chromium: https://cs.chromium.org/chromium/src/ios/chrome/browser/pref_names.h?rcl=82d8ee1e070100260963c8d76f23dcd2884099c9&l=11 https://cs.chromium.org/chromium/src/ios/chrome/browser/pref... > The rule basically says that global/static variables with non-trivial constructors and destructors are not allowed. While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. This rule isn't overly prohibitive in the context of large applications that require clean startups/shutdowns and contain objects that are sensitive to construction/destruction order. > “Do not define implicit conversions”. I would urge the reader to consider what life would be like if std::string(const char) constructor was marked explicit (especially in the absense of user defined literals, which GSG also outlaws). Nuff said. The issue with implicit conversions is that they're not explicit to callers. Honestly, having to wrap a few const char* into std::string() calls wouldn't be as bad as you seem to think it would be. Explicit code is easier to read and reason about. > “a copyable class should explicitly declare the copy operations, a move-only class should explicitly declare the move operations” – this goes against the language philosophy. Again, this is about being explicit, and using safe defaults. It's not prohibited to make things copyable and movable but defaulting to the most restrictive set up and adding things as needed ensures that developers thought about the implications of those properties. > In this pattern the private base cannot be replaced by a data member because the base is constructed before the members. I've never seen code with private inheritance that made sense and couldn't be refactored to something clearer. > Operator Overloading See my point about implicit conversion above because it's the same thing here. Operator Overloading tends to obfuscate code. > I think this rule mixes together two ideas for no good reason. C++ has a clear way to denote mutability – the const qualifier. Using pointer vs reference to signal mutability goes against the language. The const qualifier isn't visible from the call site. This rule makes it so that you can reason about what will happen to the values you're passing to a function based on whether you pass them by pointer, or by reference/value. It also prevents a library from changing an argument from const T& to T&, introduce mutations, and break callers as a result. > Exceptions Google's code is just setup to std::terminate instead of throwing. It's a minor performance gain AFAIK but it also avoids littering the code with try {} catch {} blocks. It forces developers to handle errors instead of propagating them whenever possible too. > “Use lambda expressions where appropriate.” – probably a cheap shot, but what is the alternative? – use them where it’s not appropriate? This point seems counter-productive considering the author claims the guide is written in too much of a prohibitive fashion. > Avoid complicated template programming. TMP is hard, increases compilation times, and is difficult to reason about. The Guide recognizes its usefulness but suggests avoiding it where possible. This is similar to the library writer vs application writers argument: not everyone needs it, avoid it if possible. All in all I think the author just doesn't have the same requirements, constraints, and sheer amount of developers/lines of code Google has. Nothing is forcing them to use the Guide. In fact, it's public but it was written for Google, by Google. It works for Google, and it's a great tool in that kind of organization. Disclaimer: I work on Chrome, which has its own guide derived from the GSG.
jupp0r 8y ago
It seems like the author didn't read the guidelines very well. Especially this part: "The intent of this document is to provide maximal guidance with reasonable restriction. As always, common sense and good taste should prevail." There are good arguments for deriving from the guidelines in lots of situations, guidelines merely establish the default style, but don't prohibit code that does not follow the guidelines _if_ there is a good reason to. What the guidelines establish is a duty to justify deviations.
- coliveira 8y ago
  The author is complaining about the use of these guidelines in other places outside Google, especially for people who take these guidelines more seriously than Google itself.
  - jupp0r 8y ago
    I don't work at google, but do use the guidelines for github.com/jupp0r/prometheus-cpp. Why? because it's reasonable subset of C++ and it establishes a consistent style that I can point people to in PRs.
- eyakubovich 8y ago
  The trouble with "this is not the law" argument is that it's not clear how to make the judgement. The reviewer should be using the style guide during the review and should call out any violations. A restrictive guide then places a huge burden on the author to justify the deviations.
sigacts 8y ago
Google's
rocqua 8y ago
Regarding the advice on globals, and the comments here regarding destructors, how does `constexpr` interact with RAII? Are `constexpr` values destructed when they go out of scope? Or is it simply not possible to `constexpr` them?
- Chabs 8y ago
  constexpr variables must have a literal type, which requires a trivial destructor.
kbumsik 8y ago
> The #define Guard. GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. What? I cannot read more after this.
anyfoo 8y ago
It seems to me that the biggest thing that the author does not know about the provenance of Google's style guide is just how massive Google's projects that this guide applies to really are. A lot of the author's complaints may not make sense on their project with a few engineers, but they are absolutely vital in a code base on which thousands of engineers work on every day. Those engineers often have to touch parts of the monorepo that are far away from what they usually work on, creating changes that need to go through reviews by teams that they have never interacted with before, and in turn need to be understood by future engineers in a similar position. For example, the author states that "top level/application code" does not need to be in a namespace, but it's often downright absurd to classify what's considered "top level" code in Google's monorepo. I also chuckled at this: > “Do not define implicit conversions”. I would urge the reader to consider what life would be like if std::string(const char*) constructor was marked explicit (especially in the absense of user defined literals, which GSG also outlaws). Nuff said. At least when I worked with the code (many years ago, parts of Maps specifically), Google had its entirely own string handling routines that followed their conventions, and I did not miss std::string's implicit constructor much. This is a completely different world, where the standard library does not matter, and almost every code comes from Google themselves.
- liftbigweights 8y ago
  > It seems to me that the biggest thing that the author does not know about the provenance of Google's style guide is just how massive Google's projects that this guide applies to really are. A lot of the author's complaints may not make sense on their project with a few engineers, but they are absolutely vital in a code base on which thousands of engineers work on every day. I agree. People work on their small pet projects for school or wherever and think that's how enterprise development really works. Unless they have worked on a major project, it's very difficult for them to understand. But even more important than sytle, it's consistency. You shouldn't use a coding style/standard because it works for google. You should create a coding style that works for your team or organization and stick to it consistently. Oftentimes, keeping to the style is more important than the style itself.
  - 8y ago
SlowRobotAhead 8y ago
Similar but unrelated topic... Does anyone have a good C style guide they like?
dajonker 8y ago
Perhaps it's a matter of rule-based versus principle-based view of the world. Generally speaking, in a principle-based system, there is room for different interpretations of the stated principles. Edge cases usually don't require special mention. In a rule-based system, rules are strictly defined and enforced with little room for interpretation. Anything not explicitly mentioned is a grey area and edge cases often end up amending the rules so they are covered as well. Sounds to me like the author prefers the rule-based approach and/or is assuming these rules are strictly enforced.
hellofunk 8y ago
Fun piece of trivia for you: The C++ Core Guidelines referenced in this article are longer than the language spec for Go.
ryandrake 8y ago
I’m not a huge fan of the GSG, at least the C++ one. It seems to be less about style and more about forbidding scary language features. A language feature only becomes a “foot gun” in the hands of a careless developer. Maybe it’s too idealistic but IMO it’s better to fix the “careless developer” problem at the interview stage, not in a style guideline. Hire developers who voluntarily follow best practices and use good judgment...don’t try to mandate it with an arbitrary list of forbidden practices.
olliej 8y ago
There are some super boneheaded comments in here. * "Don't use forward declarations unless necessary" with the counter example of why it's bad, being a case we're they're necessary? * The complaint against "inline" is nonsense. The only thing inline impacts is function linkage. Literally nothing else. If the compiler thinks a function is profitable to inline, it will inline it. If it does not think inlining is profitable it won't, the fact that you say "inline" means nothing. * Complaining about complex globals initialisers: they directly impact library load time, they do cause breakages on ordering. Yes you could structure your code to avoid ordering issues, but then every developer needs to work for all time to avoid breaking it. Or you could just not use them, and it's not a problem. * Restricting copies: It should be super easy to tell at a glance if code is going to be needlessly inefficient. Easiest way to ensure that is to make your code fail to compile. * Exceptions: they add significantly to code size and launch time. The failure modes are hard to reason about. The standard idiom everyone is move to is option<> or result<> types. It means you have to handle errors, or be explicit in ignoring them. * Template meta programming is extremely slow to compile, and very difficult for people to understand. constexpr is easier to read and understand, more efficient to compile, and can also be reused for non-constant cases. * Boost: if you're using boost then you're requiring all your developers to install boost. Then you need to also ensure that they're all using the same version, and then people can end up requiring multiple versions. Again, super large company, with many many engineers and many projects mean the chances of multiple projects depending on different versions of libraries will cause misery. * C++11 - Cool, it's 2018, guess what: many machines are running versions of an OS that has a 2018 edition of the C++ standard library. If you compile targeting newer versions of the standard library then you can't ship to those systems, unless you include a copy of the standard library in your binary. Then you have to hope your copy of the standard library doesn't conflict with any loaded by the system's own libraries.
- larl 8y ago
  Or even: C++11 - Cool, it's 2018, guess what: Migrating a codebase as large as Google's from C++11 to C++1x is a bunch of work. And a lot of the C++1x goodies are library additions, available in Abseil <https://github.com/abseil/abseil-cpp> https://github.com/abseil/abseil-cpp>.
DannyBee 8y ago
It's very interesting to me that the author writes a critique of a style guide, but doesn't seem to have tried to understand the perspective of those who created it or what their goals were (they basically give this lip service) They don't really try and understand the rationale in detail and then argue that that rationale would be better served through a different set of features, they mostly just say "hey I think Google is wrong and these features are pretty cool"
Carpetsmoker 8y ago
Laughs in gofmt
bausshf 8y ago
"Unnamed Namespaces and Static Variables GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants?" A constant is not a static variable.
knorker 8y ago
> GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants? Uhm… what? 1) I would challenge the author to produce any useful use of anonymous namespaces in header files that doesn't violate ODR. I'd also challenge them to not break ODR with constants in header files in the way they seem to mean. I should add: without UB, plz. 2) How else do we declare constants? Uhm, how about: extern const int foo; or just "inline constexpr int foo = 1" in header but non-static and not in anonymous namespace? > Globals like this are not problematic and very useful: [example of nontrivially destructible global] It doesn't look dangerous, but allowing it does not scale to a billion lines of code. Code easier to reason about is less buggy code. In general though: Author spends 1 SWE-year per year on coding, Google spends what, 100 SWE-millenia per year? It's optimizing for that use case.
alexeiz 8y ago
The thing that bothers me about GSG is that it seems to be enforced across Google no matter what team you're in or what project you work on. Think about it: somebody who thinks that he knows better than you decided how you and other 10000s developers at Google should use C++. I think such approach is incredibly inflexible and shortsighted. Even if that somebody is a well-renowned C++ expert I would still doubt he can come up with the C++ style guide applicable to the whole Google. If I lead a project at Google I don't want someone unrelated to my project or some committee higher up to make decisions for my team. I want my team to decide how to use C++ (or any other language or technology for that matter) based on the project needs and the team developer skills. One coding style for everyone reduces the skill level of everyone to the lowest common denominator. Even if you have better skilled developers around you, if they can't use their skills professionally, they either quit or lose their skills. Soon you'll be surrounded by developers with only poor skills who you can learn nothing from. I remember a story told by Sean Parent (if you don't know him, look him up) about the time he worked at Google and he was told that his code is hard to understand because it uses concepts and ideas nobody at Google is familiar with (because they are likely not part of the GSG). I find that story quite instructive.

77 points by eyakubovich 8y ago

jchw 8y ago

>We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes. This is presented without justification as if it must be true. But is it? My view is that you absolutely should have different worlds. You can then limit the number of people and the amount of code using foot gun prone constructs to a very small minority, and instead provide higher level abstractions. I'd absolutely hate to see code that looks like Boost on the business layer of an application.

cma 8y ago

> Exceptions vs error codes debate is much like space-vs-tabs so I will sit this one out. GSG forbids exceptions on the ground of “Google’s existing code is not exception-tolerant”. I’m not sure what that refers to. Read "Exceptional C++". It isn't worth trying to support them. It is an exercise in masochism. It isn't at all like tabs-vs-spaces.

hendzen 8y ago
it really isn't that hard if you practice RAII properly. I've used C++ exceptions on several large C++ codebases and not run in to major issues.
- cma 8y ago
  Template libraries that have to deal with stuff that could throw vs couldn't end up with lots of nearr duplicate code, twice as much needed test coverage, and/or less efficiency if you just cover the could-throw case. And you will still probably get it wrong. Exceptions can work ok in a GCed language, but I haven't seen them work well otherwise. Maybe it is possible, but in C++ it is a huge trap and isn't worth the extra care that will have to go into everything to avoid really bad problems.
  - quotemstr 8y ago
    > Template libraries that have to deal with stuff that could throw vs couldn't end up with lots of nearr duplicate code, twice as much needed test coverage, and/or less efficiency if you just cover the could-throw case. And you will still probably get it wrong. Highly-generic container libraries constitute a tiny minority of all code, and even in this kind of code (of which I've written plenty), the tiny local optimizations that we can do if we, say, know we have a nothrow move constructor do not constitute anything near a 2x code size penalty. You should be grateful that C++ lets you specialize code for can-fail and cannot-fail cases. Try doing that in a sloppy-exceptions language like Java. > Exceptions can work ok in a GCed language, but I haven't seen them work well otherwise. I have hundreds of thousands of lines of my own C++ code that say otherwise. BTW: ever hear of this weird startup called "Facebook"? All of their C++ code uses exceptions. Nevertheless, contra certain prominent C++ influencers, rivers have not flowed with blood, locusts have failed to eat the crops, and the firstborn of the nation are safe in their beds.
  - jcelerier 8y ago
    > Template libraries that have to deal with stuff that could throw vs couldn't end up with lots of nearr duplicate code, twice as much needed test coverage, and/or less efficiency if you just cover the could-throw case. And you will still probably get it wrong. what are you referring to ? I have never seen template libraries having to do anything particular wrt to exceptions. If you want to throw just throw, that's the point.
    - microtherion 8y ago
      Something like a std::vector constructor/assignment operator can get rather messy if the value type constructor can throw.
- wyldfire 8y ago
  Unwinding can be perilous across library boundaries, this is IMO its greatest weakness.
- stdplaceholder 8y ago
  It results in slower generated code and much larger binaries, neither of which would be acceptable to Google (some of their binaries are already on the brink of being unbuildable, so a bunch of junk to enable exceptions is not possible.)
  - deleted 8y ago
    [deleted]
  - hendzen 8y ago
    the slower generated code is an academic concern. Show me a realistic/non-contrived benchmark where not keeping the stack unwindable actually helps give a useful performance increase. Error codes force people to litter the code with branches and put error handling code in the hot path of the instruction stream.
    - cma 8y ago
      Using 'likely/unlikely' compiler directives, the instructions in the unlikely error branch can be hoisted out to another area and not significantly affect instruction cache. It still is a branch though.
      - jcelerier 8y ago
        and exception-based code can have much less branches since you don't need to check every function call for error but instead let the exception propagate from where they can be thrown. e.g. in a hypothetic case where you load a settings file five layers deep, in an error-code-based solution you have potentially 5 branches while in an exception-based solution you only have one
        tpolzer 8y ago
        At the cost of predictable performance though. With current implementations, the exceptional path will be several magnitudes (!) slower for cheap functions.
    - 8y ago
kazinator 8y ago
It's not like tabs-vs-spaces because we can trivially reformat large amounts of code from tabs to spaces or vice versa without changing the token stream or AST, let alone the object code.
nitwit005 8y ago
It doesn't matter how Google, or anyone else, feels about exceptions. Using them in a code base that isn't exception safe is essentially guaranteed to cause issues, and Google knew that was the current state of things when they wrote that document.

ramenmeal 8y ago

Does it work for them?

jsolson 8y ago
Yes; it enables tens of thousands of developers to work on a monorepo with billions of lines of code across over a billion files and generally be able to move between parts of that codebase without surprises. It promotes re-use, and simplifies refactoring useful utilities from one-shots into general libraries. Speaking for myself as an engineer who works in that codebase, I rarely find it gets in my way, and when I've felt exceptions were the clearest/best approach, I've not found it burdensome or difficult to justify them.

kazinator 8y ago

> GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. Google is right here; a purely build-time issue, like avoiding including a header file twice, should be solved without having to resort to nonstandard language features. The generated code doesn't change in any way, so it is gratuitous. Save the language extensions for doing something that is inherently nonportable, like gaining access to some platform feature.

loeg 8y ago
Meh. C and C++ should just standardize on #pragma once. Everything supports it. It's trivial for new compilers to implement. There's no reason a stupid preprocessor hack should still be required in order to prevent double-inclusion.
- tpolzer 8y ago
  You could say that there's no reason we're still using a stupid preprocessor hack of textual inclusion to import interfaces. As usual, the devil lies in the detail. If you want to make pragma once robust you need to checksum files, which in the end will be slower than include guards.
  - deleted 8y ago
    [deleted]
  - kazinator 8y ago
    If you want to make #pragma once robust, you firstly have to specify the requirements rigorously. The requirements can be specified in such a way that checksumming of files is avoided. We can stipulate that exact (as well as inexact) copies of a file are distinct objects with their own identity under #pragma once, and are not mutually excluded. #pragma once can be defined in terms of a reference model whereby it is equivalent to a machine-generated sequence: #ifndef <ident> #define <ident> #endif where the detailed semantics is tied to how the machine generates <ident>. If <ident> is a digest of the absolute path, then references to the same file via different hard or symbolic links look different and do not mutually exclude. If <ident> is produced from the volume and object identifier (like inode number) then different links to the same header will mutually exclude. If <ident> is a content hash, then identical files will mutually exclude (but we need to deal with hash collisions somehow). A much better solution would be to sidestep this whole thing entirely and just allow any file-scope definition in C++ to be repeated in the same translation unit (with some proviso, like that the multiple definitions have to be identical; and that could be enforced with diagnostics). The multiple inclusions of the same material aren't a problem.

rbanffy 8y ago

The Python one is particularly offensive.

jmgao 8y ago

I disagree with a lot of the Google C++ style guide, but I disagree with many of the complaints in the post. > Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. However forward declarations are useful in contexts like the following: ... Yes, those are the cases where it isn't possible to avoid forward declarations, so it's fine to use them? > While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. Globals like this are not problematic and very useful: No, globals like that are very problematic, so much so that there is a clang warning specifically to detect these (-Wexit-time-destructors). exit runs global destructors, so if one thread calls exit while some other thread is still doing stuff, it might destroy an object that's in use, leading to explosions. Also, if you're exiting, why are you paying to deallocate memory when it's all going to be blown away anyway? > The lynchpin of C++ are value-types. Such types should be copyable and moveable and the language automatically generates the necessary constructors and operators by default. “a copyable class should explicitly declare the copy operations, a move-only class should explicitly declare the move operations” – this goes against the language philosophy. It is very non-obvious which constructors will get automatically generated, because it depends on the types of the members. `Foo(Foo&& move) = default;` makes it immediately obvious.

Chabs 8y ago
The most egregious thing for me is: > in other words, the reader is expected to understand the semantics of an unknown function without consulting the declaration (or documentation). The idea that the author considers making code as self-documenting as possible at the call site a conceptual mistake is just weird to me.
gok 8y ago
> No, globals like that are very problematic Indeed, they're terrible. What I don't understand is why basically every Google-authored project has loads of these (protobuf, gflags, TensorFlow...)
- fpoling 8y ago
  In Chromium they are so widespread that makes the rule showing that complex code cannot avoid them and defeating the rule.

cwyers 8y ago

I think the author misunderstands the difference between his use of C++ and Google's use, where Google wants to treat thousands of developers are largely exchangeable over billions of lines of code.

OnlyRepliesToBS 8y ago
joy
jghn 8y ago
One problem is that many people fail to make that distinction and then cargo cult whatever google puts forth for style guides People need to remember that just because Google does something one way does not imply it is correct nor that it is incorrect

axiometry 8y ago

I can't believe we still have developers who are completely against defensive language design/style. Foot guns are real and deadly. The lack of a restrictive style guide in C++ is how you end up with one of these projects: https://news.ycombinator.com/item?id=18554272 https://news.ycombinator.com/item?id=18554272

mav3rick 8y ago

Style guide is not meant for agreement, it's a general guideline to ensure that when developers come and go, the style and readability doesn't undergo a massive change. I've found it very useful. One can nitpick each style guide to death.

saagarjha 8y ago

I don’t particularly like the style guide either, but the reasons presented here aren’t great: > GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. I mean, it’s non-standard. What more do you need? > Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. The example provided is like the one place you would use a forward declaration. I’m sure it’s fine in this case. > Marking the function “inline” lets the compiler make the decision. Not marking it inline is a sure way to prevent inlining, unless Link Time Optimizations are turned on. …for external linkage only. Inside your own code base, the compiler has this authority everywhere.

jcelerier 8y ago
> I mean, it’s non-standard. What more do you need? so are most programming languages used in the world. In practice, #pragma once is less trouble than #ifdef ; we had the debate just today in reddit :-) https://www.reddit.com/r/cpp/comments/a14o5q/real_world_problems_with_pragma_once/ https://www.reddit.com/r/cpp/comments/a14o5q/real_world_prob...
- groby_b 8y ago
  Nobody is proposing a non-standard language, that's the point. Literally the entire reason for having a coding standard is to have a well-defined subset of the language you're using. Meanwhile, the "trouble" the reddit thread comes up with is copy/paste programming for #ifdef. That's a one-time effort of writing a commit hook that checks for that. But no amount of effort will ensure the next compiler you need supports #pragma once
  - jcelerier 8y ago
    > Literally the entire reason for having a coding standard is to have a well-defined subset of the language you're using. that's a perversion of what "standards" means. Standards are originally there to codify existing practice, not to put new practice into existence. "#pragma once" is common enough that it has its own wikipedia page : https://en.wikipedia.org/wiki/Pragma_once https://en.wikipedia.org/wiki/Pragma_once with the list of all supported compilers.

forrestthewoods 8y ago

> At the same time, every feature that went into the language was vetted for being useful. It was deemed that without the said feature, the code was significantly worse Talk about an appeal to authority! Cppcon had an entire talk titled “The Nightmare of Initialization in C++”. C++ initialization is indeed a total clusterfuck. And that’s not a controversial statement! I’m sure each thing was added to solve a problem. But now there are too many things, and that’s the new problem.

haberman 8y ago

There's a lot here, just a few comments: > Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. However forward declarations are useful in contexts like the following [...] Yes, circular data structures are a great reason to use a forward declaration; probably the best one. I think it is a perfect example of why the recommendation against forward declarations is not a prohibition. Please, please do not forward-declare types you do not own. It constraints the way those types can be refactored. More info here: https://abseil.io/about/compatibility#what-users-must-and-must-not-do https://abseil.io/about/compatibility#what-users-must-and-mu... > The rule basically says that global/static variables with non-trivial constructors and destructors are not allowed. While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. Globals like this are not problematic and very useful: A safe alternative to this is to use a static object inside a function. This can serve the same purpose without the gotchas. > Even the inter-translation unit ordering is not a huge problem – Stroustrup discusses it in his D&E book. Experience says otherwise. In the protobuf code-base we have to put a thread-safe initialization check in all of our constructors because we have to statically initialize our default instances, but we can't guarantee these static initializers will run before other static initializers that use them. Static initialization ordering is a real and difficult problem.

stdplaceholder 8y ago

It's ambiguous in the article, and it's not specifically mentioned in the Google C++ Style Guide, but during code review at Google it's likely that a reviewer will object to this code: std::vector<int> v; v.resize(10, 42); That form of std::vector::resize is discouraged because nobody can remember which argument is which. A reviewer would probably prefer: std::vector<int> v(10); std::fill(v.begin(), v.end(), 42); Or even std::vector<int> v{42, 42, 42, 42, 42, 42, 42, 42, 42, 42}; I have some other quibbles with this article * "Not marking it inline is a sure way to prevent inlining" is totally wrong) * The bit about using-directives is also pretty wrong. See https://abseil.io/tips/153 https://abseil.io/tips/153 for why. And the example the author gives is terrible: void foo() { using namespace std::placeholders; std::bind(bar, _1, 2); This doesn't even need a using-directive. void foo() { using std::placeholders::_1; std::bind(bar, _1, 2); It's shorter, even. To summarize, the Google C++ Style Guide is there to assist reviewers in reviewing new code. Contrary to what the author thinks, the guide doesn't prescribe anything. At Google decisions are always left to the reviewer. Read the Abseil libraries to see how the style guide applies and when it is ignored.

Jyaif 8y ago
You'd never see `v.resize(10, 42);` because 10 and 42 would be in self-documenting variables, e.g. ``` int kDefaultValue = 42; size_type kExpectedSize = 10; v.resize(kExpectedSize, kDefaultValue); ```
- wyldfire 8y ago
  ...though in this case it's still the reviewers job to help understand what the code does and if the reviewer doesn't know for sure which arg comes first then they still can't easily check that the code does what's intended.
  - AstralStorm 8y ago
    The guide should then specify some common instances where arguments may be flipped.

cletus 8y ago

Having written C++ at Google using these (strictly enforced) style guidelines, I actually kind of likes Google's subset of C++. It should be noted that any reasonably sized shop uses a subset of C++. It's insanity not to. It's just a question of what to allow and what not to. Google C++ uses a style of error-handling that's akin to how Go handles errors. Many functions return a util::Status (which would be OK or an error) or, when you needed to return a value, return a util::StatusOr<T>. Some people who write Go rail against the verbosity of this. I actually like it because it makes it pretty easy to reason about what your code is doing without the flow control you can get from exceptions. There are definite warts in the codebase. One big one I remember was that one of the early developers (which might've been Craig Silverstein) didn't believe in unsigned types so there are a bunch of size functions that return an int and this has caused no end of problems. I'm no C++ expert so I can't speak to some of the author's gripes (eg pragma-once vs #define/#ifndef) but a lot of the criticisms suggest that the author hasn't worked on large codebases. Take operator overloading. This is fine, in theory, if used judiciously. The problem is engineers differ on what's reasonable and this may lead to unexpected behaviour, particularly when you start factoring in automatic type conversions. You see this in Scala, for example, where people go nuts, basically because they can and for no other reason. There's a certain type of engineer who gets trapped in a mindset where they start considering complexity a virtue. I once got into a discussion with someone where he was shocked that I didn't know what perfect forwarding was. After looking it up I was like "why do I need to know this (unless I'm writing a templated library)?" and the answer is I don't. Google's C++ style guide is largely about avoiding these flights of fancy making it into codebases that other people rely on, may need to debug, etc. And it achieves that goal as I found Google C++ code quite readable on the whole. There's still a lot of code that doesn't, say, use smart pointers, so you can get dereferencing errors if you're not careful or if you don't understand who owns what. And this isn't something that C++11/14/17 smart pointers are perfect at anyway (something I'm quite bullish about for Rust, in comparison). Another example: the complaint about no reference arguments. The point of this is so you can't tell what function arguments might be mutated if you allow non-const reference function arguments whereas with pointers it's obvious. To be clear: foo(c); // could be pass by value or reference (const or non-const) foo(&c); // clearly mutable I also believe the C++11 thing is outdated. Even when I still worked there some features were allowed from C++14 and some code was obviously going to be migrated to the C++14 equivalent once the issues with that were resolved (IIRC std::make_shared had an equivalent).

wutbrodo 8y ago

> I can already hear an argument that there’s a difference between “library writers” and “everyone else”. Maybe GSG should be waived for “library writers” who are expected to be language experts. This is a dangerous world view. We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes. Every developer should be a “library writer” as it is the only way to decompose the code into manageable, testable and reusable units. When I started at Google out of college, I came in writing clever, elegant code, and had to learn that, in large systems that will be touched by lots of people over large periods of time, readability is often more than worth sacrificing the maximally performant, concise, or elegant system. One of the ways to maintain this across a large organization is by limiting the set of available footguns, and granting exceptions when needed (eg, for "library code"). The latter type of code absolutely is bifurcated from generally-accessible and modifiable systems. The cost of doing the latter is increasing the hurdles to requires to modify the code, which already is and should be the case for widely-used library code. This seems obvious enough to me that I'm not sure how the author is misunderstanding it so badly (either that or I am). Now, the author isn't completely wrong: another thing I've noticed is how much cargo cult, "Google does it" behavior there is among startups. Creating such an large-org-safe subset of C++ usage may not be appropriate for every type of organization, and if you don't understand why you're using GSG, it's probably worth figuring out whether it's a good fit. But the author utterly fails to make the claim that it's "no good", or even that it isn't appropriate in plenty of situations.

sys_64738 8y ago

If you find them no good then you haven't worked for a big company. These docs are standards are your complaints have been heard before and shaped the doc.

anthonyvd 8y ago

This article is so full of misconceptions. > Unlike C++ Core Guidelines that try to explain how to use the language effectively, GSG is about forbidding the use of certain features. They aren't meant to accomplish the same thing. The GSG's goal is to standardize both style and feature set across the mono repo so that thousands of engineers can effectively contribute. > We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes. It's a style guide, not an unwavering book of law. Library writers use some features that aren't necessary in non-library code. In fact, some language features are written specifically to the benefit of the implementers. > GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. When the difference is a 2 lines of code that you probably have tooling to generate anyways, why not default to the thing that's standard? > Yes, both of these forward declarations are possible to avoid by type-punning through a void* but it is not a good pattern. The point of "Avoid using fwd-declarations where possible" is exactly that: reason about the code and whether the declaration is necessary. It's not "dogmatically avoid forward declarations". I'm sure no-one at Google is punning through void* for those examples. > Marking the function “inline” lets the compiler make the decision. Not marking it inline is a sure way to prevent inlining, unless Link Time Optimizations are turned on. That's simply not true. The compiler can inline a function if it can infer that it doesn't alter the program's behavior. Generally, tools make good decisions. Override them when you've measured that something else is better. > Library code should always be placed in a namespace. Top level/application code has questionable value being placed in a namespace. It's easier to be diligent about placing everything in a namespace because application code doesn't always remain so. Especially not in a gigantic mono repo like Google's. > GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants? I mean, you declare your constants in the header and define them in the implementation file. First example I can find in Chromium: https://cs.chromium.org/chromium/src/ios/chrome/browser/pref_names.h?rcl=82d8ee1e070100260963c8d76f23dcd2884099c9&l=11 https://cs.chromium.org/chromium/src/ios/chrome/browser/pref... > The rule basically says that global/static variables with non-trivial constructors and destructors are not allowed. While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. This rule isn't overly prohibitive in the context of large applications that require clean startups/shutdowns and contain objects that are sensitive to construction/destruction order. > “Do not define implicit conversions”. I would urge the reader to consider what life would be like if std::string(const char) constructor was marked explicit (especially in the absense of user defined literals, which GSG also outlaws). Nuff said. The issue with implicit conversions is that they're not explicit to callers. Honestly, having to wrap a few const char* into std::string() calls wouldn't be as bad as you seem to think it would be. Explicit code is easier to read and reason about. > “a copyable class should explicitly declare the copy operations, a move-only class should explicitly declare the move operations” – this goes against the language philosophy. Again, this is about being explicit, and using safe defaults. It's not prohibited to make things copyable and movable but defaulting to the most restrictive set up and adding things as needed ensures that developers thought about the implications of those properties. > In this pattern the private base cannot be replaced by a data member because the base is constructed before the members. I've never seen code with private inheritance that made sense and couldn't be refactored to something clearer. > Operator Overloading See my point about implicit conversion above because it's the same thing here. Operator Overloading tends to obfuscate code. > I think this rule mixes together two ideas for no good reason. C++ has a clear way to denote mutability – the const qualifier. Using pointer vs reference to signal mutability goes against the language. The const qualifier isn't visible from the call site. This rule makes it so that you can reason about what will happen to the values you're passing to a function based on whether you pass them by pointer, or by reference/value. It also prevents a library from changing an argument from const T& to T&, introduce mutations, and break callers as a result. > Exceptions Google's code is just setup to std::terminate instead of throwing. It's a minor performance gain AFAIK but it also avoids littering the code with try {} catch {} blocks. It forces developers to handle errors instead of propagating them whenever possible too. > “Use lambda expressions where appropriate.” – probably a cheap shot, but what is the alternative? – use them where it’s not appropriate? This point seems counter-productive considering the author claims the guide is written in too much of a prohibitive fashion. > Avoid complicated template programming. TMP is hard, increases compilation times, and is difficult to reason about. The Guide recognizes its usefulness but suggests avoiding it where possible. This is similar to the library writer vs application writers argument: not everyone needs it, avoid it if possible. All in all I think the author just doesn't have the same requirements, constraints, and sheer amount of developers/lines of code Google has. Nothing is forcing them to use the Guide. In fact, it's public but it was written for Google, by Google. It works for Google, and it's a great tool in that kind of organization. Disclaimer: I work on Chrome, which has its own guide derived from the GSG.

jupp0r 8y ago

It seems like the author didn't read the guidelines very well. Especially this part: "The intent of this document is to provide maximal guidance with reasonable restriction. As always, common sense and good taste should prevail." There are good arguments for deriving from the guidelines in lots of situations, guidelines merely establish the default style, but don't prohibit code that does not follow the guidelines _if_ there is a good reason to. What the guidelines establish is a duty to justify deviations.

coliveira 8y ago
The author is complaining about the use of these guidelines in other places outside Google, especially for people who take these guidelines more seriously than Google itself.
- jupp0r 8y ago
  I don't work at google, but do use the guidelines for github.com/jupp0r/prometheus-cpp. Why? because it's reasonable subset of C++ and it establishes a consistent style that I can point people to in PRs.
eyakubovich 8y ago
The trouble with "this is not the law" argument is that it's not clear how to make the judgement. The reviewer should be using the style guide during the review and should call out any violations. A restrictive guide then places a huge burden on the author to justify the deviations.

sigacts 8y ago

Google's

rocqua 8y ago

Regarding the advice on globals, and the comments here regarding destructors, how does `constexpr` interact with RAII? Are `constexpr` values destructed when they go out of scope? Or is it simply not possible to `constexpr` them?

Chabs 8y ago
constexpr variables must have a literal type, which requires a trivial destructor.

kbumsik 8y ago

> The #define Guard. GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported. What? I cannot read more after this.

anyfoo 8y ago

It seems to me that the biggest thing that the author does not know about the provenance of Google's style guide is just how massive Google's projects that this guide applies to really are. A lot of the author's complaints may not make sense on their project with a few engineers, but they are absolutely vital in a code base on which thousands of engineers work on every day. Those engineers often have to touch parts of the monorepo that are far away from what they usually work on, creating changes that need to go through reviews by teams that they have never interacted with before, and in turn need to be understood by future engineers in a similar position. For example, the author states that "top level/application code" does not need to be in a namespace, but it's often downright absurd to classify what's considered "top level" code in Google's monorepo. I also chuckled at this: > “Do not define implicit conversions”. I would urge the reader to consider what life would be like if std::string(const char*) constructor was marked explicit (especially in the absense of user defined literals, which GSG also outlaws). Nuff said. At least when I worked with the code (many years ago, parts of Maps specifically), Google had its entirely own string handling routines that followed their conventions, and I did not miss std::string's implicit constructor much. This is a completely different world, where the standard library does not matter, and almost every code comes from Google themselves.

liftbigweights 8y ago
> It seems to me that the biggest thing that the author does not know about the provenance of Google's style guide is just how massive Google's projects that this guide applies to really are. A lot of the author's complaints may not make sense on their project with a few engineers, but they are absolutely vital in a code base on which thousands of engineers work on every day. I agree. People work on their small pet projects for school or wherever and think that's how enterprise development really works. Unless they have worked on a major project, it's very difficult for them to understand. But even more important than sytle, it's consistency. You shouldn't use a coding style/standard because it works for google. You should create a coding style that works for your team or organization and stick to it consistently. Oftentimes, keeping to the style is more important than the style itself.
- 8y ago

SlowRobotAhead 8y ago

Similar but unrelated topic... Does anyone have a good C style guide they like?

dajonker 8y ago

Perhaps it's a matter of rule-based versus principle-based view of the world. Generally speaking, in a principle-based system, there is room for different interpretations of the stated principles. Edge cases usually don't require special mention. In a rule-based system, rules are strictly defined and enforced with little room for interpretation. Anything not explicitly mentioned is a grey area and edge cases often end up amending the rules so they are covered as well. Sounds to me like the author prefers the rule-based approach and/or is assuming these rules are strictly enforced.

hellofunk 8y ago

Fun piece of trivia for you: The C++ Core Guidelines referenced in this article are longer than the language spec for Go.

ryandrake 8y ago

I’m not a huge fan of the GSG, at least the C++ one. It seems to be less about style and more about forbidding scary language features. A language feature only becomes a “foot gun” in the hands of a careless developer. Maybe it’s too idealistic but IMO it’s better to fix the “careless developer” problem at the interview stage, not in a style guideline. Hire developers who voluntarily follow best practices and use good judgment...don’t try to mandate it with an arbitrary list of forbidden practices.

olliej 8y ago

There are some super boneheaded comments in here. * "Don't use forward declarations unless necessary" with the counter example of why it's bad, being a case we're they're necessary? * The complaint against "inline" is nonsense. The only thing inline impacts is function linkage. Literally nothing else. If the compiler thinks a function is profitable to inline, it will inline it. If it does not think inlining is profitable it won't, the fact that you say "inline" means nothing. * Complaining about complex globals initialisers: they directly impact library load time, they do cause breakages on ordering. Yes you could structure your code to avoid ordering issues, but then every developer needs to work for all time to avoid breaking it. Or you could just not use them, and it's not a problem. * Restricting copies: It should be super easy to tell at a glance if code is going to be needlessly inefficient. Easiest way to ensure that is to make your code fail to compile. * Exceptions: they add significantly to code size and launch time. The failure modes are hard to reason about. The standard idiom everyone is move to is option<> or result<> types. It means you have to handle errors, or be explicit in ignoring them. * Template meta programming is extremely slow to compile, and very difficult for people to understand. constexpr is easier to read and understand, more efficient to compile, and can also be reused for non-constant cases. * Boost: if you're using boost then you're requiring all your developers to install boost. Then you need to also ensure that they're all using the same version, and then people can end up requiring multiple versions. Again, super large company, with many many engineers and many projects mean the chances of multiple projects depending on different versions of libraries will cause misery. * C++11 - Cool, it's 2018, guess what: many machines are running versions of an OS that has a 2018 edition of the C++ standard library. If you compile targeting newer versions of the standard library then you can't ship to those systems, unless you include a copy of the standard library in your binary. Then you have to hope your copy of the standard library doesn't conflict with any loaded by the system's own libraries.

larl 8y ago
Or even: C++11 - Cool, it's 2018, guess what: Migrating a codebase as large as Google's from C++11 to C++1x is a bunch of work. And a lot of the C++1x goodies are library additions, available in Abseil <https://github.com/abseil/abseil-cpp> https://github.com/abseil/abseil-cpp>.

DannyBee 8y ago

It's very interesting to me that the author writes a critique of a style guide, but doesn't seem to have tried to understand the perspective of those who created it or what their goals were (they basically give this lip service) They don't really try and understand the rationale in detail and then argue that that rationale would be better served through a different set of features, they mostly just say "hey I think Google is wrong and these features are pretty cool"

Carpetsmoker 8y ago

Laughs in gofmt

bausshf 8y ago

"Unnamed Namespaces and Static Variables GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants?" A constant is not a static variable.

knorker 8y ago

> GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants? Uhm… what? 1) I would challenge the author to produce any useful use of anonymous namespaces in header files that doesn't violate ODR. I'd also challenge them to not break ODR with constants in header files in the way they seem to mean. I should add: without UB, plz. 2) How else do we declare constants? Uhm, how about: extern const int foo; or just "inline constexpr int foo = 1" in header but non-static and not in anonymous namespace? > Globals like this are not problematic and very useful: [example of nontrivially destructible global] It doesn't look dangerous, but allowing it does not scale to a billion lines of code. Code easier to reason about is less buggy code. In general though: Author spends 1 SWE-year per year on coding, Google spends what, 100 SWE-millenia per year? It's optimizing for that use case.

alexeiz 8y ago

The thing that bothers me about GSG is that it seems to be enforced across Google no matter what team you're in or what project you work on. Think about it: somebody who thinks that he knows better than you decided how you and other 10000s developers at Google should use C++. I think such approach is incredibly inflexible and shortsighted. Even if that somebody is a well-renowned C++ expert I would still doubt he can come up with the C++ style guide applicable to the whole Google. If I lead a project at Google I don't want someone unrelated to my project or some committee higher up to make decisions for my team. I want my team to decide how to use C++ (or any other language or technology for that matter) based on the project needs and the team developer skills. One coding style for everyone reduces the skill level of everyone to the lowest common denominator. Even if you have better skilled developers around you, if they can't use their skills professionally, they either quit or lose their skills. Soon you'll be surrounded by developers with only poor skills who you can learn nothing from. I remember a story told by Sean Parent (if you don't know him, look him up) about the time he worked at Google and he was told that his code is hard to understand because it uses concepts and ideas nobody at Google is familiar with (because they are likely not part of the GSG). I find that story quite instructive.