Live data from Hacker News

Google C++ Style Guide Is No Good

eyakubovich.github.io

21–30 of 108 posts

Re: Google C++ Style Guide Is No Good

#21
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.

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).

Re: Google C++ Style Guide Is No Good

#22
> 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.

Re: Google C++ Style Guide Is No Good

#23
post #5
post #3

> 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.

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.

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.)

Re: Google C++ Style Guide Is No Good

#24
post #11
post #5

Earlier quoted context omitted.

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.

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 e…

> 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.

Re: Google C++ Style Guide Is No Good

#25
post #5

Earlier quoted context omitted.

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.

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]

Re: Google C++ Style Guide Is No Good

#27
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...

> 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.

Re: Google C++ Style Guide Is No Good

#28
post #5

Earlier quoted context omitted.

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.

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.)

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.

Re: Google C++ Style Guide Is No Good

#29
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.

Re: Google C++ Style Guide Is No Good

#30
post #3

> 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.

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.
Post reply on HN