From 533c01d9b1ef57b4758609de8459a137b233d3e3 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Tue, 19 Apr 2016 13:29:05 +0000 Subject: [PATCH] [clang-tidy] readability-container-size-empty fixes Summary: This patch fixes PR27410 and adds std::basic_string support. Reviewers: Eugene.Zelenko, hokein Subscribers: cfe-commits, o.gyorgy Differential Revision: http://reviews.llvm.org/D19262 llvm-svn: 266734 --- .../readability/ContainerSizeEmptyCheck.cpp | 20 +++++++++++-------- .../readability-container-size-empty.cpp | 23 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp index f194b47..5639615 100644 --- a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -16,6 +16,7 @@ using namespace clang::ast_matchers; static bool isContainerName(llvm::StringRef ClassName) { static const char *const ContainerNames[] = {"array", + "basic_string", "deque", "forward_list", "list", @@ -59,14 +60,13 @@ void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { return; const auto WrongUse = anyOf( - hasParent( - binaryOperator( - anyOf(has(integerLiteral(equals(0))), - allOf(anyOf(hasOperatorName("<"), hasOperatorName(">="), - hasOperatorName(">"), hasOperatorName("<=")), - hasEitherOperand( - ignoringImpCasts(integerLiteral(equals(1))))))) - .bind("SizeBinaryOp")), + hasParent(binaryOperator( + anyOf(hasOperatorName("<"), hasOperatorName(">="), + hasOperatorName(">"), hasOperatorName("<="), + hasOperatorName("=="), hasOperatorName("!=")), + hasEitherOperand(ignoringImpCasts(anyOf( + integerLiteral(equals(1)), integerLiteral(equals(0)))))) + .bind("SizeBinaryOp")), hasParent(implicitCastExpr( hasImplicitDestinationType(booleanType()), anyOf( @@ -122,6 +122,10 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) { if (Value > 1) return; + if (Value == 1 && (OpCode == BinaryOperatorKind::BO_EQ || + OpCode == BinaryOperatorKind::BO_NE)) + return; + // Always true, no warnings for that. if ((OpCode == BinaryOperatorKind::BO_GE && Value == 0 && ContainerIsLHS) || (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContainerIsLHS)) diff --git a/clang-tools-extra/test/clang-tidy/readability-container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/readability-container-size-empty.cpp index eb453fc..a3f6385 100644 --- a/clang-tools-extra/test/clang-tidy/readability-container-size-empty.cpp +++ b/clang-tools-extra/test/clang-tidy/readability-container-size-empty.cpp @@ -7,6 +7,15 @@ template struct vector { bool empty() const; }; +template struct basic_string { + basic_string(); + unsigned long size() const; + bool empty() const; +}; + +typedef basic_string string; +typedef basic_string wstring; + inline namespace __v2 { template struct set { set(); @@ -20,10 +29,24 @@ template struct set { int main() { std::set intSet; + std::string str; + std::wstring wstr; + str.size() + 0; + str.size() - 0; + 0 + str.size(); + 0 - str.size(); 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] // CHECK-FIXES: {{^ }}if (intSet.empty()){{$}} + if (str.size() == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (str.empty()){{$}} + if (wstr.size() == 0) + ; + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used + // CHECK-FIXES: {{^ }}if (wstr.empty()){{$}} std::vector vect; if (vect.size() == 0) ; -- 2.7.4