[PATCH] D18821: Add modernize-bool-to-integer-conversion
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 26 07:04:03 PDT 2016
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
In http://reviews.llvm.org/D18821#403117, @Prazek wrote:
> In http://reviews.llvm.org/D18821#402686, @Prazek wrote:
>
> > In http://reviews.llvm.org/D18821#398843, @alexfh wrote:
> >
> > > BTW, why is the check in the 'modernize' module? It doesn't seem to make anything more modern. I would guess, the pattern it detects is most likely to result from a programming error. Also, the fix, though it retains the behavior, has a high chance to be incorrect. Can you share the results of running this check on LLVM? At least, how many problems it found and how many times the suggested fix was correct.
> > >
> > > I'd suggest to move the check to `misc` or maybe it's time to create a separate directory for checks targeting various bug-prone patterns.
> >
> >
> > Do you have any thought about the name for such a module? I belive that misc is overloaded.
> >
> > So for this we are looking for something that is probably not a bug, but it makes code a little bit inaccurate
> > Maybe something like:
> >
> > - accuracy,
> > - correctness,
> > - certainity,
> > - safety,
> > - maybebugmaybenothardtosay
>
>
> after a long though I think that "accuracy" is the best name here - we want to look for a code that is valid, but not accurate
There are many possible reasons this pattern can appear in the code. Sometimes it's a bug, sometimes, it's an attempt to make the code look better than it is ;) It seems to me though, that having a type mismatch of this kind is a bug-prone pattern, so I would start a more generic "bugprone" category of checks (and move some misc- checks there later on).
Also, please re-upload http://reviews.llvm.org/D19105 after disabling matches on 1-bit bitfields in the check.
http://reviews.llvm.org/D18821
More information about the cfe-commits
mailing list