[PATCH] D13313: [clang-tidy] new check misc-no-reinterpret-cast

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added reviewers: aaron.ballman, alexfh.
aaron.ballman added a comment.

As a slightly more broad question: I think we should have a user-customizable way to categorize these checks so that you can enable/disable them with finer-grained control. Some of the existing checkers already cover the Cpp guidelines, and we'll likely be adding plenty more. There's quite likely overlap with Google and LLVM checkers, etc. It would be really nice if we had a way to say: -checks=-*, CppGuidelines or -checks=-*, CERT, etc.

(I'm not suggesting this as part of this patch, but I think it is an idea we should consider exploring because style guidelines abound: the new C++ ones, MISRA, CERT, joint strike fighter, etc. User-customizable categorization would really help for this sort of thing. This would help assuage my issue with the checker being on by default in misc-* -- it could be off in misc-* but on in cppcoreguidelines-*, for instance.)

~Aaron


================
Comment at: clang-tidy/misc/NoReinterpretCastCheck.cpp:29
@@ +28,3 @@
+      Result.Nodes.getNodeAs<CXXReinterpretCastExpr>("cast");
+  diag(MatchedCast->getOperatorLoc(),
+       "do not use reinterpret_cast (C++ Core Guidelines, rule Type.1)");
----------------
I am worried about the amount of chattiness for this diagnostic and the fact that it does not provide the user with any idea as to what to do instead. The C-style cast checker will suggest that users don't use C-style casts, which suggests there's no way to appease this checker. The style guide suggests using variant, but that is not a workable solution for projects that don't have a variant type.

FWIW, I've run into the same thing for CERT rules like:

DCL50-CPP. Do not define a C-style variadic function 
MSC50-CPP. Do not use std::rand() for generating pseudorandom numbers (to a lesser extent, because <random> exists now.)

I don't have a particularly good solution to the issue though. I'm not opposed to the checker, but I am opposed to the checker being on by default for misc-* checkers.


http://reviews.llvm.org/D13313