Google C++ Style Guide Is No Good | Hacker News Reader

Google C++ Style Guide Is No Good

eyakubovich.github.io

77 pointseyakubovich104 comments

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.

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.

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

In Chromium they are so widespread that makes the rule showing that complex code cannot avoid them and defeating the rule.

Same sentiment here. Many of GSG constraints are good and others are purely because of legacy code at Google. So don't take GSG as religion and implement it to the letter ask why this constraint is proposed and does it apply to my code?

> GSG forbids exceptions on the ground of “Google’s existing code is not exception-tolerant”.

Unless you are doing embedded software, you should use exceptions. I have tried this "modern" trend of returning instead of raising a few times and I'm not convinced at all its less bloated or less error prone way of doing things.

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

It generates constructors and operators by default. They may not be the ones you want, though. They will satisfy the compiler, but they may do the wrong thing. If the default thing is the right thing, then yes, "= default" makes it obvious. If you don't say, then it isn't obvious whether the default is right, or the default is wrong but you missed it.

> Yes, those are the cases where it isn't possible to avoid forward declarations, so it's fine to use them? But my point was that it _is_ possible to avoid them by type-punning via void*. Now, one can argue that it's unreasonably bad way to avoid them. But will a reviewer following this guide make the same assumption?

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

Or you can structure your code in a way that all your threads join prior to main exiting. Never call exit() or _exit() or even pthread_exit(). I think it's much cleaner. Not to mention that not every program is multi-threaded.

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

Agreed but can you imagine what a simple

struct Point { int x, y; };

become if we were to make everything explicit?

> Yes, those are the cases where it isn't possible to avoid forward declarations, so it's fine to use them?

The author explicitly mentions how it is possible to avoid forward declarations in those cases: "both of these forward declarations are possible to avoid by type-punning through a void* but it is not a good pattern."

By definition, if avoiding something isn't possible, then there cannot be a "how to avoid".

Why even mention a non-starter choice, like using type punning through a void pointer, as an alternative to a forward declaration.

If a construct is "not possible to avoid" that generally means "not possible to avoid without changing the structure of code and run-time data (let alone changing them to something unsafe)".

Gratuitous forward declarations of C++ classes are "bad" for various reasons. Such as: people sometimes write them just to shut up the compiler, instead of including the right header to provide that declaration. "I'm just using a pointer to this; what's the harm."

It makes perfect sense for a coding standard to advise against this.

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.

I think the author knows full well via his reference to the guide being adopted elsewhere: he's writing for people who are not at google but adopt big g's standards as they assume they are good. Well they are, but just for google itself (as the guide admits -- it explicitly forbids exceptions not because they are a bad idea but because of so much legacy code).

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

Totally! Consistency in even small things, like "try to order the names of your includes/methods/members alphabetically" take a good significant load of your brain, especially if you are working on a completely different part of the project that you are not fully familiar with.

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

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

