blob: 68ba5e275708d61dff50b94c9f1ab5d5bbff5804 [file] [view] [edit]
# Open Screen Library Style Guide
The Open Screen Library follows the [Chromium C++ coding style](https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++.md)
which, in turn, defers to the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html).
We also follow the [Chromium C++ Do's and Don'ts](https://sites.google.com/a/chromium.org/dev/developers/coding-style/cpp-dos-and-donts).
**C++20** is the maximum language level allowed in the library. For modern
C++ features, we follow the guidelines set in the style guide's
[Modern C++ use in Chromium](https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-features.md)
document, with the exceptions noted below.
In general Open Screen follows [You Aren't Gonna Need It](https://martinfowler.com/bliki/Yagni.html)
principles. This means we avoid adding generic classes, functions, or libraries
that are only used in one place for one thing. Once there are multiple usages of
a function that can be handled in a generic way, that function is a candidate
for our base libraries in `util/` and `platform/base`.
[TOC]
## Disallowed Styles and Features
[Blink style](https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/blink-c++.md)
is *not allowed* anywhere in the Open Screen Library.
GCC does not support designated initializers for non-trivial types. This means
that the `.member = value` struct initialization syntax is not supported unless
all struct members are primitive types or structs of primitive types (i.e. no
unions, complex constructors, etc.).
Abseil is not allowed in Open Screen, although it is still a dependency due to
being needed for some third party libraries, e.g. protobuf.
## Modifications to the Chromium C++ Guidelines
- `<functional>` and `std::function` objects are allowed.
- `<chrono>` is allowed and encouraged for representation of time.
- `<thread>` and `<mutex>` are allowed, but discouraged from general use as the
library only needs to handle threading in very specific places;
see [threading.md](threading.md).
- Following YAGNI principles, only implement comparison operator overloads as
needed; for example, implementing `operator<` for use in an STL container does
not require implementing all comparison operators.
## Code Syntax
- Generally, braces are preferred for single-line if statements. Follow the
style of surrounding code.
- Using-declarations are banned from headers. Type aliases should not be
included in headers, except at class scope when they form part of the class
definition.
- Exception: Using-declarations for convenience may be put into a shared
header for internal library use. These may only be included in
.cc files.
- Exception: if a class has an associated POD identifier (int/string), then
declare a type alias at namespace scope for that identifier instead of using
the POD type. For example, if a class Foo has a string identifier, declare
`using FooId = std::string` in foo.h.
- Case by case exceptions may be made for readability/convenience,
e.g. as in `platform/base/span.h`.
## Initialization
Generally, follow the guidance in [Abseil ToTw \#88](https://abseil.io/tips/88).
This means to prefer prefer copy-initialization over direct-initialization when
available, and use parentheses over curly braces when resorting to
direct-initialization.
Preferred:
```c++
int x = 2;
std::string foo = "Hello World";
std::vector<int> v = {1, 2, 3};
auto matrix = std::make_unique<NewMatrix>(rows, cols);
MyStruct x = {true, 5.0};
```
Discouraged:
```c++
int x{2};
std::string foo{"Hello World"};
std::vector<int> v{1, 2, 3};
std::unique_ptr<Matrix> matrix{NewMatrix(rows, cols)};
MyStruct x{true, 5.0};
```
## Copy and Move Operators
Use the following guidelines when deciding on copy and move semantics for
objects:
- Objects with data members greater than 32 bytes should be move-able.
- Known large objects (I/O buffers, etc.) should be be move-only.
- Variable length objects should be move-able
(since they may be arbitrarily large in size) and, if possible, move-only.
- Inherently non-copyable objects (like sockets) should be move-only.
### Default Copy and Move Operators
We [prefer the use of `default` and `delete`](https://sites.google.com/a/chromium.org/dev/developers/coding-style/cpp-dos-and-donts#TOC-Prefer-to-use-default)
to declare the copy and move semantics of objects. See
[Stroustrup's C++ FAQ](http://www.stroustrup.com/C++11FAQ.html#default)
for details on how to do that.
Classes should prefer [member initialization](https://en.cppreference.com/w/cpp/language/data_members#Member_initialization)
for POD members (as opposed to value initialization in the constructor). Every
POD member must be initialized by every constructor, of course, to prevent
[default initialization](https://en.cppreference.com/w/cpp/language/default_initialization)
from setting them to indeterminate values.
### User Defined Copy and Move Operators
Classes should follow the [rule of three/five/zero](https://en.cppreference.com/w/cpp/language/rule_of_three).
This means that if they implement a destructor or any of the copy or move
operators, then all five (destructor, copy & move constructors, copy & move
assignment operators) should be defined or marked as `delete`d as appropriate.
Finally, polymorphic base classes with virtual destructors should `default` all
constructors, destructors, and assignment operators.
Note that operator definitions belong in the source (`.cc`) file, including
`default`, with the exception of `delete`, because it is not a definition,
rather a declaration that there is no definition, and thus belongs in the header
(`.h`) file.
## Passing Objects by Value or Reference
In most cases, pass by value is preferred as it is simpler and more flexible.
If the object being passed is move-only, then no extra copies will be made. If
it is not, be aware this may result in unintended copies.
To guarantee that ownership is transferred, pass by rvalue reference for objects
with move operators. Often this means adding an overload that takes a const
reference as well.
Pass ownership via `std::unique_ptr<>` for non-movable objects.
Ref: [Google Style Guide on Rvalue References](https://google.github.io/styleguide/cppguide.html#Rvalue_references)
## Noexcept
We prefer to use `noexcept` on move constructors. Although exceptions are not
allowed, this declaration [enables STL optimizations](https://en.cppreference.com/w/cpp/language/noexcept_spec).
Additionally, GCC requires that any type using a defaulted `noexcept` move
constructor/operator= has a `noexcept` copy or move constructor/operator= for
all of its members.
## Template Programming
Template programming should be not be used to write generic algorithms or
classes when there is no usage of the algorithm or class with more than one
dependent type. When similar code applies to multiple types, use templates
sparingly and on a case-by-case basis. Keep template metaprogramming (i.e.,
conditional and/or computed template parameters) to a minimum to ensure
type safety.
## Unit testing
Follow Google’s testing best practices for C++. Design classes in such a way
that testing the public API is sufficient. Strive to follow this guidance,
trading off with the amount of public API surfaces needed and long-term
maintainability.
Ref: [Test Behavior, Not Implementation](https://testing.googleblog.com/2013/08/testing-on-toilet-test-behavior-not.html)
## Open Screen Library Features
- For public API functions that return values or errors, please return
[`ErrorOr<T>`](https://chromium.googlesource.com/openscreen/+/main/platform/base/error.h).
- In the implementation of public APIs invoked by the embedder, use
`OSP_CHECK(<TaskRunner>.IsRunningOnTaskRunner())` to catch thread safety
problems early.
### Helpers for `std::chrono`
One of the trickier parts of the Open Screen Library is using time and clock
functionality provided by [`platform/api/time.h`](https://chromium.googlesource.com/openscreen/+/refs/heads/main/platform/api/time.h).
- When working extensively with `std::chrono` types in implementation code,
[`util/chrono_helpers.h`](https://chromium.googlesource.com/openscreen/+/refs/heads/main/util/chrono_helpers.h)
can be included for access to type aliases for common `std::chrono` types, so
they can just be referred to as `hours`, `milliseconds`, etc. This header also
includes helpful conversion functions, such as `to_milliseconds` instead of
`std::chrono::duration_cast<std::chrono::milliseconds>`.
`util/chrono_helpers.h` can only be used in library-internal code, and
this is enforced by DEPS.
- `Clock::duration` is defined currently as `std::chrono::microseconds`, and
thus is generally not suitable as a time type (developers generally think in
milliseconds). Prefer casting from explicit time types using
`Clock::to_duration`, e.g. `Clock::to_duration(seconds(2))`
instead of using `Clock::duration` types directly.
### OSP_CHECK and OSP_DCHECK
These are provided in [`platform/api/logging.h`](https://chromium.googlesource.com/openscreen/+/refs/heads/main/platform/api/logging.h)
and act as run-time assertions (i.e., they
test an expression, and crash the program if it evaluates as false). They are
not only useful in determining correctness, but also serve as inline
documentation of the assumptions being made in the code. They should be used in
cases where they would fail only due to current or future coding errors.
These should *not* be used to sanitize non-const data, or data otherwise derived
from external inputs. Instead, one should code proper error-checking and
handling for such things.
OSP_CHECKs are "turned on" for all build types. However, OSP_DCHECKs are only
"turned on" in Debug builds, or in any build where the `dcheck_always_on=true`
GN argument is being used. In fact, at any time during development (including
Release builds), it is highly recommended to use `dcheck_always_on=true` to
catch bugs.
When OSP_DCHECKs are "turned off" they effectively become code comments: All
supported compilers will not generate any code, and they will automatically
strip-out unused functions and constants referenced in OSP_DCHECK expressions
(unless they are "extern" to the local module); and so there is absolutely no
run-time/space overhead when the program runs. For this reason, a developer need
not explicitly sprinkle "#if OSP_DCHECK_IS_ON()" guards all around any
functions, variables, etc. that will be unused in "DCHECK off" builds.
Use OSP_DCHECK and OSP_CHECK in accordance with the
[Chromium guidance for DCHECK/CHECK](https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++.md#check_dcheck_and-notreached).