[LLVMdev] sample of running google c++ lint script

[David Blaikie]

On Tue, Jun 5, 2012 at 8:43 AM, Reed Kotler <rkotler at mips.com> wrote:
> Did you agree with the comment about the use of long long from the tool?
>
> Anyway, it's not really important to me that we adopt any specific
> google code rules over the
> current llvm rule.
>
> The point is to that Google has a deeper set of conventions and it would
> be a good starting point for us.
> Also, they have a tool which checks a lot of this. The lack of a tool
> for llvm style check is what originally caught my attention in this
> area. I don't like doing work that a robot can do for me.
>
> The Google tool has a wish list of other things that I would not be
> opposed to personally adding
> should we go that way. The point is to collect the kinds of style errors
> that send a patch back to the author and to try and implement those in
> the style checker. I would even open up a bug against the
> style checker for each such thing that is not checked.
>
> Not wanting to clean up the white space is exactly a simple but good
> example of technical debt that we are incurring.
>
> In that case it's very simple to see. We have a rule about that for our
> style and because we are too
> busy with other things, then we have allowed the technical debt to rise
> to a point where we don't
> want to ever pay it, or so you say.

That's sort of a misunderstanding of the issue - it's not that we're
too busy. It's that the tools (version control mostly) don't support
this use case very well. If it were just about not having the time, it
would be easy for some newbie contributors to go through & cleanup the
whole code base - such patches are not accepted, though, because of
the VCS churn. I don't entirely agree there - tools already have some
support, if there are other tools that don't, we could look into what
it'd take to improve/replace/etc. But that's what it is today.

>
> When I ran a script over the Target subtree, my simple check showed
> 20,000 violations of tab, line length and spaces at the end of line rules.

& what exactly is the cost there? in most cases it's just a few
characters over the limit here & there - I'm not much of a stickler
for those rules anyway. It seems the major benefit would be if we
actually had a tool that autoformatted/enforced those guidelines -
then it'd be potentially worth the cleanup to get us into a good state
from which we could never diverge. But better than that would be a
tool that could just ratchet up the quality by not allowing us to
regress without forcing us to fix everything up front. Such a tool is
more complicated to build/use, so it's a tradeoff between whether it's
better to fix everything or create/use such a tool.

>
>
> On 06/05/2012 04:50 AM, Sebastian Redl wrote:
>> On 05.06.2012 02:56, reed kotler wrote:
>>> Just as an example, I picked totally at random, one c++ program to run
>>> the google code style checker.
>>>
>>> There are clearly some valid points it found. I think it would good to
>>> start to adapt this tool
>>> or write a new tool to do style checking and to start to better
>>> formalize the llvm rules.
>>>
>>> I ran it against Target/Target.cpp
>>>
>>> Target.cpp:10:  Line ends in whitespace.  Consider deleting these extra
>>> spaces.  [whitespace/end_of_line] [4]
>> We have an incredible amount of those, and fixing them would create far
>> too much churn. I generally fix them on the code I touch.
>>> Target.cpp:22:  Found C++ system header after other header. Should be:
>>> Target.h, c system, c++ system, other.  [build/include_order] [4]
>> The LLVM convention is to reverse the "usual" order. The first file any
>> .cpp includes should be its own .h. Then other project headers. Then, in
>> the case of Clang, the LLVM headers. And finally system and standard
>> headers.
>>> Target.cpp:24:  Do not use namespace using-directives.  Use
>>> using-declarations instead.  [build/namespaces] [5]
>> LLVM convention is to use a using directive for the project namespace.
>>> Target.cpp:26:  Is this a non-const reference? If so, make const or use
>>> a pointer.  [runtime/references] [2]
>> What? Why?
>>
>> Sebastian
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev