[clang-tools-extra] r247297 - [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stl
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 10 09:37:46 PDT 2015
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;
+}
More information about the cfe-commits
mailing list