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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 8 07:55:53 PST 2016


alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good after fixing capitalization in comments.

Thank you!


================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:32
@@ +31,3 @@
+                               has(Calc))
+                  .bind("Cast");
+
----------------
Err, no. I was wrong. Not all of them can be combined: the stmts and decls still need to be separate, so just ignore that comment ;)

================
Comment at: clang-tidy/misc/MisplacedWideningCastCheck.cpp:80
@@ +79,3 @@
+void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Cast = Result.Nodes.getNodeAs<ExplicitCastExpr>("Cast");
+  if (Cast->getLocStart().isMacroID())
----------------
The assert here could simplify debugging a bit in case the matchers are changed and the invariant doesn't hold any more (and the check starts crashing after this line). I don't feel strongly about the presence of an assert in similar places, but there definitely should be no `if`.

================
Comment at: docs/clang-tidy/checks/misc-misplaced-widening-cast.rst:2
@@ +1,3 @@
+.. title:: clang-tidy - misc-misplaced-widening-cast
+
+misc-misplaced-widening-cast
----------------
danielmarjamaki wrote:
> For information, it seem I had to use -DLLVM_ENABLE_SPHINX=ON also ..
Yes, thanks. Apparently, I haven't found all relevant flags in ccmake.

================
Comment at: test/clang-tidy/misc-misplaced-widening-cast.cpp:32
@@ +31,3 @@
+  // the result is a 9 bit value, there is no truncation in the implicit cast.
+  l = (long)(a + 15);
+  // the result is a 12 bit value, there is no truncation in the implicit cast.
----------------
You missed the "capitalization" part ;)


http://reviews.llvm.org/D16310





More information about the cfe-commits mailing list