[clang-tidy] Readability-container-data-pointer adds new option to ignore Containers
authorFelix <felix-antoine.constantin@polymtl.ca>
Mon, 12 Jun 2023 05:34:08 +0000 (05:34 +0000)
committerPiotr Zegar <me@piotrzegar.pl>
Mon, 12 Jun 2023 06:07:27 +0000 (06:07 +0000)
Adds a new option to the clang-tidy's check : readability-container-data-pointer to ignore some containers.

This option is useful in the case of std::array where the size is known at compile time and there is no real risk to access the first index of the container. In that case some users might prefer to ignore this type of container.

Relates to : https://github.com/llvm/llvm-project/issues/57445

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D133244

clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/container-data-pointer.rst
clang-tools-extra/test/clang-tidy/checkers/readability/container-data-pointer.cpp

index 5a51b25..a05e228 100644 (file)
@@ -8,6 +8,8 @@
 
 #include "ContainerDataPointerCheck.h"
 
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -21,13 +23,22 @@ constexpr llvm::StringLiteral AddrOfContainerExprName =
     "addr-of-container-expr";
 constexpr llvm::StringLiteral AddressOfName = "address-of";
 
+void ContainerDataPointerCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoredContainers",
+                utils::options::serializeStringList(IgnoredContainers));
+}
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
                                                      ClangTidyContext *Context)
-    : ClangTidyCheck(Name, Context) {}
+    : ClangTidyCheck(Name, Context),
+      IgnoredContainers(utils::options::parseStringList(
+          Options.get("IgnoredContainers", ""))) {}
 
 void ContainerDataPointerCheck::registerMatchers(MatchFinder *Finder) {
   const auto Record =
       cxxRecordDecl(
+          unless(matchers::matchesAnyListedName(IgnoredContainers)),
           isSameOrDerivedFrom(
               namedDecl(
                   has(cxxMethodDecl(isPublic(), hasName("data")).bind("data")))
index 65f78b8..2a15b95 100644 (file)
@@ -28,6 +28,7 @@ public:
     return LO.CPlusPlus11;
   }
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
 
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
@@ -35,6 +36,9 @@ public:
   std::optional<TraversalKind> getCheckTraversalKind() const override {
     return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const std::vector<StringRef> IgnoredContainers;
 };
 } // namespace clang::tidy::readability
 
index 077be93..db78945 100644 (file)
@@ -410,6 +410,10 @@ Changes in existing checks
   for coroutines where previously a warning would be emitted with coroutines
   throwing exceptions in their bodies.
 
+- Improved :doc:`readability-container-data-pointer
+  <clang-tidy/checks/readability/container-data-pointer>` check with new
+  `IgnoredContainers` option to ignore some containers.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
index fc5d0a5..0d10829 100644 (file)
@@ -11,3 +11,11 @@ should be preferred.
 
 This also ensures that in the case that the container is empty, the data pointer
 access does not perform an errant memory access.
+
+Options
+-------
+
+.. option:: IgnoredContainers
+
+   Semicolon-separated list of containers regexp for which this check won't be
+   enforced. Default is `empty`.
index ee2ba6a..30df8ac 100644 (file)
@@ -1,4 +1,6 @@
-// RUN: %check_clang_tidy %s readability-container-data-pointer %t -- -- -isystem %clang_tidy_headers -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=,CLASSIC %s readability-container-data-pointer %t -- -- -isystem %clang_tidy_headers -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=,WITH-CONFIG %s readability-container-data-pointer %t -- -config="{CheckOptions: [{key: readability-container-data-pointer.IgnoredContainers, value: '::std::basic_string'}]}" -- -isystem %clang_tidy_headers -fno-delayed-template-parsing
+
 #include <string>
 
 typedef __SIZE_TYPE__ size_t;
@@ -50,13 +52,15 @@ void g(size_t s) {
 void h() {
   std::string s;
   f(&((s).operator[]((z))));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(s.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 
   std::wstring w;
   f(&((&(w))->operator[]((z))));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
-  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-CLASSIC: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES-CLASSIC: {{^  }}f(w.data());{{$}}
+  // CHECK-MESSAGES-WITH-CONFIG-NOT: :[[@LINE-3]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
 }
 
 template <typename T, typename U,