Could you please run this check over LLVM and give a short report of your finding? I would imagine that there is a lot of duplication, given the include-heavy nature of big c++ code bases.

Wow, a lot of work!

Hi shixiao,

Given this revision is accepted and seems ready to go: Do you need someone to commit on your behalf?

Thans for the patch!

Thank you for the patch and sorry for the long delay.

LGTM then, i can commit for you again?

Yes someone has to do it on behalf of you.
I did so for you. After a view good patches you can get commit rights from chris lattner, until then the reviewer can commit for you.

In D61850#1500330, @svenpanne wrote:

In D61850#1500298, @JonasToth wrote:

[...] please ensure that e.g. clang-analyzer-* output looks proper as well and -quiet does, too.

OK, are there any (unit) tests which I should run? If yes, how exactly? I have a successfully built a fork of https://github.com/llvm/llvm-project via ninja locally, but the documentation for the various subprojects is still a bit hard to find to me. Is there a ninja target for clang-tidy-related tests? Any hints to get started would be highly appreciated.

This and the other output-related patches are my first submissions here, and they are intentionally simple to get the workflow right. My real intention is trying to improve/fix the readability-identifier-naming check/fixer, which still has a few issues.

LGTM

I agree with the direction, please ensure that e.g. clang-analyzer-* output looks proper as well and -quiet does, too.

Hey, I do like your check!

Did you run the new version over a big project like llvm again?
If yes LGTM from my side, if not please do that before committing to ensure no new/other problems creeped in.

My understanding of the directory layout guidance in the docs is that different styles get different directories.

Hi trixirt and thanks for the patch!

LGTM

I like the new framework!

Hey Dennis,

How does that happen to be a problem? It feels that stdout.write and stderr.write should be unconnected? (if you know the reason I would like to know it too :))
Please note, that run-clang-tidy has an export-feature for the diagnostics and fix-its which might make it easier to analyze the output of clang-tidy.

LGTM

I think it's the easiest way to specify the bits of the ineteger type to limit the catches. In real life, I met with this overflow / infinite loop problem with 16-bit short type, so I think the real use cases are 8 and 16 bit integers. It seems intuitive to me to use the size of the loop variable's type to separate those catches which can lead broken functionality in practice from those use cases which are just integer incompatibilities.

I think the idea is good and implementation, too. If we iterate all checks anyway (probably?) we could think about adding a severity to the checks, too?

I think in general this is a good direction. What do you think about other heuristics?
E.g. one could say that a vector will not contain more then X GB or similar. That is probably more complicated to figure out though.

Looks like pointless code duplication that is easily avoidable.

Why not having normal overloads? The analysis for Stmt is implemented with the private methods. Explicit template specialization is a bit overkill and so easily understood (but not too complex in this case either).l

What happens on [=]() ..., direct capture [&Columns]()... and [Columns]()...?

In D57108#1433087, @baloghadamsoftware wrote:

Ping! Any news regarding this patch?

ping :)

From my side its LGTM, but I would let @serge-sans-paille accept, as he is probably more familiar with python then I am.

What is the reason you want this change to happen? I think this gives the chance to create inconsistencies which we should avoid.

@lebedev.ri @kallehuttunen what do you think of putting this into context of the proposal (i believe its being standardized with c++20) of operator==() = default and the spaceship-operator.
This check could serve as a basis to modernize the operator== to the default if appropriate and warn/diagnose in the other cases.
WDYT?

IMHO the check is close to being finished. please address the notes and mark them as done if finished with it. So its clear to see whats outstanding.
In my opinion the user-facing documentation misses a "Limitations" sections that shows the artificial goto example, that would show that the used mechanism doesn't handle it.

LGTM

In D58731#1413888, @MyDeveloperDay wrote:

@JonasToth I left a comment in the commit needed to fix the index.rst, which I don't think your later review fixes, sphinx complained about the rst file being an unreferenced octtree

https://reviews.llvm.org/rG5fbeff797a9dba504f08f14c4fa59b6f1076fe72#inline-2691

Hmm, yeah. The laptop i have here right now does not have the dependencies installed, so yeah (you can see from all the commits i needed to do get all errors that existed).
Most of the time it works though.

Thank you for the patch!

This patch introduced a build-failure for the documentation builders (see http://lab.llvm.org:8011/builders/clang-tools-sphinx-docs/builds/38188/steps/docs-clang-tools-html/logs/stdio).
Please take a look at the buildbots after you committed something (http://lab.llvm.org:8011/console) to avoid such issues.

i have no clue if that is correct. i added some reviews from the CSA guys.

You can abondon this. A short justification (with reference to the other revision) on the bug report would be great!

LGTM.

• address review comments
• rebase to master

From my side only the nits are left.

In D58731#1413498, @MyDeveloperDay wrote:

In D58731#1413476, @lewmpk wrote:

I'm happy to land this ASAP but I don't have commit rights

So one of us could land it for you.. (I've not personally done that before as I'm a bit green too! but I do have commit rights)

Its pretty easy to get commit rights, and given your looking at multiple issues I'd recommend it.. (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)