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.
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.
> 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."
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.
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:
* "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.
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.
> 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)
> 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.
> 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'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.
* "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.
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>.
>* 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.
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.
> 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.
> 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?
> 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.
[+] [-] jmgao|7 years ago|reply
> Although GSG does not forbid them categoricaly, it says to “Avoid using forward declarations where possible”. However forward declarations are useful in contexts like the following: ...
Yes, those are the cases where it isn't possible to avoid forward declarations, so it's fine to use them?
> While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve. Globals like this are not problematic and very useful:
No, globals like that are very problematic, so much so that there is a clang warning specifically to detect these (-Wexit-time-destructors). exit runs global destructors, so if one thread calls exit while some other thread is still doing stuff, it might destroy an object that's in use, leading to explosions. Also, if you're exiting, why are you paying to deallocate memory when it's all going to be blown away anyway?
> The lynchpin of C++ are value-types. Such types should be copyable and moveable and the language automatically generates the necessary constructors and operators by default. “a copyable class should explicitly declare the copy operations, a move-only class should explicitly declare the move operations” – this goes against the language philosophy.
It is very non-obvious which constructors will get automatically generated, because it depends on the types of the members. `Foo(Foo&& move) = default;` makes it immediately obvious.
[+] [-] Chabs|7 years ago|reply
> in other words, the reader is expected to understand the semantics of an unknown function without consulting the declaration (or documentation).
The idea that the author considers making code as self-documenting as possible at the call site a conceptual mistake is just weird to me.
[+] [-] gok|7 years ago|reply
Indeed, they're terrible. What I don't understand is why basically every Google-authored project has loads of these (protobuf, gflags, TensorFlow...)
[+] [-] sytelus|7 years ago|reply
> 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.
[+] [-] AnimalMuppet|7 years ago|reply
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.
[+] [-] eyakubovich|7 years ago|reply
> 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?
[+] [-] taspeotis|7 years ago|reply
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."
[+] [-] anyfoo|7 years ago|reply
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.
[+] [-] gumby|7 years ago|reply
[+] [-] liftbigweights|7 years ago|reply
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.
[+] [-] stdplaceholder|7 years ago|reply
* "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:
This doesn't even need a using-directive. 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|7 years ago|reply
```
int kDefaultValue = 42;
size_type kExpectedSize = 10;
v.resize(kExpectedSize, kDefaultValue);
```
[+] [-] cwyers|7 years ago|reply
[+] [-] jghn|7 years ago|reply
People need to remember that just because Google does something one way does not imply it is correct nor that it is incorrect
[+] [-] OnlyRepliesToBS|7 years ago|reply
[deleted]
[+] [-] wutbrodo|7 years ago|reply
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|7 years ago|reply
> 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|7 years ago|reply
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)
[+] [-] jcelerier|7 years ago|reply
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...
[+] [-] haberman|7 years ago|reply
> 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.
[+] [-] cma|7 years ago|reply
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.
[+] [-] nitwit005|7 years ago|reply
[+] [-] hendzen|7 years ago|reply
[+] [-] kazinator|7 years ago|reply
[+] [-] jchw|7 years ago|reply
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|7 years ago|reply
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|7 years ago|reply
* "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|7 years ago|reply
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>.
[+] [-] iokanuon|7 years ago|reply
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
[+] [-] jupp0r|7 years ago|reply
"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|7 years ago|reply
[+] [-] eyakubovich|7 years ago|reply
[+] [-] DannyBee|7 years ago|reply
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|7 years ago|reply
[+] [-] alexeiz|7 years ago|reply
[+] [-] ryandrake|7 years ago|reply
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|7 years ago|reply
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|7 years ago|reply
[+] [-] hellofunk|7 years ago|reply
[+] [-] knorker|7 years ago|reply
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|7 years ago|reply
> Unlike C++ Core Guidelines that try to explain how to use the language effectively, GSG is about forbidding the use of certain features.
They aren't meant to accomplish the same thing. The GSG's goal is to standardize both style and feature set across the mono repo so that thousands of engineers can effectively contribute.
> We should not bifurcate developers into two camps as the reality exists in a continuum between these two extremes.
It's a style guide, not an unwavering book of law. Library writers use some features that aren't necessary in non-library code. In fact, some language features are written specifically to the benefit of the implementers.
> GSG prefers #ifndef/#define idiom over the simpler #pragma once. Yes, #pragma once is non-standard but widely supported.
When the difference is a 2 lines of code that you probably have tooling to generate anyways, why not default to the thing that's standard?
> Yes, both of these forward declarations are possible to avoid by type-punning through a void* but it is not a good pattern.
The point of "Avoid using fwd-declarations where possible" is exactly that: reason about the code and whether the declaration is necessary. It's not "dogmatically avoid forward declarations". I'm sure no-one at Google is punning through void* for those examples.
> Marking the function “inline” lets the compiler make the decision. Not marking it inline is a sure way to prevent inlining, unless Link Time Optimizations are turned on.
That's simply not true. The compiler can inline a function if it can infer that it doesn't alter the program's behavior. Generally, tools make good decisions. Override them when you've measured that something else is better.
> Library code should always be placed in a namespace. Top level/application code has questionable value being placed in a namespace.
It's easier to be diligent about placing everything in a namespace because application code doesn't always remain so. Especially not in a gigantic mono repo like Google's.
> GSG prohibits the use of static definitions and annonymous namespaces in header files. How else do we declare constants?
I mean, you declare your constants in the header and define them in the implementation file. First example I can find in Chromium: https://cs.chromium.org/chromium/src/ios/chrome/browser/pref...
> The rule basically says that global/static variables with non-trivial constructors and destructors are not allowed. While it’s true that initialization/destruction order of globals between translation units is not defined and can pose problems, this rule is overly prohibitve.
This rule isn't overly prohibitive in the context of large applications that require clean startups/shutdowns and contain objects that are sensitive to construction/destruction order.
> “Do not define implicit conversions”. I would urge the reader to consider what life would be like if std::string(const char) constructor was marked explicit (especially in the absense of user defined literals, which GSG also outlaws). Nuff said.
The issue with implicit conversions is that they're not explicit to callers. Honestly, having to wrap a few const char* into std::string() calls wouldn't be as bad as you seem to think it would be. Explicit code is easier to read and reason about.
> “a copyable class should explicitly declare the copy operations, a move-only class should explicitly declare the move operations” – this goes against the language philosophy.
Again, this is about being explicit, and using safe defaults. It's not prohibited to make things copyable and movable but defaulting to the most restrictive set up and adding things as needed ensures that developers thought about the implications of those properties.
> In this pattern the private base cannot be replaced by a data member because the base is constructed before the members.
I've never seen code with private inheritance that made sense and couldn't be refactored to something clearer.
> Operator Overloading
See my point about implicit conversion above because it's the same thing here. Operator Overloading tends to obfuscate code.
> I think this rule mixes together two ideas for no good reason. C++ has a clear way to denote mutability – the const qualifier. Using pointer vs reference to signal mutability goes against the language.
The const qualifier isn't visible from the call site. This rule makes it so that you can reason about what will happen to the values you're passing to a function based on whether you pass them by pointer, or by reference/value. It also prevents a library from changing an argument from const T& to T&, introduce mutations, and break callers as a result.
> Exceptions
Google's code is just setup to std::terminate instead of throwing. It's a minor performance gain AFAIK but it also avoids littering the code with try {} catch {} blocks. It forces developers to handle errors instead of propagating them whenever possible too.
> “Use lambda expressions where appropriate.” – probably a cheap shot, but what is the alternative? – use them where it’s not appropriate?
This point seems counter-productive considering the author claims the guide is written in too much of a prohibitive fashion.
> Avoid complicated template programming.
TMP is hard, increases compilation times, and is difficult to reason about. The Guide recognizes its usefulness but suggests avoiding it where possible. This is similar to the library writer vs application writers argument: not everyone needs it, avoid it if possible.
All in all I think the author just doesn't have the same requirements, constraints, and sheer amount of developers/lines of code Google has. Nothing is forcing them to use the Guide. In fact, it's public but it was written for Google, by Google. It works for Google, and it's a great tool in that kind of organization.
Disclaimer: I work on Chrome, which has its own guide derived from the GSG.