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



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

Reply via email to