LegalizeAdulthood added a comment.

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.


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