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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
