Being an extensive and complicated language, there are often differences of opinions on "good" and "bad" C++ code. Bjarne Stroustrup has said "Within C++ is a smaller, simpler, safer language struggling to get out." We are striving to write in this variant of C++ and are therefore following the "C++ Core Guidelines" that Bjarne and Herb Sutter introduced at CppCon 2015.
Beyond a set of rules that help codify "good" and "bad" C++, we have general principles that help us align the software we develop with the constraints within the problem domain being solved by OpenBMC. These are:
Brevity is the soul of wit.
It is important that code be optimized for the reviewer and maintainer and not for the writer. Solutions should avoid tricks that detract from the clarity of reviewing and understanding it.
Modern practices allow C++ to be an expressive, but concise, language. We tend to favor solutions which succinctly represent the problem in as few lines as possible.
When there is a conflict between clarity and conciseness, clarity should win out.
We strive to keep our code conforming to and utilizing of the latest in C++ standards. Today, that means all C++ code should be compiled using C++14 compiler settings. As the C++17 standard is finalized and compilers support it, we will move to it as well.
We also strive to keep the codebase up-to-date with the latest recommended practices by the language designers. This is reflected by the choice in following the C++ Core Guidelines.
[[Not currently implemented]] We finally desire to have computers do our thinking for us wherever possible. This means having Continuous Integration tests on each repository so that regressions are quickly identified prior to merge. It also means having as much of this document enforced by tools as possible by, for example, astyle/clang-format and clang-tidy.
For those coming to the project from pre-C++11 environments we strongly recommend the book "Effective Modern C++" as a way to get up to speed on the differences between C++98/03 and C++11/14/17.
OpenBMC targets embedded processors that typically have 32-64MB of flash and similar processing power of a typical smart-watch available in 2016. This means that there are times where we must limit library selection and/or coding techniques to compensate for this constraint. Due to the current technology, performance evaluation is done in order of { code size, cpu utilization, and memory size }.
From a macro-optimization perspective, we expect all solutions to have an appropriate algorithmic complexity for the problem at hand. Therefore, an O(n^3)
algorithm may be rejected even though it has good clarity when an O(n*lg(n))
solution exists.
Please follow the guidelines established by the C++ Core Guidelines (CCG).
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md
[[ Last reviewed revision is 53bc78f ]]
Exceptions:
We do not currently utilize the Guideline Support Library provided by the CCG. Any recommendation within the CCG of GSL conventions may be ignored at this time.
The following are not followed:
Additional recommendations within the OpenBMC project on specific language features or libraries.
We do use exceptions as a basis for error handling within OpenBMC.
Use of boost is allowed, under the following circumstances:
The iostream conventions of using 'operator<<' contribute to an increased code size over printf-style operations, due to individual function calls for each appended value. We therefore do not use iostreams, or iostream-style APIs, for logging.
There are cases when using an iostream utility (such as sstream) can result in clearer and similar-sized code. iostream may be used in those situations.
Indentation, naming practices, etc.
[[ These should be codified as much as possible with astyle / clang-format. ]]
An astyle-invocation that closely approximates our indentation style is:
astyle --style=allman --add-brackets --convert-tabs --max-code-length=80 \ --indent=spaces=4 --indent-classes --indent-switches --indent-labels \ --indent-preproc-define --min-conditional-indent=0 --pad-oper \ --pad-header --unpad-paren --break-after-logical \ --align-pointer=type --align-reference=type
Individual OpenBMC repositories can use clang-format if desired. The OpenBMC CI infrastructure will automatically verify the code formatting on code check-in if a .clang_format file is found within the root directory of the repository. This allows for automatic validation of code formatting upon check-in.
OpenBMC requires a clang-format of version 5.0 or greater. An example of how to run clang-format against all code in your repo can be found by referencing the tool used by CI.
A .clang-format file that closely approximates our coding style is:
--- Language: Cpp # BasedOnStyle: LLVM AccessModifierOffset: -2 AlignAfterOpenBracket: Align AlignConsecutiveAssignments: false AlignConsecutiveDeclarations: false AlignEscapedNewlinesLeft: false AlignOperands: true AlignTrailingComments: true AllowAllParametersOfDeclarationOnNextLine: true AllowShortBlocksOnASingleLine: false AllowShortCaseLabelsOnASingleLine: false AllowShortFunctionsOnASingleLine: None AllowShortIfStatementsOnASingleLine: false AllowShortLoopsOnASingleLine: false AlwaysBreakAfterDefinitionReturnType: None AlwaysBreakAfterReturnType: None AlwaysBreakBeforeMultilineStrings: false AlwaysBreakTemplateDeclarations: false BinPackArguments: true BinPackParameters: true BraceWrapping: AfterClass: true AfterControlStatement: true AfterEnum: true AfterFunction: true AfterNamespace: true AfterObjCDeclaration: true AfterStruct: true AfterUnion: true BeforeCatch: true BeforeElse: true IndentBraces: false BreakBeforeBinaryOperators: None BreakBeforeBraces: Custom BreakBeforeTernaryOperators: true BreakConstructorInitializers: AfterColon ColumnLimit: 80 CommentPragmas: '^ IWYU pragma:' ConstructorInitializerAllOnOneLineOrOnePerLine: false ConstructorInitializerIndentWidth: 4 ContinuationIndentWidth: 4 Cpp11BracedListStyle: true DerivePointerAlignment: true PointerAlignment: Left DisableFormat: false ExperimentalAutoDetectBinPacking: false FixNamespaceComments: true ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ] IndentCaseLabels: true IndentWidth: 4 IndentWrappedFunctionNames: false KeepEmptyLinesAtTheStartOfBlocks: true MacroBlockBegin: '' MacroBlockEnd: '' MaxEmptyLinesToKeep: 1 NamespaceIndentation: None ObjCBlockIndentWidth: 2 ObjCSpaceAfterProperty: false ObjCSpaceBeforeProtocolList: true PenaltyBreakBeforeFirstCallParameter: 19 PenaltyBreakComment: 300 PenaltyBreakFirstLessLess: 120 PenaltyBreakString: 1000 PenaltyExcessCharacter: 1000000 PenaltyReturnTypeOnItsOwnLine: 60 PointerAlignment: Right ReflowComments: true SortIncludes: false SpaceAfterCStyleCast: false SpaceBeforeAssignmentOperators: true SpaceBeforeParens: ControlStatements SpaceInEmptyParentheses: false SpacesBeforeTrailingComments: 1 SpacesInAngles: false SpacesInContainerLiterals: true SpacesInCStyleCastParentheses: false SpacesInParentheses: false SpacesInSquareBrackets: false Standard: Cpp11 TabWidth: 4 UseTab: Never ...
If a repository has a setup.cfg file present in its root directory, then CI will automatically verify the Python code meets the pycodestyle requirements. This enforces PEP 8 standards on all Python code.
OpenBMC standards for Python match with PEP 8 so in general, a blank setup.cfg file is all that's needed. If so desired, an enforcement of 80 (vs. the default 79) chars is fine as well:
[pycodestyle] max-line-length = 80
if (condition) { ... }
void foo() { ... }
/// Wrong. if (condition) do_something; /// Correct if (condition) { do_something; }
namespace foo { content }
class Foo { public: Foo(); }
void foo() { while (1) { if (bar()) { ... } } }
switch (foo) { case bar: { bar(); break; } case baz: { baz(); break; } }
void foo() { if (bar) { do { if (baz) { goto exit; } } while(1); exit: cleanup(); } }
/// Correct. SomeBMCType someBMCVariable = bmcFunction(); /// Wrong: type and variable are mixed-case, function isn't lowerCamelCase. SomeBmcType someBmcVariable = BMCFunction();
using type = T
) while a multi-word type alias should be UpperCamelCase (using ArrayOfT = std::array<T, N>
)._t
to match the conventions of the STL.template <typename T> class foo { using type = std::decay_t<T>; }; template <typename T> using foo_t = foo<T>::type;
Prefer '#pragma once' header guard over '#ifndef'-style.
foo(T& bar, const S* baz); /// Correct. foo(T &bar, const S *baz); /// Incorrect.
Follow NL.15: Use spaces sparingly.
Insert whitespace after a conditional and before parens.
if (...) while (...) for (...)
foo((a-1)/b,c-2); /// Incorrect. foo((a - 1) / b, c - 2); /// Correct.
a = * b; /// Incorrect. a = & b; /// Incorrect. a = b -> c; /// Incorrect. if (! a) /// Incorrect.
foo(x, y); /// Correct. foo ( x , y ); /// Incorrect. do (...) { } while(0); /// 'while' here is structured like a function call.
if (this1 == that1 && this2 == that2) /// Correct. if (this1 == that1 && this2 == that2) /// Incorrect.
reallyLongFunctionCall(foo, bar, baz); // Correct. reallyLongFunctionCall( foo, bar, baz); // Also correct. reallyLongFunctionCall( foo, bar, baz); // Similarly correct. reallyLongFunctionCall(foo, bar, baz); // Incorrect.
Always use size_t
or ssize_t
for things that are sizes, counts, etc. You need a strong rationale for using a sized type (ex. uint8_t
) when a size_t will do.
Use uint8_t
, int16_t
, uint32_t
, int64_t
, etc. for types where size is important due to hardware interaction. Do not use them, without good reason, when hardware interaction is not involved; prefer size_t or int instead.