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)

I think this change is worth mentioning in the release notes as well (cte/docs/ReleaseNotes.rst)

Welcome to the LLVM community and thank you for the patch lewmpk!

another ping. @alexfh and @aaron.ballman you commented on prior versions. Would be nice if you could take a (final) look at this patch!

It took me far too long to come up with an update. Honestly, I was quite demotivated as it turned out preventing name collisions of unqualifed names after the rewrite was more difficult than I thought. Especially because this error occurs sparsely when I test the check on bigger code bases. The current approach with the AST visitor seems to work best and maybe I can even extend it at some point to automatically qualify colliding names. But for now, I would be really glad if we could achieve a state of the check that you guys can accept and commit.

I am happy now :)
Thank you for the patch, LGTM

LGTM.
is there a bug or similar? If yes please mention it somewhere in the summary or so and close it :)

• be explicit about the State enumerator values

all comments resolved. I will land this now.

• [Refactor] move support classes into the analyzer
• [Refactor] move bigger methods into implementation file
• minor adjustments

ping :)

In D57662#1392509, @MyDeveloperDay wrote:

Just a question.. If clang tidy is running with -fix in parallel, what stops each clang-tidy invocation altering a common header at the same time?

LGTM

Good idea :)

• [Refactor] move support classes into the analyzer
• [Refactor] move bigger methods into implementation file

ping