[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

Umann Kristóf via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 3 02:44:47 PDT 2018


Szelethus added a comment.

In https://reviews.llvm.org/D53974#1286434, @ztamas wrote:

> In https://reviews.llvm.org/D53974#1285930, @Szelethus wrote:
>
> > In https://reviews.llvm.org/D53974#1283759, @ZaMaZaN4iK wrote:
> >
> > > Hmm, i thought Clang has some warning for this, but I was wrong... Did you think to implement this check as Clang warning?
> >
> >
> > That is an interesting point actually -- maybe it'd be worth doing that, and if more powerful analysis is required, Static Analyzer would be the next step. I haven't actually implemented any 'regular' clang warning, so I'm not sure.
>
>
> Well, I'm implementing it as a clang-tidy check now. I guess in the future anyone can replace it with a clang warning if he/she can implement it effectively (e.g. no false positives).
>
> My first impression was that having something accepted as clang static analyzer check takes ages and so I expect that implementing something as a clang warning takes even more time. My impression is based on bugzilla activity and on some read review history. It seems to me it's not rare to have comments like: "Ping, let's not abandon this change" or the author says that he/she has no more time for further work, etc. However clang-tidy seems more progressive. So I prefer to have something as a clang-tidy check (and actually get it in the upstream tool) than implementing it as a clang warning (if it can be implemented effectively at all), wait for a year of review and most probably abandon the change. Of course, it's just a first impression, but why should I take the risk. I think this clang-tidy check is powerful, so useful to have it.


Okay, I'm sold on that :).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974





More information about the cfe-commits mailing list