Include headers files from local to global order

[yingsu00]

Currently the Velox code guidelines state the following order for include headers groups

1. Corresponding .h for the .cpp file
2. standard lib headers
3. third party lib headers
4. Velox internal headers

However, I read many discussions about include orders online, e.g. https://stackoverflow.com/questions/2762568/c-c-include-header-file-order https://blog.knatten.org/2010/07/01/the-order-of-include-directives-matter/ and found the common practice is to have local to global order:

1. The prototype/interface header for this implementation (ie, the .h/.hh file that corresponds to this .cpp/.cc file).
2. Other headers from the same project, as needed.
3. Headers from other non-standard, non-system libraries (for example, Qt, Eigen, etc).
4. Headers from other "almost-standard" libraries (for example, Boost)
5. Standard C++ headers (for example, iostream, functional, etc.)
6. Standard C headers (for example, cstdint, dirent.h, etc.)

The main reason is to avoid hidden dependencies. E.g. if myclass.cpp includes then "myclass.h", there's no way to catch at build time that myclass.h may itself depend on string; so if later onsomeone includes myclass.h but don't need string, he will get an error that needs to be fixed either in the cpp or in the header itself. This is well explained in https://blog.knatten.org/2010/07/01/the-order-of-include-directives-matter/.

As someone mentioned in the StackOverflow discussion, It was also mentioned in "Thinking in C++" referencing Lakos' "Large Scale C++ Software Design". Also Google C++ Style Guide recommends using this order.

I think these reasoning makes a lot of sense, so would suggest we follow the local->global order and update the coding guideline.

[pedroerp]

Unless I'm missing something, Google's C++ Style Guide does not recommend the order you describe. In fact, it recommends using the one we have today:

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

The main issues as I see in changing this at this point are two:

1. Changing this moving forward would make things worse IMHO, as we would have inconsistent ordering across files. So that means we would need a giant codemod touching every single file, which could very likely end up breaking client code.
2. clang-format would still enforce the current ordering we have, as it is what is enforced by default.

Besides, it it not very evident to me how this would benefit the project. I wonder what others think.

Cc: @mbasmanova @majetideepak @xiaoxmeng @Yuhta @bikramSingh91 @duanmeng

[mbasmanova]

My reading of Google's guide is similar. It seems to match what we have today.

In dir/foo.cc or dir/foo_test.cc, whose main purpose is to implement or test the stuff in dir2/foo2.h, order your includes as follows:

dir2/foo2.h.
A blank line
C system headers (more precisely: headers in angle brackets with the .h extension), e.g., <unistd.h>, <stdlib.h>.
A blank line
C++ standard library headers (without file extension), e.g., <algorithm>, <cstddef>.
A blank line
Other libraries' .h files.
A blank line
Your project's .h files.

I also share a concern about giant codemod required to adopt new order and don't quite see the benefit that would justify such an undertaking.