[clang-tools-extra] 0a93abd - [clang-tidy] Make readability-container-size-empty check using <string> header

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 12 06:35:08 PDT 2023


Author: Mike Crowe
Date: 2023-03-12T13:32:29Z
New Revision: 0a93abd9765809bb13cfdb43794bf17e9ad5e137

URL: https://github.com/llvm/llvm-project/commit/0a93abd9765809bb13cfdb43794bf17e9ad5e137
DIFF: https://github.com/llvm/llvm-project/commit/0a93abd9765809bb13cfdb43794bf17e9ad5e137.diff

LOG: [clang-tidy] Make readability-container-size-empty check using <string> header

Improve the generic <string> header by adding the size() method so that
it can be used to replace the custom implementation in the
readability-container-size-empty check.

This requires fixing an incorrect comparison of a std::wstring with a
char string literal.

Unfortunately, removing the custom basic_string implementation means
fixing the line numbers for many of the checks.

Depends on D145312

Reviewed By: PiotrZSL

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

Added: 
    

Modified: 
    clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
    clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
index ea9d1f9c32bf..91ae2af4d132 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -26,6 +26,7 @@ struct basic_string {
   const C *data() const;
 
   bool empty() const;
+  size_type size() const;
 
   _Type& append(const C *s);
   _Type& append(const C *s, size_type n);
@@ -79,6 +80,10 @@ bool operator==(const char*, const std::string&);
 bool operator!=(const std::string&, const std::string&);
 bool operator!=(const std::string&, const char*);
 bool operator!=(const char*, const std::string&);
+
+bool operator==(const std::wstring&, const std::wstring&);
+bool operator==(const std::wstring&, const wchar_t*);
+bool operator==(const wchar_t*, const std::wstring&);
 }
 
 #endif // _STRING_

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
index 47606070d69c..28ce8d5f8cd3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,6 +1,7 @@
 // RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
 // RUN: -config="{CheckOptions: [{key: readability-container-size-empty.ExcludedComparisonTypes , value: '::std::array;::IgnoredDummyType'}]}" \
