Author: alexfh Date: Thu Sep 10 11:37:46 2015 New Revision: 247297 URL: http://llvm.org/viewvc/llvm-project?rev=247297&view=rev Log: [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 Added: clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=247297&r1=247296&r2=247297&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Thu Sep 10 11:37:46 2015 @@ -12,6 +12,7 @@ add_clang_library(clangTidyMiscModule MiscTidyModule.cpp MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp + SizeofContainerCheck.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp UndelegatedConstructor.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=247297&r1=247296&r2=247297&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Thu Sep 10 11:37:46 2015 @@ -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<NoexceptMoveConstructorCheck>( "misc-noexcept-move-constructor"); + CheckFactories.registerCheck<SizeofContainerCheck>( + "misc-sizeof-container"); CheckFactories.registerCheck<StaticAssertCheck>( "misc-static-assert"); CheckFactories.registerCheck<SwappedArgumentsCheck>( Added: clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp?rev=247297&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp (added) +++ clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp Thu Sep 10 11:37:46 2015 @@ -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<BinaryOperator>(E) || isa<ConditionalOperator>(E)) + return true; + if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(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(<array of containers>) 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<UnaryExprOrTypeTraitExpr>("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<ParenExpr>(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 + Added: clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h?rev=247297&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h (added) +++ clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h Thu Sep 10 11:37:46 2015 @@ -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 + Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=247297&r1=247296&r2=247297&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Sep 10 11:37:46 2015 @@ -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 Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst?rev=247297&view=auto ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst (added) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst Thu Sep 10 11:37:46 2015 @@ -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<T>`` 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. Added: clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp?rev=247297&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp Thu Sep 10 11:37:46 2015 @@ -0,0 +1,92 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-sizeof-container %t + +namespace std { + +typedef unsigned int size_t; + +template <typename T> +struct basic_string { + size_t size() const; +}; + +template <typename T> +basic_string<T> operator+(const basic_string<T> &, const T *); + +typedef basic_string<char> string; + +template <typename T> +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<size_t>(!(sizeof(a) % sizeof(*(a))))) + +#define ARRAYSIZE2(a) \ + (((sizeof(a)) / (sizeof(*(a)))) / static_cast<size_t>(!((sizeof(a)) % (sizeof(*(a)))))) + +struct string { + std::size_t size() const; +}; + +template<typename T> +void g(T t) { + (void)sizeof(t); +} + +void f() { + string s1; + std::string s2; + std::vector<int> 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<int>{}); +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: sizeof() doesn't return the size +// CHECK-FIXES: a = std::vector<int>{}.size(); + + a = sizeof(a); + a = sizeof(int); + a = sizeof(std::string); + a = sizeof(std::vector<int>); + + 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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits