PiotrZSL updated this revision to Diff 551757.
PiotrZSL added a comment.
Rebase + Ping
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56644/new/
https://reviews.llvm.org/D56644
Files:
clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -105,9 +105,13 @@
std::string str2;
std::wstring wstr;
(void)(str.size() + 0);
+ (void)(str.length() + 0);
(void)(str.size() - 0);
+ (void)(str.length() - 0);
(void)(0 + str.size());
+ (void)(0 + str.length());
(void)(0 - str.size());
+ (void)(0 - str.length());
if (intSet.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]
@@ -124,11 +128,19 @@
// CHECK-FIXES: {{^ }}if (s_func().empty()){{$}}
if (str.size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: {{^ }}if (str.empty()){{$}}
+ if (str.length() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length'
// CHECK-FIXES: {{^ }}if (str.empty()){{$}}
if ((str + str2).size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
+ if ((str + str2).length() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length'
// CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
if (str == "")
;
@@ -140,7 +152,11 @@
// CHECK-FIXES: {{^ }}if ((str + str2).empty()){{$}}
if (wstr.size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
+ // CHECK-FIXES: {{^ }}if (wstr.empty()){{$}}
+ if (wstr.length() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length'
// CHECK-FIXES: {{^ }}if (wstr.empty()){{$}}
if (wstr == L"")
;
@@ -149,7 +165,7 @@
std::vector<int> vect;
if (vect.size() == 0)
;
- // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size'
// CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
if (vect == std::vector<int>())
;
@@ -504,6 +520,17 @@
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: the 'empty' method should be used
// CHECK-MESSAGES: :33:8: note: method 'TemplatedContainer'::empty() defined here
// CHECK-FIXES: CHECKSIZE(templated_container);
+ std::basic_string<T> s;
+ if (s.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: string:28:8: note: method 'basic_string'::empty() defined here
+ // CHECK-FIXES: {{^ }}if (!s.empty()){{$}}
+ if (s.length())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
+ // CHECK-MESSAGES: string:28:8: note: method 'basic_string'::empty() defined here
+ // CHECK-FIXES: {{^ }}if (!s.empty()){{$}}
}
void g() {
@@ -753,7 +780,7 @@
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: :731:8: note: method 'array'::empty() defined here
+// CHECK-MESSAGES: :[[@LINE-25]]:8: note: method 'array'::empty() defined here
}
bool testArrayCompareToEmpty(const Array& value) {
@@ -764,7 +791,7 @@
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: :741:8: note: method 'DummyType'::empty() defined here
+// CHECK-MESSAGES: :[[@LINE-26]]:8: note: method 'DummyType'::empty() defined here
}
bool testIgnoredDummyType(const IgnoredDummyType& value) {
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/string
@@ -27,6 +27,7 @@
bool empty() const;
size_type size() const;
+ size_type length() const;
_Type& append(const C *s);
_Type& append(const C *s, size_type n);
Index: clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/container-size-empty.rst
@@ -4,23 +4,24 @@
================================
-Checks whether a call to the ``size()`` method can be replaced with a call to
-``empty()``.
+Checks whether a call to the ``size()``/``length()`` method can be replaced
+with a call to ``empty()``.
The emptiness of a container should be checked using the ``empty()`` method
-instead of the ``size()`` method. It is not guaranteed that ``size()`` is a
-constant-time function, and it is generally more efficient and also shows
-clearer intent to use ``empty()``. Furthermore some containers may implement
-the ``empty()`` method but not implement the ``size()`` method. Using
-``empty()`` whenever possible makes it easier to switch to another container in
-the future.
+instead of the ``size()``/``length()`` method. It is not guaranteed that
+``size()``/``length()`` is a constant-time function, and it is generally more
+efficient and also shows clearer intent to use ``empty()``. Furthermore some
+containers may implement the ``empty()`` method but not implement the ``size()``
+or ``length()`` method. Using ``empty()`` whenever possible makes it easier to
+switch to another container in the future.
-The check issues warning if a container has ``size()`` and ``empty()`` methods
-matching following signatures:
+The check issues warning if a container has ``empty()`` and ``size()`` or
+``length()`` methods matching following signatures:
.. code-block:: c++
size_type size() const;
+ size_type length() const;
bool empty() const;
`size_type` can be any kind of integer type.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -222,6 +222,10 @@
<clang-tidy/checks/performance/noexcept-swap>` check to enforce a stricter
match with the swap function signature, eliminating false-positives.
+- Improved :doc:`readability-container-size-empty
+ <clang-tidy/checks/readability/container-size-empty>` check to support
+ ``length()`` method as an alternative to ``size()``.
+
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability/identifier-naming>` check to emit proper
warnings when a type forward declaration precedes its definition.
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.h
@@ -14,8 +14,8 @@
namespace clang::tidy::readability {
-/// Checks whether a call to the `size()` method can be replaced with a call to
-/// `empty()`.
+/// Checks whether a call to the `size()`/`length()` method can be replaced with
+/// a call to `empty()`.
///
/// The emptiness of a container should be checked using the `empty()` method
/// instead of the `size()` method. It is not guaranteed that `size()` is a
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -112,14 +112,14 @@
void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
const auto ValidContainerRecord = cxxRecordDecl(isSameOrDerivedFrom(
- namedDecl(
- has(cxxMethodDecl(
- isConst(), parameterCountIs(0), isPublic(), hasName("size"),
- returns(qualType(isIntegralType(), unless(booleanType()))))
- .bind("size")),
- has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
- hasName("empty"), returns(booleanType()))
- .bind("empty")))
+ namedDecl(has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+ hasAnyName("size", "length"),
+ returns(qualType(isIntegralType(),
+ unless(booleanType()))))
+ .bind("size")),
+ has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
+ hasName("empty"), returns(booleanType()))
+ .bind("empty")))
.bind("container")));
const auto ValidContainerNonTemplateType =
@@ -141,24 +141,28 @@
usedInBooleanContext());
Finder->addMatcher(
- cxxMemberCallExpr(on(expr(anyOf(hasType(ValidContainer),
- hasType(pointsTo(ValidContainer)),
- hasType(references(ValidContainer))))
- .bind("MemberCallObject")),
- callee(cxxMethodDecl(hasName("size"))), WrongUse,
- unless(hasAncestor(cxxMethodDecl(
- ofClass(equalsBoundNode("container"))))))
+ cxxMemberCallExpr(
+ on(expr(anyOf(hasType(ValidContainer),
+ hasType(pointsTo(ValidContainer)),
+ hasType(references(ValidContainer))))
+ .bind("MemberCallObject")),
+ callee(
+ cxxMethodDecl(hasAnyName("size", "length")).bind("SizeMethod")),
+ WrongUse,
+ unless(hasAncestor(
+ cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
.bind("SizeCallExpr"),
this);
Finder->addMatcher(
callExpr(has(cxxDependentScopeMemberExpr(
- hasObjectExpression(
- expr(anyOf(hasType(ValidContainer),
- hasType(pointsTo(ValidContainer)),
- hasType(references(ValidContainer))))
- .bind("MemberCallObject")),
- hasMemberName("size"))),
+ hasObjectExpression(
+ expr(anyOf(hasType(ValidContainer),
+ hasType(pointsTo(ValidContainer)),
+ hasType(references(ValidContainer))))
+ .bind("MemberCallObject")),
+ anyOf(hasMemberName("size"), hasMemberName("length")))
+ .bind("DependentExpr")),
WrongUse,
unless(hasAncestor(
cxxMethodDecl(ofClass(equalsBoundNode("container"))))))
@@ -323,9 +327,18 @@
auto WarnLoc = MemberCall ? MemberCall->getBeginLoc() : SourceLocation{};
if (WarnLoc.isValid()) {
- diag(WarnLoc, "the 'empty' method should be used to check "
- "for emptiness instead of 'size'")
- << Hint;
+ auto Diag = diag(WarnLoc, "the 'empty' method should be used to check "
+ "for emptiness instead of %0");
+ if (const auto *SizeMethod =
+ Result.Nodes.getNodeAs<NamedDecl>("SizeMethod"))
+ Diag << SizeMethod;
+ else if (const auto *DependentExpr =
+ Result.Nodes.getNodeAs<CXXDependentScopeMemberExpr>(
+ "DependentExpr"))
+ Diag << DependentExpr->getMember();
+ else
+ Diag << "unknown method";
+ Diag << Hint;
} else {
WarnLoc = BinCmpTempl
? BinCmpTempl->getBeginLoc()
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
