[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
This commit is contained in:
parent
e3b1f2b765
commit
7532d3e93d
|
@ -12,6 +12,7 @@ add_clang_library(clangTidyMiscModule
|
|||
MiscTidyModule.cpp
|
||||
MoveConstructorInitCheck.cpp
|
||||
NoexceptMoveConstructorCheck.cpp
|
||||
SizeofContainerCheck.cpp
|
||||
StaticAssertCheck.cpp
|
||||
SwappedArgumentsCheck.cpp
|
||||
UndelegatedConstructor.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<NoexceptMoveConstructorCheck>(
|
||||
"misc-noexcept-move-constructor");
|
||||
CheckFactories.registerCheck<SizeofContainerCheck>(
|
||||
"misc-sizeof-container");
|
||||
CheckFactories.registerCheck<StaticAssertCheck>(
|
||||
"misc-static-assert");
|
||||
CheckFactories.registerCheck<SwappedArgumentsCheck>(
|
||||
|
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
@ -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.
|
|
@ -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;
|
||||
}
|
Loading…
Reference in New Issue