[PATCH] D16310: new clang-tidy checker misc-long-cast

Daniel Marjamäki via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 21 05:58:30 PST 2016


danielmarjamaki added a comment.

In http://reviews.llvm.org/D16310#331538, @LegalizeAdulthood wrote:

> If you state what the check does, then
>
> In http://reviews.llvm.org/D16310#331054, @danielmarjamaki wrote:
>
> > In http://reviews.llvm.org/D16310#330367, @LegalizeAdulthood wrote:
> >
> > > Why not supply a fixit that removes the cast?
> >
> >
> > I am skeptic. There are different valid fixes.
> >
> > Example code:
> >
> >   l = (long)(a*1000);
> >   
> >
> > Fix1:
> >
> >   l = ((long)a * 1000);
> >   
> >
> > Fix2:
> >
> >   l = (a * (long)1000);
> >   
> >
> > Fix3:
> >
> >   l = (a * 1000L);
>
>
> The way I see it, the check is complaining about the pointless cast and pointing the finger at the beginning of the cast.  To me, my expectation is that the suggested fix is none of the options you gave but instead:
>
>   l = (a*1000);
>
>
> In other words, the cast is superfluous for the code as written.  Omitting the cast directly expresses the code as written without unnecessary casting.  Because the check can't know your intent, it hints that you may have intended something else, but for what was written, this **is** the semantics.
>
> If we want to get into detecting numeric overflow, then we're talking some kind of run-time assisted check and it's beyond the scope of clang-tidy.
>
> Basically, I look at this unnecessary cast as clutter.  I'm fine with static analysis that warns about intermediate expressions possibly overflowing when assigned to a "larger" type, but to me that sounds like something for the static analyzer and not for tidy.
>
> In other words, I think of clang-tidy as a refactoring tool, not a static analysis tool.


Why is there a cast in the first place? It is unlikely that the programmer added a useless cast for no reason.

I say that it looks like the programmer wanted to avoid loss of precision.

If we remove the useless cast there will still be loss of precision. And the warnings go away so maybe a sloppy programmer thinks that all is good.

> I tested this on debian projects.. and I see quite little noise. in 1212 projects I got 76 warnings.


In my opinion the proper fix for these are to cast the expressions properly.


Repository:
  rL LLVM

http://reviews.llvm.org/D16310





More information about the cfe-commits mailing list