code-contributor-README.rst 7.5 KB
Newer Older
1 2
.. code-contributor-README:

3
###########################
4 5 6 7 8 9
Core Contributor Guidelines
###########################

Code formatting
================

10 11
All C/C++ source code in the repository, including the test code, must adhere to 
the source-code formatting and style guidelines described here.
12

13 14
Adding ops to nGraph Core
-------------------------
15

16 17 18 19 20 21
Our design philosophy is that the graph is not a script for running kernels; 
rather, the graph is a snapshot of the computation's building blocks which we 
call ``ops``. Compilation should match ``ops`` to appropriate kernels for the 
backend(s) in use. Thus, we expect that adding of new Core ops should be 
infrequent and that most functionality instead gets added with new functions 
that build sub-graphs from existing core ops.  
22

23 24
The coding style described here should apply to both Core ``ops``, and to any 
functions that build out (upon) sub-graphs from the core.
25 26 27 28 29


Coding style  
-------------

30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46
We have a coding standard to help us to get development done. If part of the 
standard is impeding progress, we either adjust that part or remove it. To this 
end, we employ coding standards that facilitate understanding of *what nGraph 
components are doing*. Programs are easiest to understand when they can be 
understood locally; if most local changes have local impact, you do not need to 
dig through multiple files to understand what something does.

Names
~~~~~

Names should *briefly* describe the thing being named and follow these casing 
standards: 

- Define C++ class or type names with ``CamelCase``.
- Assign template parameters with ``UPPER_SNAKE_CASE``.
- Case variable and function names with ``snake_case``.
    
47 48
Method names for basic accessors are prefixed by ``get_`` or ``set_`` and 
should have simple :math:`\mathcal{O}(1)` implementations:
49 50 51 52

- A ``get_`` method should be externally idempotent. It may perform some simple 
  initialization and cache the result for later use.

53
- An ``is_`` may be used instead of ``get_`` for boolean accessors. Trivial ``get_`` 
54 55
  methods can be defined in a header file.

56
- A ``set_`` method should change the value returned by the corresponding ``get_`` 
57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114
  method.
  
  * Use ``set_is_`` if using ``is_`` to get a value.
  * Trivial ``set_`` methods may be defined in a header file.

- Names of variables should indicate the use of the variable.
  
  * Member variables should be prefixed with ``m_``.
  * Static member variables should be rare and be prefixed with ``s_``.

- Do not use ``using`` to define a type alias at top-level in header file.
  If the abstraction is useful, give it a class.
  
  * C++ does not enforce the abstraction. For example if ``X`` and ``Y`` are
    aliases for the same type, you can pass an ``X`` to something expecting a ``Y``.
  * If one of the aliases were later changed, or turned into a real type, many
    callers could require changes.


Namespaces
~~~~~~~~~~

- ``ngraph`` is for the public API, although this is not currently enforced.
  
  * Use a nested namespace for implementation classes.
  * Use an unnamed namespace or ``static`` for file-local names. This helps 
    prevent unintended name collisions during linking and when using shared 
    and dynamically-loaded libraries.
  * Never use ``using`` at top-level in a header file.
  
    - Doing so leaks the alias into users of the header, including headers that
      follow.
    - It is okay to use ``using`` with local scope, such as inside a class 
      definiton.
  * Be careful of C++'s implicit namespace inclusions. For example, if a 
    parameter's type is from another namespace, that namespace can be visible 
    in the body. 
  * Only use ``using std`` and/or ``using ngraph`` in ``.cpp`` files. ``using`` a
    nested namespace has can result in unexpected behavior.


File Names
~~~~~~~~~~

- Do not use the same file name in multiple directories. At least one 
  IDE/debugger ignores the directory name when setting breakpoints.

- Use ``.hpp`` for headers and ``.cpp`` for implementation.

- Reflect the namespace nesting in the directory hierarchy.

