[clang-tools-extra] [clang-tidy] Add option to cppcoreguidelines-pro-type-reinterpret-cas… (PR #106922)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: Carlos Galvez (carlosgalvezp)

<details>
<summary>Changes</summary>

…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.

---
Full diff: https://github.com/llvm/llvm-project/pull/106922.diff


5 Files Affected:

- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp (+47-3) 
- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.h (+5-2) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-reinterpret-cast.rst (+19) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-reinterpret-cast.cpp (+34-2) 


``````````diff
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]
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/106922