Discussion: Standardizing C++ Style

[davidzchen]

This is a follow-up to the discussion in: #138

As @mikedanese mentioned, we should standardize on a .clang-format. Currently, the C++ style used by Jsonnet is not codified, and it is a good idea to standardize on a style guide and .clang-format.

To recap the discussion so far:

I am in favor of adopting the Google C++ Style Guide to be consistent with most other Google open source projects and to encourage more adoption internally, though @sparkprime brings up the reasons against it discussed here and prefers codifying the existing style.

Another style guide that we may consider is the LLVM Style Guide.

[davidzchen]

To add some more comments about Sergey Zubkov's rant: we should also note that the Google Style Guide is constantly being revised, and as Sergey himself pointed out, with the recent revisions, many of the "deal breakers" he listed no longer applies:

9/21/14 update: Public style guide was updated from 3.274 to 4.45, losing two of the points I strongly disagreed with.

9/26/15 update: Public style guide was updated from 4.45 to 4.177, losing four more points, including the std::forward ban, the I/O streams ban and the operator overloading ban - although the pros for operator overloading still fail to mention most of the reasons it is required.

1/17/16 update: Public style guide was updated to an unversioned "current" on 1/6/16, relaxing the default parameter prohibition but suddenly banning ref-qualifiers as "obscure".

Currently, the key areas where the conventions used by Jsonnet differ from the Google Style Guide that are not formatting-related are:

1. Use of exceptions
2. Use of mutable references in function arguments.

Feel free to correct me if there are additional fundamental differences.

I am ambivalent about (1) and would be fine with keeping exceptions since removing them would be non-trivial. For (2), I think the Google style guide has merit and that is better to make it clear whether a function argument is mutable. This is the same reason why Microsoft implemented SAL annotations.

[sparkprime]

When you have exceptions you often need RAII, which used to be used to assist the GC but isn't any more I think (except in STL containers and such). I'm probably using unique_ptr or something like that in there somewhere as well.

The "no exceptions" rule in Google is born out of having too much old code and doesn't apply here. Removing exceptions from the code would lead to a lot of boilerplate error propagation, which IMO outweighs the supposed benefits.

At any rate I think we should stick to basic visual style like whitespace and naming conventions and not opinionated stuff like "exceptions are bad mmk"

I don't feel that strongly about mutable references in function arguments, although I don't buy the argument in favor of them at all. The function signature is always immediately available (unlike some languages), and in the case of foo(x) I have to know whether x is a pointer (unless we also use hungarian notation like p_x), so it's not apparent just from looking at the call with no context that it cannot write to a param. In general the effect of a function can be arbitrary so this very thin convention doesn't buy me much. I've also was never surprised by a non-const & despite writing probably about 200kloc of C++ over the last 10 years. SAL annotations are a real "effects system" and much more powerful. They're probably too bureaucratic for our purposes. If we were writing / validating drivers it'd be different, and MS also has some cool tech for that.

When it comes to function / variable names, I use:

• ALL_CAPS for top-level constants / macros (all top-level things should be constant)
• camelCase for member functions & variables
• CamelCase for class names, enumerations, typedefs, etc
• under_score for symbols that aren't scoped in a class, i.e. global functions, stack variables
• single letter capitals for template params

Basically that's a classic K&R-like C style but extended to C++ in a java-like fashion, to distinguish between OO and procedural styles. So there's a lot of high-level information encoded in the names (but not hungarian notation). I've never liked the "everything is an object" concept that underpins Java, so I use procedural style unless objects are a natural fit.

Whitespace:

• { on the next line for functions but nothing else (K&R style)
• 4 spaces for indentation (no tabs, ever)
• 100 char line max
• no indentation for namespaces (your recent change)
• comments on the same line separated by 2 spaces

Misc:

• use (void) if the function takes no params (throwback to C I know, but I find it more readable, and we have some C code here as well anyway)
• prefer (void) unused_param; instead of just dropping the name, because the name is usually valuable information

[davidzchen]

The latest version of the style guide does not say anything about forbidding RAII. I only see one mention of RAII and it is in a line that states that, as you said, exception safety requires RAII. In fact, there is definitely commonly-used Google C++ code that makes use of RAII, such as Mutex.

As mentioned above, I agree that getting rid of exceptions in Jsonnet would not be worthwhile. FWIW, the reason why the the exception ban itself exists was due to actual bugs in the C++ compiler in the past. That said, in general, I am not convinced that exceptions in a non-GC language such as C++ is a good idea in general. There are plenty of reasons why exceptions in C++ are not a good idea and why new static-typed languages such as Go and Rust have decided against using exceptions.

One thing I should clarify is that the Google style guide's rule against using mutable references in function arguments is to make it clear at the call site whether an argument is mutable or not. This is why the style guide points out that the problem with mutable references is that "they have value syntax but pointer semantics." There are some proposed solutions that I can point you to offline for allowing mutable references but at the same time still making the mutability explicit at the call site.

In any case, these are my suggestions and my explanation for why I prefer the Google style guide. One of the main points I am trying to make is that we should not be so quick to dismiss a particular style as "terrible," especially since the style guide in its current form is still the product of a decade's worth of experience and battle-testing. Having used the style guide on a daily basis in my job, I can definitely appreciate the merits and reasoning behind the style guide. Finally, given that the Google C++ style guide is the one used for projects such as MapReduce, BigTable, Colossus, TensorFlow, as well as many other parts of the most advanced search stack in the world, I think it is safe to say that it is not such a terrible style guide after all. :)

Of course, as the creator of Jsonnet, you certainly have the final say, and I respect and support the decision that you make.

I've opened separate bugs to write the style guide doc (#142) and add a .clang-format (#143).