Skip to content

Coding Conventions and Developer Guide

lukasm91 edited this page Mar 26, 2019 · 39 revisions

Coding Conventions

Naming Conventions

  • We use snake_case if not stated otherwise.
  • Object member names are lowercase, start with 'm_' and each word is split with an underscore (m_class_member)
  • Static object member names are lowercase and each word is split with an underscore (static_member)
  • Object names are lowercase and each word is split with an underscore (class_name)
  • Template arguments have the first letter of each word capitalized (ProperCase), no underscores (TemplateArgument)
  • Local variables are lowercase and the words are separated by underscores
  • Alias names/typedefs are lowercase, words separated by underscores, and they end with _t (some_def_t), except the case in which a value of that type is never used (apart from default constructing unnamed objects).
  • The class methods and all the functions follow the same convention: lowercase, words separated by underscores
  • Getters are the member variables without the m_ prefix (class_member())
  • Enum types follow the convention of the object names and should be scoped enums.

Namespaces:

Namespaces uses the following syntax

namespace the_name {
    namespace nested_name {
    } // namespace nested_name
} // namespace the_name

The library main namespace is gridtools.

Utilities that are not exposed to the user API are in the nested namespace <prefix>_impl_, where <prefix> uniquely identifies the current scope (usually the file), e.g. namespace array_impl_{}. All existing impl namespaces should be changed to this convention.

Formatting

Is defined by the clang-format file in the repository, see Git Hooks.

C++

Scoping

  • Never use using keyword in library header files.
  • Do not use nested classes, except for metafunctions or classes without non-static data members and non-static member functions
  • Place a function's variables in the narrowest scope possible, and initialize at the declaration.
// not ok
int a, b, c, d; // only definition, not initialization
b = 10; 
d = 10;
for (a=0; a < b; ++a) {
    c=3; //define c in the scope where you use it
}
int b = 10; 
int d = 10;
for(int a=0; a < b; ++a) {
    int c = 3;
}

Inheritance

  • Make sure inheritance models a "is a" relationship, otherwise use composition.
// not ok 
class Bus : public Ship 
...
// ok
class Bus: public Vehicle
...
  • Never redefine an inherited non virtual method
class Base
{
    void foo() { printf("base\n");}
}
class Subclass : public Base
{
    void foo() { printf("sub\n"); } //redefinition of foo
}
Subclass* sub = new Subclass();
sub->foo(); //prints sub
static_cast<Base*>(sub)->foo(); //prints base

Includes

  • All internal includes in the library are with relative paths and "", e.g. #include "../arg.hpp".
  • In examples and tests <> are used with paths relative to include, e.g. #include <gridtools/stencil_composition/stencil_composition.hpp>.
  • The GridTools is a set of modules, each providing different functionality. The user might want to use only functionality of a module. For each module, i.e. storage, there is a unique header (with the same name as the header) that the user needs to include. For example:
    • storage -> storage/storage.hpp
    • stencil_composition/stencil_composition.hpp
  • filenames should use underscore

Classes

  • When implementing a class the following structure should be used

    • First declare typedefs / using statements
    • Then declare static data members
    • Followed by data members
    • Then constructors
    • And, at the end the rest of member functions
  • If a class has virtual methods, the destructor must be virtual

  • Pass primitive types (int, double, etc.) by value, unless it is an output parameter of the function.

  • Other input only arguments are passed by const reference or forwarding reference.

Error checking and report: Exceptions, Static Asserts and Asserts

In general, exceptions are used to inform the user/caller that a function cannot be completed for some reason. Assertions are used to check if an invariant, a precondition, or a postcondition is satisfied. Also, exceptions are error conditions that the user may fix. An assert cannot be fixed.

An exception is thrown when an allocation fails, since it signifies the operation cannot be completed, but the user may be able to fix it. An assert is triggered when the internal state of an object is invalid, and from that moment the computation cannot be trusted, the user does no longer control what's going on.

Another difference between asserts and exceptions is that asserts are not active in release mode. So, the errors that are caught with assertions must be ones that can be fixed in the development phase, but they should not occur in production phase, since this would lead to pretty nasty situations.

So, for checking that the data fields are big enough to contain the iteration space an exception is perfect: maybe the user can adjust the sizes somehow. To check that a stencil composition is valid static_asserts is the way to go, but for checking that the pointers in iterate_domain are not null, asserts should be used.

Pointers

  • Use references whenever possible (instead of pointers), i.e. when assignment happens at initialization time and the reference does not change over time.
  • Always assert pointer before using them
assert(ptr);
ptr->xx();
  • Try to use RAII, i.e. use smart pointers for non performance critical sections:
std::shared_ptr<MyClass> ptr = std::make_shared<MyClass>();
  • Use delete[] for freeing array of pointers.

metaprograms

  • Avoid "Blob" anti-pattern (see Chapter 2.2 of Book C++ Template Metaprogramming from David Abrahams), where one metafunction computes multiple output. It decreases readability. For example in the following example, run_functor_traits is a blob anti-pattern
