Thank you, it looks like I missed that comment. I have altered the test
according to your recommendations. I think I get it now how those check-fixes
works :)
http://reviews.llvm.org/D6925
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ContainerSizeEmpty.cpp
clang-tidy/readability/ContainerSizeEmpty.h
clang-tidy/readability/ReadabilityTidyModule.cpp
test/clang-tidy/readibility-container-size-empty.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: clang-tidy/readability/CMakeLists.txt
===================================================================
--- clang-tidy/readability/CMakeLists.txt
+++ clang-tidy/readability/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyReadabilityModule
BracesAroundStatementsCheck.cpp
+ ContainerSizeEmpty.cpp
FunctionSize.cpp
NamespaceCommentCheck.cpp
ReadabilityTidyModule.cpp
Index: clang-tidy/readability/ContainerSizeEmpty.cpp
===================================================================
--- clang-tidy/readability/ContainerSizeEmpty.cpp
+++ clang-tidy/readability/ContainerSizeEmpty.cpp
@@ -0,0 +1,173 @@
+//===--- ContainerSizeEmpty.cpp - clang-tidy ------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "ContainerSizeEmpty.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace {
+bool isContainer(llvm::StringRef ClassName) {
+ static const llvm::StringSet<> ContainerNames = [] {
+ llvm::StringSet<> RetVal;
+ RetVal.insert("std::vector");
+ RetVal.insert("std::list");
+ RetVal.insert("std::array");
+ RetVal.insert("std::deque");
+ RetVal.insert("std::forward_list");
+ RetVal.insert("std::set");
+ RetVal.insert("std::map");
+ RetVal.insert("std::multiset");
+ RetVal.insert("std::multimap");
+ RetVal.insert("std::unordered_set");
+ RetVal.insert("std::unordered_map");
+ RetVal.insert("std::unordered_multiset");
+ RetVal.insert("std::unordered_multimap");
+ RetVal.insert("std::stack");
+ RetVal.insert("std::queue");
+ RetVal.insert("std::priority_queue");
+ return RetVal;
+ }();
+ return ContainerNames.find(ClassName) != ContainerNames.end();
+}
+}
+
+namespace clang {
+namespace ast_matchers {
+AST_MATCHER_P(QualType, unqualifiedType, internal::Matcher<Type>,
+ InnerMatcher) {
+ return InnerMatcher.matches(*Node, Finder, Builder);
+}
+
+AST_MATCHER(Type, isBoolType) { return Node.isBooleanType(); }
+
+AST_MATCHER(NamedDecl, stlContainer) {
+ return isContainer(Node.getQualifiedNameAsString());
+}
+}
+}
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
+ const auto WrongUse = anyOf(
+ hasParent(
+ binaryOperator(
+ anyOf(has(integerLiteral(equals(0))),
+ allOf(anyOf(hasOperatorName("<"), hasOperatorName(">="),
+ hasOperatorName(">"), hasOperatorName("<=")),
+ anyOf(hasRHS(integerLiteral(equals(1))),
+ hasLHS(integerLiteral(equals(1)))))))
+ .bind("SizeBinaryOp")),
+ hasParent(implicitCastExpr(
+ hasImplicitDestinationType(unqualifiedType(isBoolType())),
+ anyOf(
+ hasParent(unaryOperator(hasOperatorName("!")).bind("NegOnSize")),
+ anything()))),
+ hasParent(
+ explicitCastExpr(hasDestinationType(unqualifiedType(isBoolType())))));
+
+ Finder->addMatcher(
+ memberCallExpr(
+ on(expr(anyOf(hasType(namedDecl(stlContainer())),
+ hasType(qualType(pointsTo(namedDecl(stlContainer())))),
+ hasType(qualType(references(
+ namedDecl(stlContainer())))))).bind("STLObject")),
+ callee(methodDecl(hasName("size"))), WrongUse).bind("SizeCallExpr"),
+ this);
+}
+
+void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MemberCall =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("SizeCallExpr");
+ const auto *BinaryOp = Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
+ const auto *E = Result.Nodes.getNodeAs<Expr>("STLObject");
+ FixItHint Hint;
+ std::string ReplacementText =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
+ *Result.SourceManager, LangOptions());
+ if (E->getType()->isPointerType())
+ ReplacementText += "->empty()";
+ else
+ ReplacementText += ".empty()";
+
+ if (BinaryOp) { // Determine the correct transformation.
+ bool Negation = false;
+ const bool ContainerIsLHS = !llvm::isa<IntegerLiteral>(BinaryOp->getLHS());
+ const auto OpCode = BinaryOp->getOpcode();
+ uint64_t Value = 0;
+ if (ContainerIsLHS) {
+ if (const auto *Literal =
+ llvm::dyn_cast<IntegerLiteral>(BinaryOp->getRHS()))
+ Value = Literal->getValue().getLimitedValue();
+ else
+ return;
+ } else {
+ Value = llvm::dyn_cast<IntegerLiteral>(BinaryOp->getLHS())
+ ->getValue()
+ .getLimitedValue();
+ }
+
+ // Constant that is not handled.
+ if (Value > 1)
+ return;
+
+ // Always true, no warnings for that.
+ if ((OpCode == BinaryOperatorKind::BO_GE && Value == 0 && ContainerIsLHS) ||
+ (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContainerIsLHS))
+ return;
+
+ if (OpCode == BinaryOperatorKind::BO_NE && Value == 0)
+ Negation = true;
+ if ((OpCode == BinaryOperatorKind::BO_GT ||
+ OpCode == BinaryOperatorKind::BO_GE) &&
+ ContainerIsLHS)
+ Negation = true;
+ if ((OpCode == BinaryOperatorKind::BO_LT ||
+ OpCode == BinaryOperatorKind::BO_LE) &&
+ !ContainerIsLHS)
+ Negation = true;
+
+ if (Negation)
+ ReplacementText = "!" + ReplacementText;
+ Hint = FixItHint::CreateReplacement(BinaryOp->getSourceRange(),
+ ReplacementText);
+
+ } else {
+ // If there is a conversion above the size call to bool, it is safe to just
+ // replace size with empty.
+ if (const auto *UnaryOp =
+ Result.Nodes.getNodeAs<UnaryOperator>("NegOnSize"))
+ Hint = FixItHint::CreateReplacement(UnaryOp->getSourceRange(),
+ ReplacementText);
+ else
+ Hint = FixItHint::CreateReplacement(MemberCall->getSourceRange(),
+ "!" + ReplacementText);
+ }
+ diag(MemberCall->getLocStart(),
+ "The 'empty' method should be used to check for emptiness instead "
+ "of 'size'.")
+ << Hint;
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/readability/ContainerSizeEmpty.h
===================================================================
--- clang-tidy/readability/ContainerSizeEmpty.h
+++ clang-tidy/readability/ContainerSizeEmpty.h
@@ -0,0 +1,40 @@
+//===--- ContainerSizeEmpty.h - clang-tidy ----------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTY_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTY_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Checks whether a call to the \c size() method can be replaced with a
+/// call to \c empty().
+///
+/// The emptiness of a container should be checked using the \c empty() method
+/// instead of the \c size() method. It is not guaranteed that \c size() is a
+/// constant-time function, and it is generally more efficient and also shows
+/// clearer intent to use \c empty(). Furthermore some containers may implement
+/// the \c empty() method but not implement the \c size() method. Using \c
+/// empty() whenever possible makes it easier to switch to another container in
+/// the future.
+class ContainerSizeEmptyCheck : public ClangTidyCheck {
+public:
+ ContainerSizeEmptyCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTY_H
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===================================================================
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "BracesAroundStatementsCheck.h"
+#include "ContainerSizeEmpty.h"
#include "FunctionSize.h"
#include "RedundantSmartptrGet.h"
@@ -23,6 +24,8 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<BracesAroundStatementsCheck>(
"readability-braces-around-statements");
+ CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
+ "readability-container-size-empty");
CheckFactories.registerCheck<FunctionSizeCheck>(
"readability-function-size");
CheckFactories.registerCheck<RedundantSmartptrGet>(
Index: test/clang-tidy/readibility-container-size-empty.cpp
===================================================================
--- test/clang-tidy/readibility-container-size-empty.cpp
+++ test/clang-tidy/readibility-container-size-empty.cpp
@@ -0,0 +1,106 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s readability-container-size-empty %t
+// REQUIRES: shell
+
+namespace std {
+template <typename T> struct vector {
+ vector() {}
+ int size() const {}
+ bool empty() const {}
+};
+}
+
+int main() {
+ std::vector<int> vect;
+ if (vect.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 (vect.empty()){{$}}
+ if (vect.size() != 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (0 == vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
+ if (0 != vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (vect.size() > 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (0 < vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (vect.size() < 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
+ if (1 > vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
+ if (vect.size() >= 1)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (1 <= vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+ if (!vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect.empty()){{$}}
+ if (vect.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect.empty()){{$}}
+
+ if (vect.empty())
+ ;
+
+ const std::vector<int> vect2;
+ if (vect2.size() != 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!vect2.empty()){{$}}
+
+ std::vector<int> *vect3 = new std::vector<int>();
+ if (vect3->size() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect3->empty()){{$}}
+
+ delete vect3;
+
+ const std::vector<int> &vect4 = vect2;
+ if (vect4.size() == 0)
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (vect4.empty()){{$}}
+}
+
+#define CHECKSIZE(x) if (x.size())
+// CHECK-FIXES: #define CHECKSIZE(x) if (x.size())
+
+template <typename T> void f() {
+ std::vector<T> v;
+ if (v.size())
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: The 'empty' method should be used
+ // CHECK-FIXES: {{^ }}if (!v.empty()){{$}}
+ // CHECK-FIXES-NEXT: ;
+ CHECKSIZE(v);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: The 'empty' method should be used
+ // CHECK-MESSAGES: CHECKSIZE(v);
+}
+
+void g() {
+ f<int>();
+ f<double>();
+ f<char *>();
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
