You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mxnet.apache.org by Pedro Larroy <pe...@gmail.com> on 2019/01/08 22:44:21 UTC
Order of includes in cpplint
Hi MXNet community
cpplint seems to complain when the order of includes is not [own,
system, other]
But the general best practice in C++ is [own, project, 3rd party,
system] for the reasons explained in this stackoverflow answer: (
https://stackoverflow.com/questions/614302/c-header-order )
A contribution to cpplint could be made to make this configurable:
https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
Thoughts?
Pedro.
RE: Order of includes in cpplint
Posted by "Qin, Zhennan" <zh...@intel.com>.
Ah, I see the problem. The real problem is, we shouldn't use diamond include instead of quotes include for project header, which is cheating cpplint to consider them as system header. Diamond include should only be used for system header, including OS header, STL header and host-installed library header(opencv for example). For others, quote include should be used.
Thanks,
Zhennan
-----Original Message-----
From: Pedro Larroy [mailto:pedro.larroy.lists@gmail.com]
Sent: Wednesday, January 9, 2019 9:29 AM
To: dev@mxnet.incubator.apache.org
Subject: Re: Order of includes in cpplint
I worked around the case, I saw several cases where project and third party headers were included with diamond includes instead of quotes.
Agree I think is too much hassle to change the order of includes in the whole project anyway. But this convention can cause some headers not to be self sufficient as system headers are being included before other library headers which might be using a system header like <string> without including it themselves.
Pedro.
On Wed, Jan 9, 2019 at 2:12 AM Qin, Zhennan <zh...@intel.com> wrote:
>
> Hi Pedro,
>
> Interesting topic. Google style does have guidance for this:
>
> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_I
> ncludes
>
> According to it, the order is,
>
> dir2/foo2.h.
> A blank line
> C system files.
> C++ system files.
> A blank line
> Other libraries' .h files.
> Your project's .h files.
>
> As MXNet follows this style, I guess we shouldn't break it unless we have some problems. Do you have such a case that need the change?
>
> Thanks,
> Zhennan
>
> -----Original Message-----
> From: Pedro Larroy [mailto:pedro.larroy.lists@gmail.com]
> Sent: Wednesday, January 9, 2019 6:44 AM
> To: dev@mxnet.incubator.apache.org
> Subject: Order of includes in cpplint
>
> Hi MXNet community
>
> cpplint seems to complain when the order of includes is not [own,
> system, other]
>
> But the general best practice in C++ is [own, project, 3rd party,
> system] for the reasons explained in this stackoverflow answer: (
> https://stackoverflow.com/questions/614302/c-header-order )
>
> A contribution to cpplint could be made to make this configurable:
>
> https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
>
> Thoughts?
>
> Pedro.
Re: Order of includes in cpplint
Posted by Pedro Larroy <pe...@gmail.com>.
I worked around the case, I saw several cases where project and third
party headers were included with diamond includes instead of quotes.
Agree I think is too much hassle to change the order of includes in
the whole project anyway. But this convention can cause some headers
not to be self sufficient as system headers are being included before
other library headers which might be using a system header like
<string> without including it themselves.
Pedro.
On Wed, Jan 9, 2019 at 2:12 AM Qin, Zhennan <zh...@intel.com> wrote:
>
> Hi Pedro,
>
> Interesting topic. Google style does have guidance for this:
>
> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
>
> According to it, the order is,
>
> dir2/foo2.h.
> A blank line
> C system files.
> C++ system files.
> A blank line
> Other libraries' .h files.
> Your project's .h files.
>
> As MXNet follows this style, I guess we shouldn't break it unless we have some problems. Do you have such a case that need the change?
>
> Thanks,
> Zhennan
>
> -----Original Message-----
> From: Pedro Larroy [mailto:pedro.larroy.lists@gmail.com]
> Sent: Wednesday, January 9, 2019 6:44 AM
> To: dev@mxnet.incubator.apache.org
> Subject: Order of includes in cpplint
>
> Hi MXNet community
>
> cpplint seems to complain when the order of includes is not [own, system, other]
>
> But the general best practice in C++ is [own, project, 3rd party, system] for the reasons explained in this stackoverflow answer: ( https://stackoverflow.com/questions/614302/c-header-order )
>
> A contribution to cpplint could be made to make this configurable:
>
> https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
>
> Thoughts?
>
> Pedro.
RE: Order of includes in cpplint
Posted by "Qin, Zhennan" <zh...@intel.com>.
Hi Pedro,
Interesting topic. Google style does have guidance for this:
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
According to it, the order is,
dir2/foo2.h.
A blank line
C system files.
C++ system files.
A blank line
Other libraries' .h files.
Your project's .h files.
As MXNet follows this style, I guess we shouldn't break it unless we have some problems. Do you have such a case that need the change?
Thanks,
Zhennan
-----Original Message-----
From: Pedro Larroy [mailto:pedro.larroy.lists@gmail.com]
Sent: Wednesday, January 9, 2019 6:44 AM
To: dev@mxnet.incubator.apache.org
Subject: Order of includes in cpplint
Hi MXNet community
cpplint seems to complain when the order of includes is not [own, system, other]
But the general best practice in C++ is [own, project, 3rd party, system] for the reasons explained in this stackoverflow answer: ( https://stackoverflow.com/questions/614302/c-header-order )
A contribution to cpplint could be made to make this configurable:
https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
Thoughts?
Pedro.
Re: Order of includes in cpplint
Posted by Tianqi Chen <tq...@cs.washington.edu>.
On Tue, Jan 8, 2019 at 2:44 PM Pedro Larroy <pe...@gmail.com>
wrote:
> Hi MXNet community
>
> cpplint seems to complain when the order of includes is not [own,
> system, other]
>
> But the general best practice in C++ is [own, project, 3rd party,
> system] for the reasons explained in this stackoverflow answer: (
> https://stackoverflow.com/questions/614302/c-header-order )
>
> A contribution to cpplint could be made to make this configurable:
>
> https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
>
> Thoughts?
>
> Pedro.
>
Re: Order of includes in cpplint
Posted by Tianqi Chen <tq...@cs.washington.edu>.
I think this is a nit and it is hard to argue which one is best, everyone
has their own preference.
In this case, Google C style is a simple convention that everyone can refer
to, and I would suggest we just stick with that convention
Tianqi
On Tue, Jan 8, 2019 at 2:44 PM Pedro Larroy <pe...@gmail.com>
wrote:
> Hi MXNet community
>
> cpplint seems to complain when the order of includes is not [own,
> system, other]
>
> But the general best practice in C++ is [own, project, 3rd party,
> system] for the reasons explained in this stackoverflow answer: (
> https://stackoverflow.com/questions/614302/c-header-order )
>
> A contribution to cpplint could be made to make this configurable:
>
> https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
>
> Thoughts?
>
> Pedro.
>
Re: Order of includes in cpplint
Posted by Chris Olivier <cj...@gmail.com>.
opinions vary. there’s arguments the other way, too, such as an ideal
precompiled header compiler would work best with invariant headers first as
well as template matching order in some cases
On Tue, Jan 8, 2019 at 2:44 PM Pedro Larroy <pe...@gmail.com>
wrote:
> Hi MXNet community
>
> cpplint seems to complain when the order of includes is not [own,
> system, other]
>
> But the general best practice in C++ is [own, project, 3rd party,
> system] for the reasons explained in this stackoverflow answer: (
> https://stackoverflow.com/questions/614302/c-header-order )
>
> A contribution to cpplint could be made to make this configurable:
>
> https://github.com/cpplint/cpplint/blob/master/cpplint.py#L605
>
> Thoughts?
>
> Pedro.
>