Coding Conventions and Developer Guide

Coding Conventions

Naming Conventions

• The object member names are lowercase, start with 'm_' and each word is split with an underscore (m_class_member)
• The static object member names are lowercase, start with 's_' and each word is split with an underscore (s_static_member)
• The object names are lowercase and each word is split with an underscore (class_name)
• The template arguments have the first letter of each word capitalized (ProperCase), no underscores (TemplateArgument)
• The local variables are lowercase and the words are separated by underscores (same as for object names, well, it's not easy mistake them)
• The typedef names 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
• TODO Getter & Setters?
• TODO class names?
• enum types follow the convention of the object names, but are defined inside the enumtype namespace (enumtype::some_id)

Namespaces:

Namespaces uses the following synthax

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

At current time, the library main namespace is gridtools. All library components should be in this namespace except gridtools::enumtype that refers to enums.

Namespace _impl contains the utilities that are not exposed to the user API.

Q: Change _impl in _detail ?

Formatting

• Always use spaces, never tabs.
• Indent with exactly 4 spaces.
• Try to keep line width no longer than 120 characters. Code in an IDE with large screens looks good with longer lines, but code review tools like github does not play well with long lines.

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

PAOLO: OK as general rule, but not too strict: I find it useful in certain occasions to override behaviours in the subclasses, see the arg_decorator implementation

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

• You must define a default constructor if your class defines member variables and has no other constructors. Otherwise the compiler will do it for you, badly. For example

class Rectangle
{
public:
    void Rectangle()
    {
        // initialize variables with impossible default value
        // -> debugging easier
        m_width = -1;
        m_heigth = -1;
    }
private:
    int m_width;
    int m_height;
};

• Do never rely on default initialization of compiler

• Always disable the copy constructor and assignment operators of a class (unless you explicitly provide those) by means of the DISALLOW_COPY_AND_ASSIGN macro.

class Foo 
{
    DISALLOW_COPY_AND_ASSIGN(Foo);
public: 
    Foo(int f);
    ~Foo();

• 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, except the argument can be NULL, then a const pointer is 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:

boost::shared_ptr<MyClass> ptr = boost::make_shared<MyClass>();

• Use delete[] for freeing array of pointers.

metaprograms

• Every type passed as a template argument is checked (asserted). This increase both safety and readability (from trait that checks the type we infer its function)

template<
    typename MssArray, 
    typename Coords, 
    typename MssLocalDomainArray, 
    enumtype::backend BackendId, 
    enumtype::strategy StrategyId
>
struct mss_functor
{
    BOOST_STATIC_ASSERT((
        is_sequence_of<
            MssLocalDomainArray, 
            is_mss_local_domain
        >::value));
    BOOST_STATIC_ASSERT((
        is_meta_array_of<MssArray, is_mss_descriptor>::value
    ));
    BOOST_STATIC_ASSERT((is_coordinates<Coords>::value));
    ....
}

• 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
{
   typedef typename run_functor_traits<run_functor_derived>::
       execute_policy execute_policy_t;
   typedef typename run_functor_traits<run_functor_derived>::
       arguments arguments_t;
   typedef typename run_functor_traits<run_functor_derived>::
       local_domain local_domain_t;
}

Replace it by single output type patterns:

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

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 in a <type>_metafunctions.h file. For example the metafunction

template<...> struct make_interval{...};

will be placed in interval_metafunctions.h

• Put implementation details in <type>_impl (or _detail if the internal namespace is turned into _detail)

Tests

• Everything is tested: runtime functions, metdhos, 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>.h into a file in the unit_tests folder, named test_<type>.h

Documentation

• Use doxygen documentations. Below is an example

    /**
     * @brief functor that executes all the functors contained within the mss
     * @tparam MssArray meta array containing all the mss descriptors
     * @tparam Coords coordinates
     * @tparam MssLocalDomainArray sequence of mss local domain (each contains the local domain list of a mss)
     * @tparam BackendId id of backend
     * @tparam StrategyId id of strategy
     */
    template<typename MssArray, typename Coords, typename MssLocalDomainArray, enumtype::backend BackendId, enumtype::strategy StrategyId>
    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_smething(MssLocalDomainArray& local_domain_lists, const Coords& coords, const int block_idx, const int block_idy) :
             {...}

Git & Code Review

• Create one branch per functionality. Avoid personal branches that mix multiple functionality and keep growing.
• Always merge and close a branch after a pull request is completed (do not continue developing in the branch)
• Do not create branches from branches (unless you a have a good reason), always branch from master

Before merging into master

• Any branch candidate for merge into the trunk, for which we have created a pull request should have been compiled and tested for all the different cmake options (at least the ones that generate different compilations), like GPU=ON, OFF, CXX11=ON|OFF, DEBUG|Release, etc.
• master should always be in a healthy state. It must not contain bug nor performance bugs. That means it compiles, and run for all backends and cmake options, and performance numbers for all backends are matching the reference values.
• Check the performance for all backends for the representative benchmarks. Verify that performance was not degraded with respect to reference.

PAOLO: I have the following considerations:

• If we plan having a master which is very stable I would create a 'release' and a 'testing' branches, so that we merge our topic branches in 'testing', while 'release' is only updated from 'testing'
• I would make a FIFO queue for the branches, first one in the FIFO => first to get merged (also so that one can rebase his branch with the previous one in the FIFO)
• bugfixes (if they are small modifications) are placed in other branches and bypass the FIFO.