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

[Richard via cfe-commits]

LegalizeAdulthood added a comment.

In http://reviews.llvm.org/D16310#332200, @danielmarjamaki wrote:

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


If this has universally been your experience on a code base, then I say you should count your blessings that you have worked only with such good programmers!

Sadly, I have encountered many code bases where such silly things were written.  Sometimes people just write the cast because that is the return type of the function.

At any rate, the check simply cannot divine programmer intent from the source code alone, which is why for clang-tidy (not static analyzer) purposes, my expectation is that clang-tidy would say "this cast is redundant, so I suggest you remove it".  Hence the fixit removes the cast.

Only a human being can look at the change suggested by clang-tidy and say "hmm.... looks like the correct thing here is that there was some loss of precision in the intermediate result, so I should fix that instead".

If a developer is going to blindly accept the output of clang-tidy, then there is nothing we can do about that.  (A case in point: I am currently reviewing thousands of changes proposed by google-readability-braces-around-statements; it has quite a few bugs and can actually generate syntactically invalid code from its suggested fixits!)  At a bare minimum, clang-tidy should never offer fixits that change the meaning of the code and my suggested fixit does not do that.  The other proposed fixits discussed on this thread **do** change the meaning of the code by changing the precision of the intermediate calculations and are therefore not refactorings.  They are bug fixes.  Without dynamic analysis, only a human can decide on their validity by considering the larger context of the code.

Again, my feeling is that clang-tidy should be a refactoring tool when it comes to fixits -- the suggested changes should never change the semantic meaning of the code.


Repository:
  rL LLVM

http://reviews.llvm.org/D16310