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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to