diff flang/docs/C++style.md @ 221:79ff65ed7e25

LLVM12 Original
author Shinji KONO <kono@ie.u-ryukyu.ac.jp>
date Tue, 15 Jun 2021 19:15:29 +0900
parents
children 5f17cb93ff66
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/flang/docs/C++style.md	Tue Jun 15 19:15:29 2021 +0900
@@ -0,0 +1,343 @@
+<!--===- 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.