
Google C++ Style Guide Is No Good - eyakubovich
https://eyakubovich.github.io/2018-11-27-google-cpp-style-guide-is-no-good/
======
jmgao
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.

~~~
gok
> 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
In Chromium they are so widespread that makes the rule showing that complex
code cannot avoid them and defeating the rule.

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

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

------
stdplaceholder
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
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
...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
The guide should then specify some common instances where arguments may be
flipped.

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

~~~
jghn
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

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

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

~~~
bluGill
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)

~~~
kazinator
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](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.

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

~~~
kazinator
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

~~~
loeg
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](https://stackoverflow.com/a/13172784/218830)

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

~~~
kazinator
Copies of a file have different inode numbers.

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

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

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

------
haberman
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...](https://abseil.io/about/compatibility#what-users-must-and-must-not-do)

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

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

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

~~~
hendzen
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
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
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
At the cost of predictable performance though. With current implementations,
the exceptional path will be several magnitudes (!) slower for cheap
functions.

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

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

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

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

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

~~~
Chyzwar
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](https://en.wikipedia.org/wiki/Phoenix_pay_system)

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

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

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

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

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

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

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

~~~
kazinator
> _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.

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

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

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

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

------
anthonyvd
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...](https://cs.chromium.org/chromium/src/ios/chrome/browser/pref_names.h?rcl=82d8ee1e070100260963c8d76f23dcd2884099c9&l=11)

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

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

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

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

~~~
loeg
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...](https://github.com/gcc-
mirror/gcc/commit/de9554eb8ae74764555178fc8bdf04cad9f3aa82)

(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...](https://github.com/gcc-
mirror/gcc/commit/7030c69607d547b3227ad0aa3842bf7864c690f3) . 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.

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

------
rocqua
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
constexpr variables must have a literal type, which requires a trivial
destructor.

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

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

------
rbanffy
The Python one is particularly offensive.

------
Carpetsmoker
_Laughs in gofmt_

------
ramenmeal
Does it work for them?

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

------
sigacts
Google's

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