template <typename run_functor_derived>
class run_functor
{
   using execute_policy_t = typename run_functor_traits<run_functor_derived>::execute_policy;
   using arguments_t = typename run_functor_traits<run_functor_derived>::arguments;
   using local_domain_t = typename run_functor_traits<run_functor_derived>::local_domain;
}

Replace it by single output type patterns:

template <typename run_functor_derived>
class run_functor
{
   using arguments_t = typename run_functor_arguments<run_functor_derived>::type;
   using local_domain_t = typename run_functor_local_domain<run_functor_derived>::type 
}

Developer's Guide

Code Structure

  • Different classes with different functionality go into different header files. Do not build .h files with a blob of classes.
  • Name of header files is after name of the class. For example local_domain.h contains the local_domain class
  • Put metafunctions associated to, or act on a type either in the same file (if only a few) or in a <type>_metafunctions.hpp file. For example the metafunction
template <...> struct make_interval{...};

will be placed in interval_metafunctions.h

Tests

  • Everything is tested: runtime functions, methods, functors but as well metafunctions. If you write new classes or method, provide the corresponding metafunctions before creating a pull request.
  • Write all tests of a class or functionality from <type>.hpp into a file in the unit_tests folder, named test_type.cpp

Documentation

We document code using Doxygen. The documentation is organized by groups. In Doxygen, groups can be arbitrarily nested, so a group can be defined inside another one. A top-level group will result in the final documentation as a Module, while the nested groups as sub-modules (the itemized list will be indented consequently). Let's look at an example. The file selected for this example is stencil_composition/expressions/expressions.hpp.

namespace gridtools {
    /** \ingroup stencil_composition
        @{
    */

    /** \defgroup expressions Expressions
        @{
    */

    /** Namespace containing all the components to enable using expressions in stencil operators
     */
    namespace expressions {
    } // namespace expressions

    /** @} */
    /** @} */
} // namespace gridtools

This file will define a sub-group of the stencil_composition group, so after having said that the content is part of the stencil composition group, we can define (we can use \ingroup if the group is defined somewhere else) the group for expressions and declare that the whole thing belong there. Notice the double @} at the end since we need to say that we need to close the documentation for both groups.

Remember to put enclose the documentation of your classes in the right group. Usually this is done by using the \ingroup command followed by @{ at the beginning of the file after the gridtools namespace declaration, and by using the @} command at the end before closing the gridtools namespace declaration.

For the common folder the situation is slightly different, since it contains different utilities needed by the rest of the code. There we will have a Common group, in which each utility is put in a separate \defgroup.

As a rule, if you are developing a new module, you should do the proper sectioning. If you are modifying a file that does not have a section, you are not supposed to add sectioning. The rule is to not make the documentation worse than what it is.

Remember to use \param for regular parameter, \tparam for template parameters, and \return for the returned value if not void, to document the arguments of a function. When the argument name is unnamed, it may be tricky to get Doxygen to not complain about it. I found out that sometime a * in place of the argument name works, but other times a generic x works.

Below is an example

    /**
     * @brief functor that executes all the functors contained within the mss
     * @tparam MssComponentsArray meta array containing all the mss descriptors
     * @tparam Grid grid where the mss functor will be applied
     * @tparam BackendId id of backend
     * @tparam ExecutionInfo information about the execution, e.g. block sizes
     */
     template <typename MssComponentsArray, 
         typename Grid,
         typename LocalDomains,
         typename BackendIds,
         typename ExecutionInfo>
    struct mss_functor
    {

        /**
         * @brief This is a do something method
         * @param local_domain_lists this is a param
         * @param coords this is a param
         * @param block_idx this is a param
         * @param block_idy this is a param
         * @return this is the return value
         */
        static int do_something(MssLocalDomainArray& local_domain_lists, const Coords& coords, const int block_idx, const int block_idy) :
             {...}

We are not enforcing the use of \ or @ for the commands. Some commands requires @, like @{ and @}, but others can use both. While the \ because is less dense, code readability is not impacted by this choice and could be left to the personal taste of the developer.

If there are several specializations of a template, for instance in a recursive template pattern, a specialization with obvious role can be left out of Doxygen documentation by putting the following

/// \private

In place of the usual Doxygen documentation.

To make the Doxygen documentation you need doxygen and dot available and you need to run make doc from the build folder.

Code comments

For other comments that are not meant to be parsed by Doxygen, it is important to document non-obvious statements and algorithms. For instance, when calling a meta-function to iterate over a sequence of types, it would be useful to document why that iteration happen, since the documentation of the meta-function is usually too generic to be applied to the particular use. The programmer should then describe the objective of the function (this should be done at the Doxygen level), plus any intermediate step that may help a future reader to understand the code.

Git & Code Review

  • Create one branch per functionality. Avoid personal branches that mix multiple functionality and keep growing.
  • Do not create branches from branches (unless you a have a good reason), always branch from master

Before merging into master

  • Run the Jenkins tests, see Jenkins
  • The master branch should always be in a healthy state. It must not contain bugs nor performance bugs.

CMake Conventions

  • Cached CMake variable names are always uppercase and prefixed with GT_. Multi-word variables are separated with '_'. For example set(GT_ENABLE_TARGET_CUDA "")