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

Richard via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 20 13:42:56 PST 2016


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





More information about the cfe-commits mailing list