danielmarjamaki marked an inline comment as done.
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:43
@@ +42,3 @@
+
+static unsigned getMaxCalculationWidth(ASTContext &C, const Expr *E) {
+ E = E->IgnoreParenImpCasts();
----------------
LegalizeAdulthood wrote:
> Prefer anonymous namespace over `static` to scope visibility.
As I read the LLVM coding standards we prefer static for functions.
http://llvm.org/docs/CodingStandards.html#anonymous-namespaces
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:97
@@ +96,3 @@
+
+ if (!CastType->isIntegerType() || !SubType->isIntegerType())
+ return;
----------------
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.
================
Comment at: clang-tidy/misc/MiscTidyModule.cpp:61
@@ -59,1 +60,3 @@
+ CheckFactories.registerCheck<LongCastCheck>(
+ "misc-long-cast");
CheckFactories.registerCheck<MacroParenthesesCheck>(
----------------
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.
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
