-
Notifications
You must be signed in to change notification settings - Fork 49
Charm Runtime Style Guide
In terms of indentation GeoSoft is basically indistinguishable from the Google Style guide. There are a bunch of things specific to google in their guide that make using that as base spec overly distracting. Additionally, the google style for member variables uses underscores instead of the capitalization scheme. The existing Charm runtime code base is much more heavily camel-case, so this would be a rather extreme change. The existing charm base a weird mix of capitalization with quasi arbitrary use of underscores in some places and some variables with both. I advocate a more standard capitalization approach going forward and don't see much benefit to rewriting a lot of old code solely for the purpose of style compliance.
NOTE: the GNU indent tool does not fully understand C++ and should not be expected to work reliably as a whitespace policy correction tool in the general case. It should work correctly on C code and on relatively simple C++ code.
I recommend that we adopt the GeoSoft style, referenced below, with the noted changes. The google guide is similar in many respects, but has more exceptions for us.
I am currently examining the cpplint.py tool to see if it can be made suitable for our purposes. It should be noted that cpplint does not fix anything, it simply reports noncompliance. Parsing C++ is so horrible that google didn't think it was worth their time to implement an autocorrecter. It is also a C++ specific tool and ignores .c files unless hacked to do otherwise. In point of fact, it also ignores .C files unless hacked to do otherwise, which makes the vanilla google version almost useless for our purposes. As a sampling of use, the version hacked to accept .C files finds 1159 formatting errors in the 2531 lines of ck.C. Most of the non-indent complaints (after fixing those using their emacs mode) are about missing whitespace around operators and after "," and lines longer than 80 columns. None of the stuff it complains about represents a superior expression of code readability, so their linter seems to be pretty close to what we would want in a linter.
The google emacs mode basically just enforces their whitespace guidelines. Since their indent guidelines are whitespace conserving in a way that we like, this is quite useful. Running an automatic emacs reindent on ck.C reduces the complaints of cpplint from 1159 to 875.
Someone who uses vim should check out the vim version.
Use #if instead of #ifdef for macros. The preference is for macro values to be explicitly set to 1 or 0, rather than using #if defined(CMK_FOO).
Rule 70: Should be reversed. You should use NULL, not 0.
NULL is part of the standard C library, but Stroustrup wants us to believe it was made obsolete in C++. However, distinguishing the initial value of a pointer using NULL is a powerful way to have the code be self documenting in a properly type aware fashion. It also allows GCC and other compilers to raise some useful warnings. Initializing a pointer with 0 really gets things off on the wrong foot by making the user think too much about physical address space, which can lead to highly error prone manual pointer mathematics. Dr Stroustrup's opinions aside, NULL retains its utility as syntactical sugar to improve code readability. This is especially relevant for Converse/Charm++ due to the fact that we frequently cross between the C and C++ modes at the machine layer boundary, so pretenses about escaping from C semantics are dangerous to our readability.
Google already recommends NULL, so it is superior to GeoSoft in that respect.
Google doesn't enumerate their rules, so revisions need to be referential against their anchors.
File Names: Google standardizes on .cc, we standardized on .C. Changing to their way is a sort of pointless conformity in that any superiority that .cc might have over .C may be too marginal to be worth the pain of conversion. Phil argues that we should just do this massive rename anyway due to .C being an antiquated mid-90s convention and capitalized filename extensions not working correctly on non unix filesystems. I'm not fully persuaded by that argument, so its worth some debate.
Smart Pointers Google prefers them and makes a number of statements about variations on these and not using variable length arrays. Their approach requires the use of Boost and is a significant departure from our historical practices. Their arguments are worth reviewing, but we can't support a wholesale switch to their point of view without a lot more impact analysis. Boost is nice, but introduces another API versioning consideration along with another way for templates to massively explode your compiler error output with a typo.
Namespaces Google is a lot more aggressive about namespace advocacy than I like. Their recommendations are sound, with the proviso that namespaces should be applied under the golden rule of readability. Apply when their use clearly improves readability, otherwise don't because they can add a lot of relatively pointless characters to variable names.
VarArrays And Alloca We do allow them, though the cautions google gives in this respect are worth considering.
Boost We don't currently advocate any specific use of boost in Converse/Charm++.
VariableNames We completely disagree with google, variable names should start lowercase and capitalize each word. Don't use underscores in variable names. Using them in macro names is a good idea.
Naming rules The examples use google's ugly underscore scheme that we rejected for variables, otherwise their principles are sound.
Comments You should use the doxygen style commenting scheme using /// to briefly to describe things and longer form blocks as appropriate. ck-cp/controlpoints.h is a decent example of this usage.