From 7532d3e93d3980d628f10c63c2bc97bca2e7c2c1 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Thu, 10 Sep 2015 16:37:46 +0000 Subject: [PATCH] [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stl containers. Summary: sizeof(some_std_string) is likely to be an error. This check finds this pattern and suggests using .size() instead. Reviewers: djasper, klimek, aaron.ballman Subscribers: aaron.ballman, cfe-commits Differential Revision: http://reviews.llvm.org/D12759 llvm-svn: 247297 --- clang-tools-extra/clang-tidy/misc/CMakeLists.txt | 1 + .../clang-tidy/misc/MiscTidyModule.cpp | 3 + .../clang-tidy/misc/SizeofContainerCheck.cpp | 83 +++++++++++++++++++ .../clang-tidy/misc/SizeofContainerCheck.h | 35 ++++++++ clang-tools-extra/docs/clang-tidy/checks/list.rst | 3 +- .../clang-tidy/checks/misc-sizeof-container.rst | 20 +++++ .../test/clang-tidy/misc-sizeof-container.cpp | 92 ++++++++++++++++++++++ 7 files changed, 236 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-container.rst create mode 100644 clang-tools-extra/test/clang-tidy/misc-sizeof-container.cpp diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index 0e69a80..6cdd621 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -12,6 +12,7 @@ add_clang_library(clangTidyMiscModule MiscTidyModule.cpp MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp + SizeofContainerCheck.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp UndelegatedConstructor.cpp diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index 9976131..dc9a4eb 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -20,6 +20,7 @@ #include "MacroRepeatedSideEffectsCheck.h" #include "MoveConstructorInitCheck.h" #include "NoexceptMoveConstructorCheck.h" +#include "SizeofContainerCheck.h" #include "StaticAssertCheck.h" #include "SwappedArgumentsCheck.h" #include "UndelegatedConstructor.h" @@ -54,6 +55,8 @@ public: "misc-move-constructor-init"); CheckFactories.registerCheck( "misc-noexcept-move-constructor"); + CheckFactories.registerCheck( + "misc-sizeof-container"); CheckFactories.registerCheck( "misc-static-assert"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp new file mode 100644 index 0000000..44753b9 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp @@ -0,0 +1,83 @@ +//===--- SizeofContainerCheck.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 "SizeofContainerCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +namespace { + +bool needsParens(const Expr *E) { + E = E->IgnoreImpCasts(); + if (isa(E) || isa(E)) + return true; + if (const auto *Op = dyn_cast(E)) { + return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && + Op->getOperator() != OO_Subscript; + } + return false; +} + +} // anonymous namespace + +void SizeofContainerCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + expr(unless(isInTemplateInstantiation()), + expr(sizeOfExpr(has(expr(hasType(hasCanonicalType(hasDeclaration( + recordDecl(matchesName("^(::std::|::string)"), + hasMethod(methodDecl(hasName("size"), isPublic(), + isConst())))))))))) + .bind("sizeof"), + // Ignore ARRAYSIZE() pattern. + unless(hasAncestor(binaryOperator( + anyOf(hasOperatorName("/"), hasOperatorName("%")), + hasLHS(ignoringParenCasts(sizeOfExpr(expr()))), + hasRHS(ignoringParenCasts(equalsBoundNode("sizeof"))))))), + this); +} + +void SizeofContainerCheck::check(const MatchFinder::MatchResult &Result) { + const auto *SizeOf = + Result.Nodes.getNodeAs("sizeof"); + + SourceLocation SizeOfLoc = SizeOf->getLocStart(); + auto Diag = diag(SizeOfLoc, "sizeof() doesn't return the size of the " + "container; did you mean .size()?"); + + // Don't generate fixes for macros. + if (SizeOfLoc.isMacroID()) + return; + + SourceLocation RParenLoc = SizeOf->getRParenLoc(); + + // sizeof argument is wrapped in a single ParenExpr. + const auto *Arg = cast(SizeOf->getArgumentExpr()); + + if (needsParens(Arg->getSubExpr())) { + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(SizeOfLoc, SizeOfLoc)) + << FixItHint::CreateInsertion(RParenLoc.getLocWithOffset(1), + ".size()"); + } else { + Diag << FixItHint::CreateRemoval( + CharSourceRange::getTokenRange(SizeOfLoc, Arg->getLParen())) + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(RParenLoc, RParenLoc), + ".size()"); + } +} + +} // namespace tidy +} // namespace clang + diff --git a/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h new file mode 100644 index 0000000..eae5a06 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.h @@ -0,0 +1,35 @@ +//===--- SizeofContainerCheck.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_MISC_SIZEOF_CONTAINER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// Find usages of sizeof on expressions of STL container types. Most likely the +/// user wanted to use `.size()` instead. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-sizeof-container.html +class SizeofContainerCheck : public ClangTidyCheck { +public: + SizeofContainerCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SIZEOF_CONTAINER_H + diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index be7bf3b..600b09d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -31,6 +31,7 @@ List of clang-tidy Checks misc-macro-repeated-side-effects misc-move-constructor-init misc-noexcept-move-constructor + misc-sizeof-container misc-static-assert misc-swapped-arguments misc-undelegated-constructor @@ -54,4 +55,4 @@ List of clang-tidy Checks readability-named-parameter readability-redundant-smartptr-get readability-redundant-string-cstr - readability-simplify-boolean-expr \ No newline at end of file + readability-simplify-boolean-expr diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-container.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-container.rst new file mode 100644 index 0000000..2e880d4 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-container.rst @@ -0,0 +1,20 @@ +misc-sizeof-container +===================== + +The check finds usages of ``sizeof`` on expressions of STL container types. Most +likely the user wanted to use ``.size()`` instead. + +Currently only ``std::string`` and ``std::vector`` are supported. + +Examples: + +.. code:: c++ + + std::string s; + int a = 47 + sizeof(s); // warning: sizeof() doesn't return the size of the container. Did you mean .size()? + // The suggested fix is: int a = 47 + s.size(); + + int b = sizeof(std::string); // no warning, probably intended. + + std::string array_of_strings[10]; + int c = sizeof(array_of_strings) / sizeof(array_of_strings[0]); // no warning, definitely intended. diff --git a/clang-tools-extra/test/clang-tidy/misc-sizeof-container.cpp b/clang-tools-extra/test/clang-tidy/misc-sizeof-container.cpp new file mode 100644 index 0000000..4ef9238 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/misc-sizeof-container.cpp @@ -0,0 +1,92 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-sizeof-container %t + +namespace std { + +typedef unsigned int size_t; + +template +struct basic_string { + size_t size() const; +}; + +template +basic_string operator+(const basic_string &, const T *); + +typedef basic_string string; + +template +struct vector { + size_t size() const; +}; + +class fake_container1 { + size_t size() const; // non-public +}; + +struct fake_container2 { + size_t size(); // non-const +}; + +} + +using std::size_t; + +#define ARRAYSIZE(a) \ + ((sizeof(a) / sizeof(*(a))) / static_cast(!(sizeof(a) % sizeof(*(a))))) + +#define ARRAYSIZE2(a) \ + (((sizeof(a)) / (sizeof(*(a)))) / static_cast(!((sizeof(a)) % (sizeof(*(a)))))) + +struct string { + std::size_t size() const; +}; + +template +void g(T t) { + (void)sizeof(t); +} + +void f() { + string s1; + std::string s2; + std::vector v; + + int a = 42 + sizeof(s1); +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: sizeof() doesn't return the size of the container; did you mean .size()? [misc-sizeof-container] +// CHECK-FIXES: int a = 42 + s1.size(); + a = 123 * sizeof(s2); +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: sizeof() doesn't return the size +// CHECK-FIXES: a = 123 * s2.size(); + a = 45 + sizeof(s2 + "asdf"); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: sizeof() doesn't return the size +// CHECK-FIXES: a = 45 + (s2 + "asdf").size(); + a = sizeof(v); +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size +// CHECK-FIXES: a = v.size(); + a = sizeof(std::vector{}); +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size +// CHECK-FIXES: a = std::vector{}.size(); + + a = sizeof(a); + a = sizeof(int); + a = sizeof(std::string); + a = sizeof(std::vector); + + g(s1); + g(s2); + g(v); + + std::fake_container1 f1; + std::fake_container2 f2; + + a = sizeof(f1); + a = sizeof(f2); + + + std::string arr[3]; + a = ARRAYSIZE(arr); + a = ARRAYSIZE2(arr); + a = sizeof(arr) / sizeof(arr[0]); + + (void)a; +} -- 2.7.4