[PATCH] Suggest automated replacements of C-style casts with C++ casts.

Alexander Kornienko alexfh at google.com
Sat Jul 12 11:49:30 PDT 2014


================
Comment at: clang-tidy/google/AvoidCStyleCastsCheck.cpp:33
@@ -30,1 +32,3 @@
 
+bool needConstCast(QualType SourceType, QualType DestType) {
+  SourceType = SourceType.getNonReferenceType();
----------------
Daniel Jasper wrote:
> nit: Grammatically, I'd prefer "needsConstCast"
It used to be named like this before a few rounds of refactoring. Renamed.

================
Comment at: clang-tidy/google/AvoidCStyleCastsCheck.cpp:70
@@ +69,3 @@
+
+  auto ParenRange = CharSourceRange::getTokenRange(CastExpr->getLParenLoc(),
+                                                   CastExpr->getRParenLoc());
----------------
Daniel Jasper wrote:
> Create a test where the cast is inside a macro. I think you'll want to use the spelling location (or completely abort as the required cast type might actually be different between different macro invocations).
To the moment we completely ignored casts in macros. Maybe we should warn, just refrain from suggesting fixes. I've implemented it this way now, please take a look.

================
Comment at: clang-tidy/google/AvoidCStyleCastsCheck.cpp:120
@@ +119,3 @@
+      return;
+    }
+  default:
----------------
Daniel Jasper wrote:
> Though benign now, don't fall through here.
We definitely need to turn on -Wimplicit-fallthrough in LLVM/Clang.

================
Comment at: test/clang-tidy/avoid-c-style-casts.cpp:9
@@ +8,3 @@
+
+void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
+  const char *cpc2 = (const char*)cpc;
----------------
Daniel Jasper wrote:
> What happens for the following code?
> 
>   template <typename T> void f(T t) { int i = (int)t; }
>   f(1);
>   f(1.0);
> 
> (With the first call to f() hopefully leading to a message about a redundant cast and the second call leading to a reinterpret_cast)
Good point. Now that I avoid matches in template instantiations, fixes are not suggested for casts to/from dependent types.

http://reviews.llvm.org/D4478






More information about the cfe-commits mailing list