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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to