-// RUN: -- -fno-delayed-template-parsing
+// RUN: -- -fno-delayed-template-parsing -isystem %clang_tidy_headers
+#include <string>
 
 namespace std {
 template <typename T> struct vector {
@@ -11,20 +12,6 @@ template <typename T> struct vector {
   bool empty() const;
 };
 
-template <typename T> struct basic_string {
-  basic_string();
-  bool operator==(const basic_string<T>& other) const;
-  bool operator!=(const basic_string<T>& other) const;
-  bool operator==(const char *) const;
-  bool operator!=(const char *) const;
-  basic_string<T> operator+(const basic_string<T>& other) const;
-  unsigned long size() const;
-  bool empty() const;
-};
-
-typedef basic_string<char> string;
-typedef basic_string<wchar_t> wstring;
-
 inline namespace __v2 {
 template <typename T> struct set {
   set();
@@ -125,12 +112,12 @@ bool returnsBool() {
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :21:8: note: method 'set'::empty() defined here
   if (intSet == std::set<int>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :34:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :21:8: note: method 'set'::empty() defined here
   if (s_func() == "")
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
@@ -155,7 +142,7 @@ bool returnsBool() {
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
-  if (wstr == "")
+  if (wstr == L"")
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
@@ -452,7 +439,7 @@ class ConstructWithBoolField {
 public:
   ConstructWithBoolField(const std::vector<int> &C) : B(C.size()) {}
 // CHECK-MESSAGES: :[[@LINE-1]]:57: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}ConstructWithBoolField(const std::vector<int> &C) : B(!C.empty()) {}
 };
 
@@ -460,21 +447,21 @@ struct StructWithNSDMI {
   std::vector<int> C;
   bool B = C.size();
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}bool B = !C.empty();
 };
 
 int func(const std::vector<int> &C) {
   return C.size() ? 0 : 1;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+// CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
 // CHECK-FIXES: {{^  }}return !C.empty() ? 0 : 1;
 }
 
 constexpr Lazy L;
 static_assert(!L.size(), "");
 // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: the 'empty' method should be used
-// CHECK-MESSAGES: :103:18: note: method 'Lazy'::empty() defined here
+// CHECK-MESSAGES: :90:18: note: method 'Lazy'::empty() defined here
 // CHECK-FIXES: {{^}}static_assert(L.empty(), "");
 
 struct StructWithLazyNoexcept {
@@ -489,7 +476,7 @@ template <typename T> void f() {
   if (v.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v == std::vector<T>())
     ;
@@ -498,24 +485,24 @@ template <typename T> void f() {
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(v);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(v);
 
   TemplatedContainer<T> templated_container;
   if (templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer<T>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   CHECKSIZE(templated_container);
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: CHECKSIZE(templated_container);
 }
 
@@ -531,7 +518,7 @@ bool neverInstantiatedTemplate() {
   if (v.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
 
   if (v == std::vector<T>())
@@ -542,22 +529,22 @@ bool neverInstantiatedTemplate() {
   if (v.size() == 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
   if (v.size() != 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v.size() < 1)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
   if (v.size() > 0)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty()){{$}}
   if (v.size() == 1)
     ;
@@ -571,91 +558,91 @@ bool neverInstantiatedTemplate() {
   if (static_cast<bool>(v.size()))
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (static_cast<bool>(!v.empty())){{$}}
   if (v.size() && false)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!v.empty() && false){{$}}
   if (!v.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}if (v.empty()){{$}}
 
   TemplatedContainer<T> templated_container;
   if (templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer<T>())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   while (templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty()){{$}}
 
   do {
   }
   while (templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}while (!templated_container.empty());
 
   for (; templated_container.size();)
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}for (; !templated_container.empty();){{$}}
 
   if (true && templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true && !templated_container.empty()){{$}}
 
   if (true || templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (true || !templated_container.empty()){{$}}
 
   if (!templated_container.size())
     ;
   // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (templated_container.empty()){{$}}
 
   bool b1 = templated_container.size();
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}bool b1 = !templated_container.empty();
 
   bool b2(templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}bool b2(!templated_container.empty());
 
   auto b3 = static_cast<bool>(templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}auto b3 = static_cast<bool>(!templated_container.empty());
 
   auto b4 = (bool)templated_container.size();
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}auto b4 = (bool)!templated_container.empty();
 
   auto b5 = bool(templated_container.size());
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}auto b5 = bool(!templated_container.empty());
 
   takesBool(templated_container.size());
@@ -664,7 +651,7 @@ bool neverInstantiatedTemplate() {
 
   return templated_container.size();
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :46:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}return !templated_container.empty();
 }
 
@@ -710,14 +697,14 @@ struct unique_ptr {
 bool call_through_unique_ptr(const std::unique_ptr<std::vector<int>> &ptr) {
   return ptr->size() > 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}return !ptr->empty();
 }
 
 bool call_through_unique_ptr_deref(const std::unique_ptr<std::vector<int>> &ptr) {
   return (*ptr).size() > 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :11:8: note: method 'vector'::empty() defined here
+  // CHECK-MESSAGES: :12:8: note: method 'vector'::empty() defined here
   // CHECK-FIXES: {{^  }}return !(*ptr).empty();
 }
 
@@ -766,7 +753,7 @@ bool testArraySize(const Array& value) {
   return value.size() == 0U;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
 // CHECK-FIXES: {{^  }}return value.empty();{{$}}
-// CHECK-MESSAGES: :744:8: note: method 'array'::empty() defined here
+// CHECK-MESSAGES: :731:8: note: method 'array'::empty() defined here
 }
 
 bool testArrayCompareToEmpty(const Array& value) {
@@ -777,7 +764,7 @@ bool testDummyType(const DummyType& value) {
   return value == DummyType();
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object [readability-container-size-empty]
 // CHECK-FIXES: {{^  }}return value.empty();{{$}}
-// CHECK-MESSAGES: :754:8: note: method 'DummyType'::empty() defined here
+// CHECK-MESSAGES: :741:8: note: method 'DummyType'::empty() defined here
 }
 
 bool testIgnoredDummyType(const IgnoredDummyType& value) {


        


More information about the cfe-commits mailing list