Line data    Source code

       1           0 : \cond NEVER
       2             : Distributed under the MIT License.
       3             : See LICENSE.txt for details.
       4             : \endcond
       5             : # Code Review Guide {#code_review_guide}
       6             : 
       7             : \tableofcontents
       8             : 
       9             : Code must follow the
      10             : <a href="https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md">C++ Core Guidelines</a>
      11             : and the [Google style guide](https://google.github.io/styleguide/cppguide.html).
      12             : If the Google style guide disagrees with the Core Guidelines, follow the Core
      13             : Guidelines. Python code must also follow the
      14             : [Google style guide](https://google.github.io/styleguide/pyguide.html).
      15             : 
      16             : Here we summarize what we view as the more important portions of the guides.
      17             : 
      18             : Stylistic Items:
      19             : 
      20             : * Adhere to [Google style](https://google.github.io/styleguide/cppguide.html).
      21             :   [Can use `clang-format -style=google`][clang_format].
      22             : * CamelCase: class names template parameters, file names, and directory names.
      23             : * snake_case: function, variable, metafunction and metavariable names.
      24             : * SCREAMING_SNAKE_CASE: macros.
      25             : * Functions or classes only used internally in a library should be in a
      26             :   namespace named `LibraryOrFile_detail`. For example, `databox_detail`,
      27             :   `Tensor_detail` or `ConstantExpressions_detail`.
      28             : * Name unused function parameters `/*parameter_name*/` or `/*meta*/` for TMP
      29             :   cases
      30             : * Type aliases that wrap type traits have a trailing `_t` following the STL
      31             : * Private member variables have a [trailing underscore][variable_names].
      32             : * Do not use
      33             :   [Hungarian notation](https://en.wikipedia.org/wiki/Hungarian_notation),
      34             :   e.g. `double* pd_blah` is bad
      35             : * Header order:
      36             :   1. (If a test:) `Framework/TestingFramework.hpp`, followed by one blank line
      37             :   2. (If a cpp file with a corresponding hpp file:) hpp corresponding to cpp,
      38             :      followed by one blank line
      39             :   3. STL and externals (in alphabetical order)
      40             :   4. Blank line
      41             :   5. SpECTRE includes (in alphabetical order)
      42             : * Template definitions in header files are separated from the declaration of
      43             :   the class by the following line, which contains exactly 64 equal signs
      44             :   ``` cpp
      45             :   // ================================================================
      46             :   ```
      47             : 
      48             : * File lists in CMake are alphabetical.
      49             : * No blank lines surrounding Doxygen group comments
      50             :   (<code>// \@{</code> and <code>// \@}</code>).
      51             : * Use the [alternative tokens][alternative_tokens] `or`, `and`, and `not`
      52             :   instead of `||`, `&&`, and `!`.
      53             : * Use C-style Doxygen comments (`/*! ... */`) when using multi-line math,
      54             :   otherwise C-style and C++ style comments are accepted.
      55             : * Use the `align` environment instead of `eqnarray`. See the
      56             :   [texfaq](https://texfaq.org/FAQ-eqnarray) for an explanation as to why.
      57             : * Multi-line equations must have a blank Doxygen line before and after the
      58             :   equation.
      59             : * When addressing requests on a PR, the commit message must start with
      60             :   `fixup` followed by a descriptive message.
      61             : * All python code must comply with the [black](https://github.com/psf/black)
      62             :   formatting specified by the `pyproject.toml` file.
      63             : * Imports in Python code must be sorted to comply to the
      64             :   [isort](https://github.com/PyCQA/isort) formatting specified in the
      65             :   `pyproject.toml` file.
      66             : 
      67             : Code Quality Items:
      68             : 
      69             : * All code passes Clang and CppCheck static analyzers. For help
      70             :   with these tools see \ref static_analysis_tools "here".
      71             : * Almost always `auto`, except with expression templates, i.e. `DataVector`
      72             : * All loops and if statements use braces.
      73             : * Order of declaration in classes is `public` before `private` and member
      74             :   functions before data. See
      75             :   [Google](https://google.github.io/styleguide/cppguide.html#Declaration_Order).
      76             : * Prefer return by value over pass-by-mutable-reference except when mutable
      77             :   reference provides superior performance (in practice if you need a mutable
      78             :   reference use `const gsl::not_null<Type*>` instead of `Type&`). The mutable
      79             :   references must be the first arguments passed to the function.
      80             : * All commits for performance changes provide quantitative evidence and the
      81             :   tests used to obtain said evidence.
      82             : * Never include `<iostream>`, use `Parallel::printf` inside
      83             :   `Parallel/Printf/Printf.hpp` instead, which is safe to use in parallel.
      84             : * When using charm++ nodelocks include `<converse.h>` instead of `<lrtslock.h>`.
      85             : * Do not add anything to [the `std` namespace][extending_std].
      86             : * Virtual functions are explicitly overridden using the `override` keyword.
      87             : * `#%pragma once` is to be used for header guards
      88             : * Prefer range-based for loops
      89             : * Use `size_t` for positive integers rather than `int`, specifically when
      90             :   looping over containers. This is in compliance with what the STL uses.
      91             : * Error messages should be helpful. An example of a bad error message is "Size
      92             :   mismatch". Instead this message could read "The number of grid points in the
      93             :   matrix 'F' is not the same as the number of grid points in the determinant.",
      94             :   along with the runtime values of the mentioned quantities if applicable.
      95             : * Mark immutable objects as `const`
      96             : * Make classes serializable by writing a `pup` function
      97             : * If a class stores an object passed into a constructor the object should
      98             :   be taken by-value and `std::move`d into the member variable.
      99             : * Definitions of function and class templates should be in `.cpp` files with
     100             :   explicit instantiations whenever possible. The macro
     101             :   `GENERATE_EXPLICIT_INSTANTIATIONS` is useful for generating many
     102             :   explicit instantiations.
     103             : * Functions should not be marked `noexcept` unless it is part of the
     104             :   signature of an overriden function (e.g. the what() function of an exception
     105             :   derived from std::exception).
     106             : * Variable names in macros must avoid name collisions, e.g. inside the
     107             :   `PARSE_ERROR` macro you would write
     108             :   `double variable_name_avoid_name_collisions_PARSE_ERROR = 0.0;`
     109             : * Avoid macros if possible. Prefer `constexpr` functions, constexpr
     110             :   variables, and/or template metaprogramming
     111             : * Explicitly specify `double`s, e.g. `sqrt(2.)` or `sqrt(2.0)` instead of
     112             :   `sqrt(2)`.
     113             : * When the index of a `Tensor` is known at compile time, use
     114             :   `get<a, b>(tensor)` instead of the member `get` function.
     115             : * All necessary header files must be included. In header files, prefer
     116             :   forward declarations if class definitions aren't necessary.
     117             : * Explicitly make numbers floating point, e.g. `2.` or `2.0` over `2`.
     118             : * Avoid `mutable` member variables. If you think you really need these, discuss
     119             :   with at least 2 core developers. In a parallel environment `mutable` variables
     120             :   lead to race conditions since multiple threads can mutate a single object by
     121             :   calling a `const` member function.
     122             : 
     123             : [alternative_tokens]:
     124             :   http://en.cppreference.com/w/cpp/language/operator_alternative
     125             : [clang_format]: http://clang.llvm.org/docs/ClangFormat.html
     126             : [extending_std]: http://en.cppreference.com/w/cpp/language/extending_std
     127             : [variable_names]:
     128             :   https://google.github.io/styleguide/cppguide.html#Variable_Names