- Unit test files are in the ``tests`` directory.
  
  * Tranformer-dependent tests are tests running on the default transformer or 
    specifying a transformer. For these, use the form

    .. code-block:: cpp

L.S. Cook's avatar
L.S. Cook committed
115
       TEST(file_name, test_name)
116 117 118 119 120 121 122 123 124 125 126

  * Transformer-independent tests:
  
    - File name is ``file_name.in.cpp``
    - Use

      .. code-block:: sh

         TEST(${BACKEND_NAME}, test_name)

      for each test. Fies will be
L.S. Cook's avatar
L.S. Cook committed
127
      generated for each transformer and the ``${BACKEND_NAME}`` will be replaced
128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161
      with the transformer name.


Formatting
~~~~~~~~~~

Things that look different should look different because they are different. We 
use **clang format** to enforce certain formatting. Although not always ideal, 
it is automatically enforced and reduces merge conflicts.

- The :file:`.clang-format` file located in the root of the project specifies 
  our format.
  
  * The script :file:`maint/apply-code-format.sh` enforces that formatting
    at the C/C++ syntactic level.
  * The script at :file:`maint/check-code-format.sh` verifies that the formatting 
    rules are met by all C/C++ code (again, at the syntax level). The script has 
    an exit  code of ``0`` when code meets the standard and non-zero otherwise.  
    This script does *not* modify the source code.

- Formatting with ``#include`` files:
  
  * Put headers in groups separated by a blank line. Logically order the groups
    downward from system-level to 3rd-party to ``ngraph``.
  * Formatting will keep the files in each group in alphabetic order.
  * Use this syntax for files that **do not change during development**; they 
    will not be checked for changes during builds. Normally this will be  
    everything but the ngraph files:

    .. code-block:: cpp

       #include <file>
  
  * Use this syntax for files that **are changing during development**; they will
L.S. Cook's avatar
L.S. Cook committed
162
    be checked for changes during builds. Normally this will be ngraph headers:
163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186

    .. code-block:: cpp

       #include "file"

  * Use this syntax for system C headers with C++ wrappers:

    .. code-block:: cpp

       #include <c...>

- To guard against multiple inclusion, avoid using the ``#define X_H`` style. 
  Use this syntax instead: 

  .. code-block:: cpp

     #pragma once

- The initialization

  .. code-block:: cpp

     Foo x{4, 5};

L.S. Cook's avatar
L.S. Cook committed
187
  is preferred over
188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215

  .. code-block:: cpp

     Foo x(4, 5);

- Indentation should be accompanied by braces; this includes single-line bodies 
  for conditionals and loops.

- Exception checking:
  
  * Throw an exception to report a problem.
  * Nothing that calls ``abort``, ``exit`` or ``terminate`` should be used. Remember 
    that ngraph is a guest of the framework.
  * Do not use exclamation points in messages!
  * Be as specific as practical. Keep in mind that the person who sees the error 
    is likely to be on the other side of the framework and the message might be 
    the only information they see about the problem.

- If you use ``auto``, know what you are doing. ``auto`` uses the same 
  type-stripping rules as template parameters. If something returns a reference, 
  ``auto`` will strip the reference unless you use ``auto&``:
  
  * Don't do things like

    .. code-block:: cpp

       auto s = Shape{2,3};

L.S. Cook's avatar
L.S. Cook committed
216
    Instead, use
217

L.S. Cook's avatar
L.S. Cook committed
218
    .. code-block:: cpp
219

L.S. Cook's avatar
L.S. Cook committed
220
       Shape s{2, 3};
221

222
  * Indicate the type in the variable name.
223

224
- One variable declaration/definition per line
225

226
  - Don't use the C-style
227

228
    .. code-block:: cpp
229

230
       int x, y, *z;
231

232
    Instead, use:
233

234
    .. code-block:: cpp
235

236 237 238
       int x;
       int y;
       int* z;
239