Hi klimek, alexfh,
We are porting some of the checkers at a company we developed to the Clang Tidy
infrastructure. We would like to open source the checkers that may be useful
for the community as well. This patch is the first checker that is being ported
to Clang Tidy. We also added fix-it hints, and applied them to LLVM:
http://reviews.llvm.org/D6924
The code compiled and the unit tests are passed after the fixits was applied.
The documentation of the checker:
/// 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.
It also uses some custom ASTMatchers. In case you find them useful I can submit
them as separate patches to clang. I will apply your suggestions to this patch.
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
modularize/ModuleAssistant.cpp
module-map-checker/ModuleMapChecker.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,160 @@
+//===--- 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 <set>
+#include <string>
+
+#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 {
+const std::set<std::string> containerNames = {
+ "std::vector", "std::list", "std::array", "std::deque", "std::forward_list",
+ "std::set", "std::map", "std::multiset", "std::multimap",
+ "std::unordered_set", "std::unordered_map", "std::unordered_multiset",
+ "std::unordered_multimap", "std::stack", "std::queue",
+ "std::priority_queue"};
+
+bool isContainer(const std::string &className) {
+ 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 wrong_use = 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"))), wrong_use).bind("SizeCallExpr"),
+ this);
+}
+
+void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
+
+ const CXXMemberCallExpr *MC =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("SizeCallExpr");
+ const BinaryOperator *BOP =
+ Result.Nodes.getNodeAs<BinaryOperator>("SizeBinaryOp");
+ const Expr *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 (BOP) { // Determine the correct transformation
+ bool Negation = false;
+ const bool ContIsLHS = !llvm::isa<IntegerLiteral>(BOP->getLHS());
+ const auto OpCode = BOP->getOpcode();
+ uint64_t Value = 0;
+ if (ContIsLHS) {
+ if (IntegerLiteral *LIT = llvm::dyn_cast<IntegerLiteral>(BOP->getRHS()))
+ Value = LIT->getValue().getLimitedValue();
+ else
+ return;
+ } else {
+ Value = llvm::dyn_cast<IntegerLiteral>(BOP->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 && ContIsLHS) ||
+ (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContIsLHS))
+ return;
+
+ if (OpCode == BinaryOperatorKind::BO_NE && Value == 0)
+ Negation = true;
+ if ((OpCode == BinaryOperatorKind::BO_GT ||
+ OpCode == BinaryOperatorKind::BO_GE) &&
+ ContIsLHS)
+ Negation = true;
+ if ((OpCode == BinaryOperatorKind::BO_LT ||
+ OpCode == BinaryOperatorKind::BO_LE) &&
+ !ContIsLHS)
+ Negation = true;
+
+ if (Negation)
+ ReplacementText = "!" + ReplacementText;
+ hint = FixItHint::CreateReplacement(BOP->getSourceRange(), ReplacementText);
+
+ } else { // If there is a conversion above the size call to bool, it is safe
+ // to just replace size with empty
+ const UnaryOperator *UO =
+ Result.Nodes.getNodeAs<UnaryOperator>("NegOnSize");
+ if (UO)
+ hint =
+ FixItHint::CreateReplacement(UO->getSourceRange(), ReplacementText);
+ else
+ hint = FixItHint::CreateReplacement(MC->getSourceRange(),
+ "!" + ReplacementText);
+ }
+ diag(MC->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,38 @@
+//===--- 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 that whether a size method call can be replaced by 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.
+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: modularize/ModuleAssistant.cpp
===================================================================
--- modularize/ModuleAssistant.cpp
+++ modularize/ModuleAssistant.cpp
@@ -68,7 +68,7 @@
// Destructor.
Module::~Module() {
// Free submodules.
- while (SubModules.size()) {
+ while (!SubModules.empty()) {
Module *last = SubModules.back();
SubModules.pop_back();
delete last;
Index: module-map-checker/ModuleMapChecker.cpp
===================================================================
--- module-map-checker/ModuleMapChecker.cpp
+++ module-map-checker/ModuleMapChecker.cpp
@@ -261,7 +261,7 @@
findUnaccountedForHeaders();
// Check for warnings.
- if (UnaccountedForHeaders.size())
+ if (!UnaccountedForHeaders.empty())
returnValue = std::error_code(1, std::generic_category());
// Dump module map if requested.
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,71 @@
+// 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-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: vect.empty()
+ if(vect.size() != 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: !vect.empty()
+ if(0 == vect.size());
+ // 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: vect.empty()
+ if(0 != vect.size());
+ // 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: !vect.empty()
+ if(vect.size() > 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: !vect.empty()
+ if(0 < vect.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: !vect.empty()
+ if(vect.size() < 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: vect.empty()
+ if(1 > vect.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: vect.empty()
+ if(vect.size() >= 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: !vect.empty()
+ if(1 <= vect.size());
+ // 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: !vect.empty()
+ if(!vect.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: vect.empty()
+ if(vect.size());
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: !vect.empty()
+
+ if(vect.empty());
+
+ const std::vector<int> vect2;
+ if(vect2.size() != 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: !vect2.empty()
+
+ std::vector<int> *vect3 = new std::vector<int>();
+ if(vect3->size() == 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: vect3->empty()
+
+ delete vect3;
+
+ const std::vector<int> &vect4 = vect2;
+ if(vect4.size() == 0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: The 'empty' method should be used to check for emptiness instead of 'size'. [readability-container-size-empty]
+ // CHECK-FIXES: vect4.empty()
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
