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

[Richard via cfe-commits]

LegalizeAdulthood added inline comments.

================
Comment at: clang-tidy/misc/LongCastCheck.cpp:97
@@ +96,3 @@
+
+  if (!CastType->isIntegerType() || !SubType->isIntegerType())
+    return;
----------------
danielmarjamaki wrote:
> LegalizeAdulthood wrote:
> > Why don't you check for casting a `float` expression to a `double` or `long double`?
> > 
> > Isn't this the exact same issue?
> > 
> > If so, add a test case for casting a `float` expression to `double` and a test case for casting a `double` expression to a `long double`.
> in theory yes.. but somehow that feels strange to me. yes there will possibly be loss of precision in some decimals, that is normal when using floating point numbers. if such loss of precision would be unwanted then float should be avoided to start with.
> 
> so I do agree in theory but I don't think I would feel good about adding such warnings.
> 
For floating-point quantities, when I think of the term "precision" I am thinking of the number of bits allocated to the mantissa.  This may or may not be correct terminology according to floating-point experts, but from what I can tell it seems to agree with how the term is used on wikipedias [[ https://en.wikipedia.org/wiki/Floating_point | article on floating-point ]].

So while digits are technically lost when we add a small floating-point quantity to a large floating-point quantity (the large quantity gobbles up all the bits of the mantissa and the small quantity has its least-significant mantissa bits discarded), the precision of the result isn't changed -- the number of bits in the mantissa is the same for the result as it was for the inputs.

To then take a quantity of N bits of mantissa and cast that to a quantity of M bits of mantissa where M > N seems just as pointless as the approach of casting an `int` to a `long`.  The extra tokens for casting do absolutely nothing and are redundant as far as the computation as written is concerned.

================
Comment at: clang-tidy/misc/MiscTidyModule.cpp:61
@@ -59,1 +60,3 @@
+    CheckFactories.registerCheck<LongCastCheck>(
+        "misc-long-cast");
     CheckFactories.registerCheck<MacroParenthesesCheck>(
----------------
danielmarjamaki wrote:
> alexfh wrote:
> > danielmarjamaki wrote:
> > > LegalizeAdulthood wrote:
> > > > The documentation describes this check as one that looks for a cast to a "bigger type", but the name of the check implies that it only works for casts to `long`.
> > > > 
> > > > The name of the check should be made more generic to reflect reality.
> > > > 
> > > > Perhaps `misc-redundant-cast-to-larger-type` or `misc-redundant-bigger-type-cast`?
> > > Yes I agree.. will fix..
> > > 
> > > I used long because that is the typical case that I envision.
> > How about misc-misplaced-widening-cast?
> I already changed.. but I like misc-misplaced-widening-cast better so I will change again..
Yes, Alex's name suggestion is better than mine.  Yay code reviews!


Repository:
  rL LLVM

http://reviews.llvm.org/D16310