<div dir="ltr"><pre style="color:rgb(0,0,0)">Added a comment in r212033.</pre></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, Jun 29, 2014 at 11:56 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm not particularly opposed to duplicating -Wold-style-cast in clang-tidy using ASTMatches because if nothing else, it's interesting to compare and contrast the two methods.<br>
<br>
But please do add a brief comment explaining how this new check relates to the existing warning option in clang and whether the output locations are expected to match precisely, or if they differ, why that is.<br>
<br>
Thanks<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
On 30/06/2014 01:19, Alexander Kornienko wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: alexfh<br>
Date: Sun Jun 29 17:19:53 2014<br>
New Revision: 212002<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=212002&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=212002&view=rev</a><br>
Log:<br>
Another attempt to add a clang-tidy check for flagging C-style casts.<br>
<br>
Summary:<br>
The first version failed the SubstNonTypeTempateParmExpr-<u></u>related test<br>
on some buildbots. This one uses the new substNonTypeTempateParmExpr matcher to<br>
filter out implicit C-style casts.<br>
<br>
This patch depends on D4327.<br>
<br>
Reviewers: djasper<br>
<br>
Reviewed By: djasper<br>
<br>
Subscribers: aemerson, cfe-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D4328" target="_blank">http://reviews.llvm.org/D4328</a><br>
<br>
Added:<br>
clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.cpp<br>
clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.h<br>
clang-tools-extra/trunk/test/<u></u>clang-tidy/avoid-c-style-<u></u>casts.cpp<br>
Modified:<br>
clang-tools-extra/trunk/clang-<u></u>tidy/google/CMakeLists.txt<br>
clang-tools-extra/trunk/clang-<u></u>tidy/google/GoogleTidyModule.<u></u>cpp<br>
<br>
Added: clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp?rev=212002&view=auto" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/clang-tools-extra/<u></u>trunk/clang-tidy/google/<u></u>AvoidCStyleCastsCheck.cpp?rev=<u></u>212002&view=auto</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.cpp (added)<br>
+++ clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.cpp Sun Jun 29 17:19:53 2014<br>
@@ -0,0 +1,52 @@<br>
+//===--- AvoidCStyleCastsCheck.cpp - clang-tidy -----------------*- C++ -*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===------------------------<u></u>------------------------------<u></u>----------------===//<br>
+<br>
+#include "AvoidCStyleCastsCheck.h"<br>
+#include "clang/ASTMatchers/<u></u>ASTMatchFinder.h"<br>
+#include "clang/ASTMatchers/<u></u>ASTMatchers.h"<br>
+<br>
+using namespace clang::ast_matchers;<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+namespace readability {<br>
+<br>
+void<br>
+AvoidCStyleCastsCheck::<u></u>registerMatchers(ast_matchers:<u></u>:MatchFinder *Finder) {<br>
+ Finder->addMatcher(<br>
+ cStyleCastExpr(<br>
+ // Filter out (EnumType)IntegerLiteral construct, which is generated<br>
+ // for non-type template arguments of enum types.<br>
+ // FIXME: Remove this once this is fixed in the AST.<br>
+ unless(hasParent(<u></u>substNonTypeTemplateParmExpr()<u></u>))).bind("cast"),<br>
+ this);<br>
+}<br>
+<br>
+void AvoidCStyleCastsCheck::check(<u></u>const MatchFinder::MatchResult &Result) {<br>
+ const auto *CastExpr = Result.Nodes.getNodeAs<<u></u>CStyleCastExpr>("cast");<br>
+<br>
+ // Ignore casts in macros for now.<br>
+ if (CastExpr->getLocStart().<u></u>isMacroID())<br>
+ return;<br>
+<br>
+ // Casting to void is an idiomatic way to mute "unused variable" and similar<br>
+ // warnings.<br>
+ if (CastExpr->getTypeAsWritten()-<u></u>>isVoidType())<br>
+ return;<br>
+<br>
+ diag(CastExpr->getLocStart(), "C-style casts are discouraged. Use "<br>
+ "static_cast/const_cast/<u></u>reinterpret_cast "<br>
+ "instead.");<br>
+ // FIXME: Suggest appropriate C++ cast. See [expr.cast] for cast notation<br>
+ // semantics.<br>
+}<br>
+<br>
+} // namespace readability<br>
+} // namespace tidy<br>
+} // namespace clang<br>
<br>
Added: clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.h?rev=212002&view=auto" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/clang-tools-extra/<u></u>trunk/clang-tidy/google/<u></u>AvoidCStyleCastsCheck.h?rev=<u></u>212002&view=auto</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.h (added)<br>
+++ clang-tools-extra/trunk/clang-<u></u>tidy/google/<u></u>AvoidCStyleCastsCheck.h Sun Jun 29 17:19:53 2014<br>
@@ -0,0 +1,33 @@<br>
+//===--- AvoidCStyleCastsCheck.h - clang-tidy -------------------*- C++ -*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===------------------------<u></u>------------------------------<u></u>----------------===//<br>
+<br>
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_<u></u>TIDY_GOOGLE_AVOID_C_STYLE_<u></u>CASTS_CHECK_H<br>
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_<u></u>TIDY_GOOGLE_AVOID_C_STYLE_<u></u>CASTS_CHECK_H<br>
+<br>
+#include "../ClangTidy.h"<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+namespace readability {<br>
+<br>
+/// \brief Finds usages of C-style casts.<br>
+///<br>
+/// <a href="http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Casting#Casting" target="_blank">http://google-styleguide.<u></u>googlecode.com/svn/trunk/<u></u>cppguide.xml?showone=Casting#<u></u>Casting</a><br>
+/// Corresponding cpplint.py check name: 'readability/casting'.<br>
+class AvoidCStyleCastsCheck : public ClangTidyCheck {<br>
+public:<br>
+ void registerMatchers(ast_matchers:<u></u>:MatchFinder *Finder) override;<br>
+ void check(const ast_matchers::MatchFinder::<u></u>MatchResult &Result) override;<br>
+};<br>
+<br>
+} // namespace readability<br>
+} // namespace tidy<br>
+} // namespace clang<br>
+<br>
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_<u></u>TIDY_GOOGLE_AVOID_C_STYLE_<u></u>CASTS_CHECK_H<br>
<br>
Modified: clang-tools-extra/trunk/clang-<u></u>tidy/google/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=212002&r1=212001&r2=212002&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/clang-tools-extra/<u></u>trunk/clang-tidy/google/<u></u>CMakeLists.txt?rev=212002&r1=<u></u>212001&r2=212002&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- clang-tools-extra/trunk/clang-<u></u>tidy/google/CMakeLists.txt (original)<br>
+++ clang-tools-extra/trunk/clang-<u></u>tidy/google/CMakeLists.txt Sun Jun 29 17:19:53 2014<br>
@@ -1,6 +1,7 @@<br>
set(LLVM_LINK_COMPONENTS support)<br>
add_clang_library(<u></u>clangTidyGoogleModule<br>
+ AvoidCStyleCastsCheck.cpp<br>
ExplicitConstructorCheck.cpp<br>
GoogleTidyModule.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clang-<u></u>tidy/google/GoogleTidyModule.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=212002&r1=212001&r2=212002&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/clang-tools-extra/<u></u>trunk/clang-tidy/google/<u></u>GoogleTidyModule.cpp?rev=<u></u>212002&r1=212001&r2=212002&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- clang-tools-extra/trunk/clang-<u></u>tidy/google/GoogleTidyModule.<u></u>cpp (original)<br>
+++ clang-tools-extra/trunk/clang-<u></u>tidy/google/GoogleTidyModule.<u></u>cpp Sun Jun 29 17:19:53 2014<br>
@@ -10,6 +10,7 @@<br>
#include "../ClangTidy.h"<br>
#include "../ClangTidyModule.h"<br>
#include "../ClangTidyModuleRegistry.h"<br>
+#include "AvoidCStyleCastsCheck.h"<br>
#include "ExplicitConstructorCheck.h"<br>
using namespace clang::ast_matchers;<br>
@@ -23,6 +24,9 @@ public:<br>
CheckFactories.<u></u>addCheckFactory(<br>
"google-explicit-constructor",<br>
new ClangTidyCheckFactory<<u></u>ExplicitConstructorCheck>());<br>
+ CheckFactories.<u></u>addCheckFactory(<br>
+ "google-readability-casting",<br>
+ new ClangTidyCheckFactory<<u></u>readability::<u></u>AvoidCStyleCastsCheck>());<br>
}<br>
};<br>
<br>
Added: clang-tools-extra/trunk/test/<u></u>clang-tidy/avoid-c-style-<u></u>casts.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/avoid-c-style-casts.cpp?rev=212002&view=auto" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/clang-tools-extra/<u></u>trunk/test/clang-tidy/avoid-c-<u></u>style-casts.cpp?rev=212002&<u></u>view=auto</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- clang-tools-extra/trunk/test/<u></u>clang-tidy/avoid-c-style-<u></u>casts.cpp (added)<br>
+++ clang-tools-extra/trunk/test/<u></u>clang-tidy/avoid-c-style-<u></u>casts.cpp Sun Jun 29 17:19:53 2014<br>
@@ -0,0 +1,24 @@<br>
+// RUN: clang-tidy -checks=-*,google-readability-<u></u>casting %s -- | FileCheck %s<br>
+<br>
+// CHECK-NOT: warning:<br>
+<br>
+bool g() { return false; }<br>
+<br>
+void f(int a, double b) {<br>
+ int b1 = (int)b;<br>
+ // CHECK: :[[@LINE-1]]:12: warning: C-style casts are discouraged. Use static_cast{{.*}}<br>
+<br>
+ // CHECK-NOT: warning:<br>
+ int b2 = int(b);<br>
+ int b3 = static_cast<double>(b);<br>
+ int b4 = b;<br>
+ double aa = a;<br>
+ (void)b2;<br>
+ return (void)g();<br>
+}<br>
+<br>
+// CHECK-NOT: warning:<br>
+enum E { E1 = 1 };<br>
+template <E e><br>
+struct A { static const E ee = e; };<br>
+struct B : public A<E1> {};<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br></div></div><span class="HOEnZb"><font color="#888888">
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div><div><font color="#666666"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Alexander Kornienko |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span></font><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"><font color="#666666"> </font><a href="mailto:alexfh@google.com" style="color:rgb(17,85,204)" target="_blank">alexfh@google.com</a> |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> Google Germany, Munich</span></div>
</div></div>
</div>