```

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

The guide should then specify some common instances where arguments may be flipped.

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.

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

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

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.

it is also less bugprone in the real world. There are cases where it doesn't work, but they amount to stupid things that nobody does outside of contrived examples.

I switched our codebase after finding several variations of #ifdef message_h #define massage_h That potential bug existed in our code base for years (I checked history)

Fwiw Google solves this problem by having tooling generate the file template.

In other words, my vimrc pulls in a global vimrc that, when I open a new .cc file, it starts as a stub that has the correct ifdefs and some common imports (flags iirc). This, combined with a lint/presubmit that prevents submission of files missing the correct defs makes this a nonissue in practice.

The worst thing that happens due to this "potential bug" is that someone introduces a double inclusion of that header, which fails to be suppressed and the build blows up in their face due to duplicate definitions.

The error could be diagnosed by the compiler---submit a patch to gcc!

That is to say, if the following pattern occurs around the content of the file:

   #ifndef <whatever0>
   #define <whatever1>

   #endif

the compiler can validate that <whatever0> and <whatever1> are the same symbol, and emit a warning if they aren't.

Note that GCC already looks for this pattern and optimizes it! That has been implemented for ages; probably twenty years if not more. See here:

https://gcc.gnu.org/onlinedocs/cpp/Once-Only-Headers.html

That optimization must have a check in place that it's the same symbol in both places. And so that piece of code which checks can probably be augmented to emit a warning fairly easily.

A sophisticated version of the warning would only kick in if the symbols appear to be similar (e.g. close by Levenschtein distance) so that one of them is plausibly a typo for the other.

Another solution is to have a commit hook which looks for such a problem: it enforces the include guard on headers, and checks that the symbols match.

clang actually does have the warning you want gcc to have - this is how I found our instances of it.

What none of the above can do is two different projects have MESSAGE_H headers that are different can fully compatible to include both. #pragma once handles this correctly.

The clash between two different MESSAGE_H can be rendered vanishingly unlikely by having a convention of adding some random digits.

I adopted the following pattern some twenty years ago of affixing a random 32 bit number in hex:

   #ifndef MESSAGE_H_3DF0_9A73
   #define MESSAGE_H_3DF0_9A73

   #endif

If only the filesystem would assign some sort of unique, 32- to 64-bit identifier to files that a compiler could use to distinguish one header from another. We could call them inode numbers. Hmm. No, I'm sure no one has ever thought of the problem of identifying unique files before.[0][1]

I guess instead we must embed GUIDs in every file to keep track of uniqueness.

[0]: https://stackoverflow.com/a/13172784/218830

[1]: https://stackoverflow.com/a/1866547/218830

Copies of a file have different inode numbers.

I can agree with this statement of fact. I'm missing the part where it's relevant to the conversation or informs some conclusion.

It's relevant if I have a requirement that if a file happens to be duplicated, then the once-include mechanism should treat those copies as one object.

This could arise if the code is originally on a system where some header files are symlinks to a common file. These symlinks happen to break (due to the way the code is archived, or on some system that doesn't have symlinks or whatever); they turn into separate copies.

Exclusion that is keyed to a symbol that is encoded in the file has no problem with this; filesystem-id-based excusion doesn't handle the case.

> It's relevant if I have a requirement that if a file happens to be duplicated, then the once-include mechanism should treat those copies as one object.

I don't believe that is a reasonable requirement.

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

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

> 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 with the list of all supported compilers.

"#pragma once" used to cause GCC to launch a game of Tetris instead of compiling your code. Not super relevant today, but a fun fact.

IIRC, it cycled through NetHack, Rogue, and Tower of Hanoi.

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

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

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 extra care that will have to go into everything to avoid really bad problems.

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

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.

Something like a std::vector constructor/assignment operator can get rather messy if the value type constructor can throw.

Unwinding can be perilous across library boundaries, this is IMO its greatest weakness.

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.

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.

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

At the cost of predictable performance though. With current implementations, the exceptional path will be several magnitudes (!) slower for cheap functions.

Exceptions impose indirect costs other than just code size. For example, when std::vector reallocates, it copies instead of moves its elements, in case the move constructor throws and leaves the vector in an inconsistent state.

Also... there is a proposal to fix the overhead of the code that current compilers generate for exception based code.

P0709 R0 – Zero-overhead deterministic exceptions

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p070...

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.

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

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

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.

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

This attitude leads to companies to maintain COBOL codebases on the obscure mainframe for decades.

Why is that bad?

If the system works, and is fast and efficient enough for the task (which is clearly is), and supports the needed features (there was an article on the things you could do in COBOL a while back that went over this), then what is the benefit?

Sure maintaining something is not as "fun" as writing a new thing, but that doesn't make it better.

Anyway, the "obscure mainframes" isn't true - yes there are cases where the easy path was simulate to emulate a PDP on modern hardware - but there are, for example, COBOL implementations the target .net, in addition to regular unix and windows targeting compilers.

Edit: and I forgot my original reason for replying: the use case for mainframe software is drastically different from consumers.

The big scary mainframe software is generally designed to be essentially a few users, running on a restricted set of homogenous hardware and system software. They are generally specialized so that's all they have to do.

Consumer software is not as forgiving - generally a user expects software they bought 20 years to still work, and likewise will assume that their 10 year old machine should still be able to run new software "because it still works". Look at the commentary on Apple deprecating 32bit software. Or the complaints about Windows N breaking legacy software, while also laughing about the things MS does to keep old things running.

Problem is that you are locking yourself. Even if this work for decades it can destroy your company. At some point, you will not be able to respond to change. More agile, faster-moving competitors will eat your lunch.

I am not suggesting rewriting to use new shiny things. If the platform is deprecated (32bit, mainframes, windows98, C89, etc) you migrate your code when it is still possible. Customers can still use an older version of your software is they are stuck in the past but you are not compromising future of your product.

Yes,it motivates companies not to do expensive, unnecessary, high-risk complete replacements.

This may be less fun for developers, but it's not usually a bad decision.

High-risk complete replacements happen because some risk-averse developers/management resisted change for decades and now the company cannot gradually modernize.

https://en.wikipedia.org/wiki/Phoenix_pay_system

right?

All engineers love rewriting everything, because that's human nature: "I can totally do this better than the last person".

In fact, Google has been actively trying to migrate their code base to C++17 compatible status for several years. It just took much more time than the initial estimation.

>* The complaint against "inline" is nonsense. The only thing inline impacts is function linkage. Literally nothing else.

That's not actually true. At least in clang it lowers the threshold at which a function will become inlined. You can see that happening in this wonderful demonstration by Jason Turner: https://youtu.be/GldFtXZkgYo

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.

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.

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.

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.

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"

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

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.

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.

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

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.

Is it trivial to implement?

The detailed semantics of #pragma once is murky. If #pragma once appears in a file, will that preclude an identical copy of the file from being included? Do we go by content, or just some filesystem identifier like device and inode number? Or the absolute path? Will #pragma once preclude that same file from being included again through a symbolic or hard link? Is the answer the same for all compilers?

The semantics of the #ifndef/#define/#endif trick is crystal clear; we can predict what it will do in any given situation. The identity of the file, for the purpose of suppressing duplicates, is tied to a made-up symbol which the program explicitly specifies, and that's that. It may go wrong (e.g. due to clashes on the identifier between unrelated files), but in a way that is understandable.

#pragma once seems like a bad trade-off for the sake of saving two lines of preprocessing.

> Is it trivial to implement?

Yes.

https://github.com/llvm-mirror/clang/blob/799b6c6d7/lib/Lex/...

https://github.com/llvm-mirror/clang/blob/799b6c6d7/include/...

I agree that it's trivial to invoke a member function called MarkFileIncludeOnce on some object in response to encountering a #pragma once.

I don't think there is any good way to specify how this should behave for all the corner cases; but a given specification isn't hard to implement.

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.

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.

> You could say that there's no reason we're still using a stupid preprocessor hack of textual inclusion to import interfaces.

You could say that. I didn't. Bolting on some sort of module system is just a totally different scope of enhancement than my modest proposal.

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

That's just not true. It doesn't need to be robust against byzantine source trees and/or build systems to be defined in the C standard or to be useful. The standard can leave the concept of "the same file" implementation-defined, as it does many other concepts.

On Unix system C implementations, it is sufficient to use stat() and compare st_ino and st_dev against previously observed values for a given compilation unit. You do not need to checksum files. You especially do not need to write the specific behavior of checksumming header files into the C standard.

> It doesn't need to be robust against byzantine source trees

A locally developed hack, or even a compiler extension, doesn't have to be; an ISO-standard feature should be well specified.

If something is specified in such a way that it is less robust than the #ifndef trick, then any programmer worth their salt will use the #ifndef trick.

An acceptable pragma-once would look like this:

  #pragma once 9DF9-C3D9-BDF0

Basically it should take an argument string which specifies an ID for the file, intended to be unique.

> ISO-standard feature should be well specified.

That claim doesn't match up with the C standard I've read. I.e., this is an "isolated demand for rigor." Are we looking at the same document? Quite a lot is underspecified or implementation defined to accommodate differences in architectures and systems.

The 2018 C standard doesn't specify whether NULL is a pointer or integer; what a null pointer's representation is; nor the representation of negative (signed) integers. The C standard defines some loose requirements around observable behavior and leaves the specific details to the implementation.

> Basically it should take an argument string which specifies an ID for the file, intended to be unique.

Or the standard could leave it up to the implementation to identify file uniqueness without this additional, incompatible argument. Like I said before, all you have to do for Unix implementations to be as robust as the stupid ifndef trick is to check st_ino and st_dev.

It's important to recognize that implementations and implementation details are distinct from the standard.

Also note that the ifndef hack is non-robust in its own way — false positive exclusions due to accidental identifier conflicts. #pragma once does not have this problem.

The #ifndef trick is highly portable and has clear semantics that is under the program's control. Nobody needs a platform-specific, underspecified alternative that saves two lines of code compared to the robust, portable solution. If I am paying with nonportability, I want "bang for the buck": like faster or smaller code on the target, or detailed control of its hardware or host platform features. I don't need unspecified build machine behavior so much. I am not programming the build machine.

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

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

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

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.

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.

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

I don't accept that any reasonable sized shop needs to subset C++ or insanity will prevail. Any particular slice of code needs to subset the language. You may have some perf sensitive code that can't afford virtual function overhead. Or some code that uses dynamically linked libraries for which RTTI may not work right (so you avoid RTTI there). Perhaps strict determinism is required and you can't use exceptions in this part of the code base.

However this is vastly different than saying that we don't want any of these features anywhere because they might not be appropriate under some conditions. I advocate education so developers can make decisions when to use and not to use certain features (same as selecting the right algorithm -- we don't ban binary search b/c it's harder to understand than linear search).

Same with the "we have tons of programmers" argument -- let's use the lowest common denominator (everything else is "fancy"/"clever") to make sure every line of code is understandable by anyone. Encourage people to learn, not everyone else to dumb down.

There's a very big C inherited footgun you can avoid with signed sizes: implicit conversions to unsigned on comparison.

GCC has had -Wsign-compare:

       -Wsign-compare
           Warn when a comparison between signed and unsigned values could
           produce an incorrect result when the signed value is converted to
           unsigned.  In C++, this warning is also enabled by -Wall.  In C, it
           is also enabled by -Wextra.

Since 1996. https://github.com/gcc-mirror/gcc/commit/de9554eb8ae74764555...

(And prior to that explicit option, it was just enabled as part of -Wextra/-W, from 1995: https://github.com/gcc-mirror/gcc/commit/7030c69607d547b3227... . For comparison, Google was founded in 1996.)

I buy that some of the integer promotion rules really suck, and Google's founders learned C prior to 1996. But the compiler has been able to warn us about this particular footgun for a long time.

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

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?

constexpr variables must have a literal type, which requires a trivial destructor.

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

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.

The Python one is particularly offensive.

Laughs in gofmt

Does it work for them?

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.

Google's

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.

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

Reply on news.ycombinator.com