Taichi: Code formats and how to enforce them

[yuanming-hu]

We are currently making use of automatic tools (clang-format and yapf) to (weakly) enforce C++ and Python code format. Starting from v0.6.0, we want to strictly enforce code format. A few topics on this:

Actions

• We are considering adding a githook to make sure any modified file is processed by clang-format and yapf
  
  
  
  • Pro: code will always get formatted before committing
  
  
  • Con: every developer has to install the tools. Also git commit may take a few seconds longer
  
  
• Is there an available online "CI" service that check for code format? If so we should also set that up in case anyone's code is not properly formatted

Code styles

(It's hard to come up with a code style that makes everybody happy and I believe as long as we follow a set of consistent rules, then we are good.)

C++: we are using a clang-format profile that mostly follows the 'Chromium` code style

Header orders (following Google's C++ style guide) as suggested by @k-ye:

• #include <...> only for standard libraries
• #include "..." for everything else. No relative paths allowed: even Taichi headers must use the full path starting with "taichi/..."

Python: The python code format is simply Chromium style

I suggest that we just stick to these styles unless there's a solution that's significantly better. What do you guys think? :-)

Additional comments

Should we also make use of linting tools such as cpplint to statically check for potential bugs/bad patterns?

Inputs are welcome!

[archibate]

Usually people can do style well with self-dicipiline, having a githook on this really just slow down time for us, if we want we can run ti format by hand.
For newcomers, we may want to add this style infomation to Contribution Guideline? If one don't follows the style, we reviewers can run ti format before merge, and mind them for the next time :)

[archibate]

instead of giving https://github.com/taichi-dev/taichi/blob/master/.clang-format, I think have some sample codes is more friendly to people:

int foo() { // all `{`'s follow last line, function name is in the same line of return type declaration
  int x = 1; // tab=2, expanded in space
  if (x == 4) // space after controling keywords like `if`
    return x + 233; // having spaces before and after operators
}

[yuanming-hu]

Yeah, being self-discipline is good yet inevitably people (including me myself) may accidentally commit style-violating code. Actually, a well-done git hook should work on the files changed only, and will not take more than 5 seconds per commit. So I would still lean towards having a githook.

[yuanming-hu]

instead of giving https://github.com/taichi-dev/taichi/blob/master/.clang-format, I think have some sample codes is more friendly to people

Yeah maybe we should also have a minimal sample file in Contributor Guidelines. CLion has a small piece of code that covers many C++ constructs, which we can make use of:

#if !defined(OS)
#define OS_NOT_DEFINED
#endif

/*********************************************
* ...globalFunc...
*********************************************/
void globalFunc();

namespace foo {
  /**
  ...Foo...
  */
  class Foo {
  public:
    Foo();

    ~Foo();

    virtual Foo *getSelf() { return Foo::getSelf(); }

  private:
    void innerFunc();

    int var;
  };
}
struct FooPOD {
#ifdef OS_NOT_DEFINED
#define OS "unknown"
#endif
#define FooPOD_OS OS
  int i;
};
struct FooC {
private:
  int i;
};
extern int a;

static int innerFunc();

int a = innerFunc();

int innerFunc() { return 5; }

void foo::Foo::innerFunc() {
  label1:
  int continuation = 0xCD
                     + 0xFD + 0xBAADF00D + 0xDEADBEEF;
  auto la = [](int i1, int i2) -> bool mutable {
    label2:
    return i1 < i2;
  }(1, 2);
}

[yuanming-hu]

we reviewers can run ti format before merge, and mind them for the next time :)

That's a good idea - we don't want code styles to stop people from contributing.

[archibate]

#ifdef/ifndef x, #if !x or #if !defined(x)? Please choose one, it seems we are mainly ifndef for now.
Was virtual Foo *getSelf() { return Foo::getSelf(); } expected? Less than 80 chars so it can be inlined?

label1:
  int continuation = 0xCD

Some compilers don't like labels to be followed by variable declaration, please tell them to use label1:; instead, or completely give up goto (which is used by many devs).

[yuanming-hu]

#ifdef/ifndef x, #if !x or #if !defined(x)? Please choose one, it seems we are mainly ifndef for now.
Was virtual Foo *getSelf() { return Foo::getSelf(); } expected? Less than 80 chars so it can be inlined?

label1:
  int continuation = 0xCD

Some compilers don't like labels to be followed by variable declaration, please tell them to use label1:; instead, or completely give up goto (which is used by many devs).

Let's use #if !defined(x).

This is just a piece of sample code trying to cover as many language constructs as possible. Clearly we don't need every part, for example, the labels for goto.

[yuanming-hu]

It seems that we can make use of Github actions to automatically check formats of PRs: https://github.com/marketplace/actions/clang-format-lint

[yuanming-hu]

Merged into #592