Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -12,6 +12,7 @@ MiscTidyModule.cpp MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp + SizeofContainerCheck.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp UndelegatedConstructor.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ 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 @@ "misc-move-constructor-init"); CheckFactories.registerCheck( "misc-noexcept-move-constructor"); + CheckFactories.registerCheck( + "misc-sizeof-container"); CheckFactories.registerCheck( "misc-static-assert"); CheckFactories.registerCheck( Index: clang-tidy/misc/SizeofContainerCheck.h =================================================================== --- /dev/null +++ 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 + Index: clang-tidy/misc/SizeofContainerCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/SizeofContainerCheck.cpp @@ -0,0 +1,76 @@ +//===--- 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(sizeOfExpr( + has(expr(hasType(hasCanonicalType(hasDeclaration(recordDecl( + matchesName("std::(basic_string|vector)|::string"))))))))) + .bind("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 + Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -31,6 +31,7 @@ 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 @@ 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 Index: docs/clang-tidy/checks/misc-sizeof-container.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/misc-sizeof-container.rst @@ -0,0 +1,17 @@ +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. Index: test/clang-tidy/misc-sizeof-container.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-sizeof-container.cpp @@ -0,0 +1,42 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-sizeof-container %t + +namespace std { +template +class basic_string {}; + +template +basic_string operator+(const basic_string &, const T *); + +typedef basic_string string; + +template +class vector {}; +} + +class string {}; + +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(a); + a = sizeof(int); + a = sizeof(std::string); + a = sizeof(std::vector); + + (void)a; +}