forked from OSchip/llvm-project
344 lines
17 KiB
Markdown
344 lines
17 KiB
Markdown
<!--===- docs/C++style.md
|
|
|
|
Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
|
See https://llvm.org/LICENSE.txt for license information.
|
|
SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
|
|
|
-->
|
|
|
|
# Flang C++ Style Guide
|
|
|
|
```eval_rst
|
|
.. contents::
|
|
:local:
|
|
```
|
|
|
|
This document captures the style guide rules that are followed in the Flang codebase.
|
|
|
|
## In brief:
|
|
* Use *clang-format*
|
|
from llvm 7
|
|
on all C++ source and header files before
|
|
every merge to master. All code layout should be determined
|
|
by means of clang-format.
|
|
* Where a clear precedent exists in the project, follow it.
|
|
* Otherwise, where [LLVM's C++ style guide](https://llvm.org/docs/CodingStandards.html#style-issues)
|
|
is clear on usage, follow it.
|
|
* Otherwise, where a good public C++ style guide is relevant and clear,
|
|
follow it. [Google's](https://google.github.io/styleguide/cppguide.html)
|
|
is pretty good and comes with lots of justifications for its rules.
|
|
* Reasonable exceptions to these guidelines can be made.
|
|
* Be aware of some workarounds for known issues in older C++ compilers that should
|
|
still be able to compile f18. They are listed at the end of this document.
|
|
|
|
## In particular:
|
|
|
|
Use serial commas in comments, error messages, and documentation
|
|
unless they introduce ambiguity.
|
|
|
|
### Error messages
|
|
1. Messages should be a single sentence with few exceptions.
|
|
1. Fortran keywords should appear in upper case.
|
|
1. Names from the program appear in single quotes.
|
|
1. Messages should start with a capital letter.
|
|
1. Messages should not end with a period.
|
|
|
|
### Files
|
|
1. File names should use dashes, not underscores. C++ sources have the
|
|
extension ".cpp", not ".C" or ".cc" or ".cxx". Don't create needless
|
|
source directory hierarchies.
|
|
1. Header files should be idempotent. Use the usual technique:
|
|
```
|
|
#ifndef FORTRAN_header_H_
|
|
#define FORTRAN_header_H_
|
|
// code
|
|
#endif // FORTRAN_header_H_
|
|
```
|
|
1. `#include` every header defining an entity that your project header or source
|
|
file actually uses directly. (Exception: when foo.cpp starts, as it should,
|
|
with `#include "foo.h"`, and foo.h includes bar.h in order to define the
|
|
interface to the module foo, you don't have to redundantly `#include "bar.h"`
|
|
in foo.cpp.)
|
|
1. In the source file "foo.cpp", put its corresponding `#include "foo.h"`
|
|
first in the sequence of inclusions.
|
|
Then `#include` other project headers in alphabetic order; then C++ standard
|
|
headers, also alphabetically; then C and system headers.
|
|
1. Don't use `#include <iostream>`. If you need it for temporary debugging,
|
|
remove the inclusion before committing.
|
|
|
|
### Naming
|
|
1. C++ names that correspond to well-known interfaces from the STL, LLVM,
|
|
and Fortran standard
|
|
can and should look like their models when the reader can safely assume that
|
|
they mean the same thing -- e.g., `clear()` and `size()` member functions
|
|
in a class that implements an STL-ish container.
|
|
Fortran intrinsic function names are conventionally in ALL CAPS.
|
|
1. Non-public data members should be named with leading miniscule (lower-case)
|
|
letters, internal camelCase capitalization, and a trailing underscore,
|
|
e.g. `DoubleEntryBookkeepingSystem myLedger_;`. POD structures with
|
|
only public data members shouldn't use trailing underscores, since they
|
|
don't have class functions from which data members need to be distinguishable.
|
|
1. Accessor member functions are named with the non-public data member's name,
|
|
less the trailing underscore. Mutator member functions are named `set_...`
|
|
and should return `*this`. Don't define accessors or mutators needlessly.
|
|
1. Other class functions should be named with leading capital letters,
|
|
CamelCase, and no underscores, and, like all functions, should be based
|
|
on imperative verbs, e.g. `HaltAndCatchFire()`.
|
|
1. It is fine to use short names for local variables with limited scopes,
|
|
especially when you can declare them directly in a `for()`/`while()`/`if()`
|
|
condition. Otherwise, prefer complete English words to abbreviations
|
|
when creating names.
|
|
|
|
### Commentary
|
|
1. Use `//` for all comments except for short `/*notes*/` within expressions.
|
|
1. When `//` follows code on a line, precede it with two spaces.
|
|
1. Comments should matter. Assume that the reader knows current C++ at least as
|
|
well as you do and avoid distracting her by calling out usage of new
|
|
features in comments.
|
|
|
|
### Layout
|
|
Always run `clang-format` on your changes before committing code. LLVM
|
|
has a `git-clang-format` script to facilitate running clang-format only
|
|
on the lines that have changed.
|
|
|
|
Here's what you can expect to see `clang-format` do:
|
|
1. Indent with two spaces.
|
|
1. Don't indent public:, protected:, and private:
|
|
accessibility labels.
|
|
1. Never use more than 80 characters per source line.
|
|
1. Don't use tabs.
|
|
1. Don't indent the bodies of namespaces, even when nested.
|
|
1. Function result types go on the same line as the function and argument
|
|
names.
|
|
|
|
Don't try to make columns of variable names or comments
|
|
align vertically -- they are maintenance problems.
|
|
|
|
Always wrap the bodies of `if()`, `else`, `while()`, `for()`, `do`, &c.
|
|
with braces, even when the body is a single statement or empty. The
|
|
opening `{` goes on
|
|
the end of the line, not on the next line. Functions also put the opening
|
|
`{` after the formal arguments or new-style result type, not on the next
|
|
line. Use `{}` for empty inline constructors and destructors in classes.
|
|
|
|
If any branch of an `if`/`else if`/`else` cascade ends with a return statement,
|
|
they all should, with the understanding that the cases are all unexceptional.
|
|
When testing for an error case that should cause an early return, do so with
|
|
an `if` that doesn't have a following `else`.
|
|
|
|
Don't waste space on the screen with needless blank lines or elaborate block
|
|
commentary (lines of dashes, boxes of asterisks, &c.). Write code so as to be
|
|
easily read and understood with a minimum of scrolling.
|
|
|
|
Avoid using assignments in controlling expressions of `if()` &c., even with
|
|
the idiom of wrapping them with extra parentheses.
|
|
|
|
In multi-element initializer lists (especially `common::visitors{...}`),
|
|
including a comma after the last element often causes `clang-format` to do
|
|
a better jobs of formatting.
|
|
|
|
### C++ language
|
|
Use *C++17*, unless some compiler to which we must be portable lacks a feature
|
|
you are considering.
|
|
However:
|
|
1. Never throw or catch exceptions.
|
|
1. Never use run-time type information or `dynamic_cast<>`.
|
|
1. Never declare static data that executes a constructor.
|
|
(This is why `#include <iostream>` is contraindicated.)
|
|
1. Use `{braced initializers}` in all circumstances where they work, including
|
|
default data member initialization. They inhibit implicit truncation.
|
|
Don't use `= expr` initialization just to effect implicit truncation;
|
|
prefer an explicit `static_cast<>`.
|
|
With C++17, braced initializers work fine with `auto` too.
|
|
Sometimes, however, there are better alternatives to empty braces;
|
|
e.g., prefer `return std::nullopt;` to `return {};` to make it more clear
|
|
that the function's result type is a `std::optional<>`.
|
|
1. Avoid unsigned types apart from `size_t`, which must be used with care.
|
|
When `int` just obviously works, just use `int`. When you need something
|
|
bigger than `int`, use `std::int64_t` rather than `long` or `long long`.
|
|
1. Use namespaces to avoid conflicts with client code. Use one top-level
|
|
`Fortran` project namespace. Don't introduce needless nested namespaces within the
|
|
project when names don't conflict or better solutions exist. Never use
|
|
`using namespace ...;` outside test code; never use `using namespace std;`
|
|
anywhere. Access STL entities with names like `std::unique_ptr<>`,
|
|
without a leading `::`.
|
|
1. Prefer `static` functions over functions in anonymous namespaces in source files.
|
|
1. Use `auto` judiciously. When the type of a local variable is known,
|
|
monomorphic, and easy to type, be explicit rather than using `auto`.
|
|
Don't use `auto` functions unless the type of the result of an outlined member
|
|
function definition can be more clear due to its use of types declared in the
|
|
class.
|
|
1. Use move semantics and smart pointers to make dynamic memory ownership
|
|
clear. Consider reworking any code that uses `malloc()` or a (non-placement)
|
|
`operator new`.
|
|
See the section on Pointers below for some suggested options.
|
|
1. When defining argument types, use values when object semantics are
|
|
not required and the value is small and copyable without allocation
|
|
(e.g., `int`);
|
|
use `const` or rvalue references for larger values (e.g., `std::string`);
|
|
use `const` references to rather than pointers to immutable objects;
|
|
and use non-`const` references for mutable objects, including "output" arguments
|
|
when they can't be function results.
|
|
Put such output arguments last (_pace_ the standard C library conventions for `memcpy()` & al.).
|
|
1. Prefer `typename` to `class` in template argument declarations.
|
|
1. Prefer `enum class` to plain `enum` wherever `enum class` will work.
|
|
We have an `ENUM_CLASS` macro that helps capture the names of constants.
|
|
1. Use `constexpr` and `const` generously.
|
|
1. When a `switch()` statement's labels do not cover all possible case values
|
|
explicitly, it should contain either a `default:;` at its end or a
|
|
`default:` label that obviously crashes; we have a `CRASH_NO_CASE` macro
|
|
for such situations.
|
|
1. On the other hand, when a `switch()` statement really does cover all of
|
|
the values of an `enum class`, please insert a call to the `SWITCH_COVERS_ALL_CASES`
|
|
macro at the top of the block. This macro does the right thing for G++ and
|
|
clang to ensure that no warning is emitted when the cases are indeed all covered.
|
|
1. When using `std::optional` values, avoid unprotected access to their content.
|
|
This is usually by means of `x.has_value()` guarding execution of `*x`.
|
|
This is implicit when they are function results assigned to local variables
|
|
in `if`/`while` predicates.
|
|
When no presence test is obviously protecting a `*x` reference to the
|
|
contents, and it is assumed that the contents are present, validate that
|
|
assumption by using `x.value()` instead.
|
|
1. We use `c_str()` rather than `data()` when converting a `std::string`
|
|
to a `const char *` when the result is expected to be NUL-terminated.
|
|
1. Avoid explicit comparisions of pointers to `nullptr` and tests of
|
|
presence of `optional<>` values with `.has_value()` in the predicate
|
|
expressions of control flow statements, but prefer them to implicit
|
|
conversions to `bool` when initializing `bool` variables and arguments,
|
|
and to the use of the idiom `!!`.
|
|
|
|
#### Classes
|
|
1. Define POD structures with `struct`.
|
|
1. Don't use `this->` in (non-static) member functions, unless forced to
|
|
do so in a template member function.
|
|
1. Define accessor and mutator member functions (implicitly) inline in the
|
|
class, after constructors and assignments. Don't needlessly define
|
|
(implicit) inline member functions in classes unless they really solve a
|
|
performance problem.
|
|
1. Try to make class definitions in headers concise specifications of
|
|
interfaces, at least to the extent that C++ allows.
|
|
1. When copy constructors and copy assignment are not necessary,
|
|
and move constructors/assignment is present, don't declare them and they
|
|
will be implicitly deleted. When neither copy nor move constructors
|
|
or assignments should exist for a class, explicitly `=delete` all of them.
|
|
1. Make single-argument constructors (other than copy and move constructors)
|
|
'explicit' unless you really want to define an implicit conversion.
|
|
|
|
#### Pointers
|
|
There are many -- perhaps too many -- means of indirect addressing
|
|
data in this project.
|
|
Some of these are standard C++ language and library features,
|
|
while others are local inventions in `lib/Common`:
|
|
* Bare pointers (`Foo *p`): these are obviously nullable, non-owning,
|
|
undefined when uninitialized, shallowly copyable, reassignable, and often
|
|
not the right abstraction to use in this project.
|
|
But they can be the right choice to represent an optional
|
|
non-owning reference, as in a function result.
|
|
Use the `DEREF()` macro to convert a pointer to a reference that isn't
|
|
already protected by an explicit test for null.
|
|
* References (`Foo &r`, `const Foo &r`): non-nullable, not owning,
|
|
shallowly copyable, and not reassignable.
|
|
References are great for invisible indirection to objects whose lifetimes are
|
|
broader than that of the reference.
|
|
Take care when initializing a reference with another reference to ensure
|
|
that a copy is not made because only one of the references is `const`;
|
|
this is a pernicious C++ language pitfall!
|
|
* Rvalue references (`Foo &&r`): These are non-nullable references
|
|
*with* ownership, and they are ubiquitously used for formal arguments
|
|
wherever appropriate.
|
|
* `std::reference_wrapper<>`: non-nullable, not owning, shallowly
|
|
copyable, and (unlike bare references) reassignable, so suitable for
|
|
use in STL containers and for data members in classes that need to be
|
|
copyable or assignable.
|
|
* `common::Reference<>`: like `std::reference_wrapper<>`, but also supports
|
|
move semantics, member access, and comparison for equality; suitable for use in
|
|
`std::variant<>`.
|
|
* `std::unique_ptr<>`: A nullable pointer with ownership, null by default,
|
|
not copyable, reassignable.
|
|
F18 has a helpful `Deleter<>` class template that makes `unique_ptr<>`
|
|
easier to use with forward-referenced data types.
|
|
* `std::shared_ptr<>`: A nullable pointer with shared ownership via reference
|
|
counting, null by default, shallowly copyable, reassignable, and slow.
|
|
* `Indirection<>`: A non-nullable pointer with ownership and
|
|
optional deep copy semantics; reassignable.
|
|
Often better than a reference (due to ownership) or `std::unique_ptr<>`
|
|
(due to non-nullability and copyability).
|
|
Can be wrapped in `std::optional<>` when nullability is required.
|
|
Usable with forward-referenced data types with some use of `extern template`
|
|
in headers and explicit template instantiation in source files.
|
|
* `CountedReference<>`: A nullable pointer with shared ownership via
|
|
reference counting, null by default, shallowly copyable, reassignable.
|
|
Safe to use *only* when the data are private to just one
|
|
thread of execution.
|
|
Used sparingly in place of `std::shared_ptr<>` only when the overhead
|
|
of that standard feature is prohibitive.
|
|
|
|
A feature matrix:
|
|
|
|
| indirection | nullable | default null | owning | reassignable | copyable | undefined type ok? |
|
|
| ----------- | -------- | ------------ | ------ | ------------ | -------- | ------------------ |
|
|
| `*p` | yes | no | no | yes | shallowly | yes |
|
|
| `&r` | no | n/a | no | no | shallowly | yes |
|
|
| `&&r` | no | n/a | yes | no | shallowly | yes |
|
|
| `reference_wrapper<>` | no | n/a | no | yes | shallowly | yes |
|
|
| `Reference<>` | no | n/a | no | yes | shallowly | yes |
|
|
| `unique_ptr<>` | yes | yes | yes | yes | no | yes, with work |
|
|
| `shared_ptr<>` | yes | yes | yes | yes | shallowly | no |
|
|
| `Indirection<>` | no | n/a | yes | yes | optionally deeply | yes, with work |
|
|
| `CountedReference<>` | yes | yes | yes | yes | shallowly | no |
|
|
|
|
### Overall design preferences
|
|
Don't use dynamic solutions to solve problems that can be solved at
|
|
build time; don't solve build time problems by writing programs that
|
|
produce source code when macros and templates suffice; don't write macros
|
|
when templates suffice. Templates are statically typed, checked by the
|
|
compiler, and are (or should be) visible to debuggers.
|
|
|
|
### Exceptions to these guidelines
|
|
Reasonable exceptions will be allowed; these guidelines cannot anticipate
|
|
all situations.
|
|
For example, names that come from other sources might be more clear if
|
|
their original spellings are preserved rather than mangled to conform
|
|
needlessly to the conventions here, as Google's C++ style guide does
|
|
in a way that leads to weirdly capitalized abbreviations in names
|
|
like `Http`.
|
|
Consistency is one of many aspects in the pursuit of clarity,
|
|
but not an end in itself.
|
|
|
|
## C++ compiler bug workarounds
|
|
Below is a list of workarounds for C++ compiler bugs met with f18 that, even
|
|
if the bugs are fixed in latest C++ compiler versions, need to be applied so
|
|
that all desired tool-chains can compile f18.
|
|
|
|
### Explicitly move noncopyable local variable into optional results
|
|
|
|
The following code is legal C++ but fails to compile with the
|
|
default Ubuntu 18.04 g++ compiler (7.4.0-1ubuntu1~18.0.4.1):
|
|
|
|
```
|
|
class CantBeCopied {
|
|
public:
|
|
CantBeCopied(const CantBeCopied&) = delete;
|
|
CantBeCopied(CantBeCopied&&) = default;
|
|
CantBeCopied() {}
|
|
};
|
|
std::optional<CantBeCopied> fooNOK() {
|
|
CantBeCopied result;
|
|
return result; // Legal C++, but does not compile with Ubuntu 18.04 default g++
|
|
}
|
|
std::optional<CantBeCopied> fooOK() {
|
|
CantBeCopied result;
|
|
return {std::move(result)}; // Compiles OK everywhere
|
|
}
|
|
```
|
|
The underlying bug is actually not specific to `std::optional` but this is the most common
|
|
case in f18 where the issue may occur. The actual bug can be reproduced with any class `B`
|
|
that has a perfect forwarding constructor taking `CantBeCopied` as argument:
|
|
`template<typename CantBeCopied> B(CantBeCopied&& x) x_{std::forward<CantBeCopied>(x)} {}`.
|
|
In such scenarios, Ubuntu 18.04 g++ fails to instantiate the move constructor
|
|
and to construct the returned value as it should, instead it complains about a
|
|
missing copy constructor.
|
|
|
|
Local result variables do not need to and should not be explicitly moved into optionals
|
|
if they have a copy constructor.
|