LGTM.
But i was inactive for a long time, if @aaron.ballman accepts as well you can commit instantly. Otherwise please let the other people that commented some time to react. :)

Do you know who is responsible for it? Because i haven't worked with documentation before and don't know what i need to do to update it.

The new switch needs documentation as well, and maybe even a note in the release notes (as it is publicly discussed as issue?).
Otherwise I am fine with the changes!

In D68694#1701772, @aaron.ballman wrote:

In my opinion this check should be disabled in case of integer literals, since there are a lot of existing code (even in system libraries) where user can do nothing, e.g.:

I think that this check should behave how the coding standard dictates it behaves. By my reading of HIC++, the check is behaving by design. I think there are a few ways forward though (not mutually exclusive):

0) See if the HIC++ folks are interested in clarifying their standard for integer literals and if they are, modify the check

1. Add an off-by-default option that suppresses the diagnostic with positive integer literals so that users can explicitly decide to deviate locally
2. Document that this happens and is expected behavior; users can disable the check if they don't wish to conform to that standard.

Sorry for the long delay!

In D64671#1606084, @jpakkane wrote:

In D64671#1603427, @alexfh wrote:

A general comment: "misc" is a sort of a heap of checks that otherwise don't have a good home. This one would probably better go to bugprone (or maybe there's a relevant CERT or C++ Core Guidelines rule?).

The closest I could find is ES.20 "Always initialize an object": https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es20-always-initialize-an-object

So a potential name for this could be "cppcoreguidelines-init-variables". However that rule is about initializing all variables, even those in structs and classes. This is about local variables.

Just as Sidenote in case you were not aware: There is a check in clang-tidy for gsl::owner (https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-owning-memory.html)
It will benefit from your annotation and can be simplified.

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.