https://github.com/carlosgalvezp created https://github.com/llvm/llvm-project/pull/106922
…t to allow casts to byte types These casts are safe according to the Standard, so add an option to allow them and not emit a warning. This helps silencing some noise and focusing on the unsafe casts. >From 87f3bca5a4f0cdb7f560da1d11d76ee9f3ce9d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20G=C3=A1lvez?= <carlos.gal...@zenseact.com> Date: Sun, 1 Sep 2024 19:14:22 +0000 Subject: [PATCH] [clang-tidy] Add option to cppcoreguidelines-pro-type-reinterpret-cast to allow casts to byte types These casts are safe according to the Standard, so add an option to allow them and not emit a warning. This helps silencing some noise and focusing on the unsafe casts. --- .../ProTypeReinterpretCastCheck.cpp | 50 +++++++++++++++++-- .../ProTypeReinterpretCastCheck.h | 7 ++- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++ .../pro-type-reinterpret-cast.rst | 19 +++++++ .../pro-type-reinterpret-cast.cpp | 36 ++++++++++++- 5 files changed, 109 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp index 14456caab612b7..6385dd5440901c 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp @@ -9,20 +9,64 @@ #include "ProTypeReinterpretCastCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/STLExtras.h" +#include <array> +#include <string> using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { +static bool isCastToBytes(ASTContext const &Ctx, + CXXReinterpretCastExpr const &Expr) { + // https://eel.is/c++draft/basic.lval#11.3 + static constexpr std::array<StringRef, 3> AllowedByteTypes = { + "char", + "unsigned char", + "std::byte", + }; + + // We only care about pointer casts + QualType DestType = Expr.getTypeAsWritten(); + if (!DestType->isPointerType()) + return false; + + // Get the unqualified canonical type, and check if it's allowed + // We need to wrap the Type into a QualType to call getAsString() + const Type *UnqualDestType = + DestType.getCanonicalType()->getPointeeType().getTypePtr(); + std::string DestTypeString = QualType(UnqualDestType, /*Quals=*/0) + .getAsString(Ctx.getPrintingPolicy()); + return llvm::any_of(AllowedByteTypes, [DestTypeString](StringRef Type) { + return Type == DestTypeString; + }); +} + +ProTypeReinterpretCastCheck::ProTypeReinterpretCastCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowCastToBytes(Options.getLocalOrGlobal("AllowCastToBytes", false)) {} + +void ProTypeReinterpretCastCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowCastToBytes", AllowCastToBytes); +} + void ProTypeReinterpretCastCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(cxxReinterpretCastExpr().bind("cast"), this); } void ProTypeReinterpretCastCheck::check( const MatchFinder::MatchResult &Result) { - const auto *MatchedCast = - Result.Nodes.getNodeAs<CXXReinterpretCastExpr>("cast"); - diag(MatchedCast->getOperatorLoc(), "do not use reinterpret_cast"); + + if (const auto *MatchedCast = + Result.Nodes.getNodeAs<CXXReinterpretCastExpr>("cast")) { + ASTContext const &Ctx = *Result.Context; + if (AllowCastToBytes && isCastToBytes(Ctx, *MatchedCast)) + return; + + diag(MatchedCast->getOperatorLoc(), "do not use reinterpret_cast"); + } } } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h index da001bfb85d787..66b46ba7e9f5b9 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h @@ -19,13 +19,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast.html class ProTypeReinterpretCastCheck : public ClangTidyCheck { public: - ProTypeReinterpretCastCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + ProTypeReinterpretCastCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const bool AllowCastToBytes; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b001a6ad446695..1b38a3572e02f3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,10 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Add option to :doc:`cppcoreguidelines-pro-type-reinterpret-cast + <clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast>` to allow + casting an object to its byte representation. + - Improved :doc:`modernize-use-std-format <clang-tidy/checks/modernize/use-std-format>` check to support replacing member function calls too. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast.rst index a0946825156fcf..c8d9130f4f2534 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast.rst @@ -12,3 +12,22 @@ unrelated type ``Z``. This rule is part of the `Type safety (Type.1.1) <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-type-reinterpretcast>`_ profile from the C++ Core Guidelines. + +Options +------- + +.. option:: AllowCastToBytes + + When this setting is set to `true`, it will not warn when casting an object + to its byte representation, which is safe according to the C++ Standard. + The allowed byte types are: ``char``, ``unsigned char`` and ``std::byte``. + Example: + + .. code-block:: cpp + + float x{}; + auto a = reinterpret_cast<char*>(&x); // OK + auto b = reinterpret_cast<unsigned char*>(&x); // OK + auto c = reinterpret_cast<std::byte*>(&x); // OK + + Default value is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-reinterpret-cast.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-reinterpret-cast.cpp index 54208102e6f07f..1bfdc4b0eca7b8 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-reinterpret-cast.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-reinterpret-cast.cpp @@ -1,6 +1,38 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-reinterpret-cast %t +// RUN: %check_clang_tidy -check-suffix=DEFAULT %s cppcoreguidelines-pro-type-reinterpret-cast %t +// RUN: %check_clang_tidy -check-suffix=ALLOW-CAST-TO-BYTES %s cppcoreguidelines-pro-type-reinterpret-cast %t \ +// RUN: -config="{CheckOptions: { \ +// RUN: cppcoreguidelines-pro-type-reinterpret-cast.AllowCastToBytes: True \ +// RUN: }}" int i = 0; void *j; void f() { j = reinterpret_cast<void *>(i); } -// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] +// CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:16: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] +// CHECK-MESSAGES-ALLOW-CAST-TO-BYTES: :[[@LINE-2]]:16: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + +namespace std +{ +enum class byte : unsigned char +{}; +} + +void check_cast_to_bytes() +{ + float x{}; + auto a = reinterpret_cast<char*>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + auto b = reinterpret_cast<char const*>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + auto c = reinterpret_cast<unsigned char*>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + auto d = reinterpret_cast<unsigned char const*>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + auto e = reinterpret_cast<std::byte*>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + auto f = reinterpret_cast<std::byte const*>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] + + using CharPtr = char*; + auto g = reinterpret_cast<CharPtr>(&x); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:12: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast] +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits