danielmarjamaki marked an inline comment as done.
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:21
@@ +20,3 @@
+ Finder->addMatcher(
+ returnStmt(
+ has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
----------------
alexfh wrote:
> Any reason to limit this to returnStmt, varDecl and assignment? This pattern
> can appear in any expression and lead to equally incorrect results.
Yes. There could be some extra pattern we want to look for later. But I don't
want to warn in general. Example:
int A,B,C;
long ABC = (long)(A * B) + C;
That code makes perfect sense if the calculation A*B won't overflow and you
want that the addition is done with long precision.
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:22
@@ +21,3 @@
+ returnStmt(
+ has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
+ hasOperatorName("*"),
----------------
alexfh wrote:
> Why only c-style casts? The problem applies to static_cast as well. Not sure
> how likely a reinterpret_cast is to appear in this situation, maybe it should
> be handled as well.
ok, I will handle that too in next patch.
================
Comment at: clang-tidy/misc/LongCastCheck.cpp:23
@@ +22,3 @@
+ has(cStyleCastExpr(has(binaryOperator(anyOf(hasOperatorName("+"),
+ hasOperatorName("*"),
+ hasOperatorName("<<")))))
----------------
alexfh wrote:
> Why other operators are not considered here? Subtraction, for example, can
> suffer the same problem.
yes. true. the ~ operator also. I can't say how noisy that would be but I will
test it.
================
Comment at: clang-tidy/misc/MiscTidyModule.cpp:61
@@ -59,1 +60,3 @@
+ CheckFactories.registerCheck<LongCastCheck>(
+ "misc-long-cast");
CheckFactories.registerCheck<MacroParenthesesCheck>(
----------------
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..
Repository:
rL LLVM
http://reviews.llvm.org/D16310
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
