Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[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
- Loading branch information
Showing
7 changed files
with
236 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83 changes: 83 additions & 0 deletions
83
clang-tools-extra/clang-tidy/misc/SizeofContainerCheck.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
clang-tools-extra/docs/clang-tidy/checks/misc-sizeof-container.rst
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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. |
92 changes: 92 additions & 0 deletions
92
clang-tools-extra/test/clang-tidy/misc-sizeof-container.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
} |