[clang-tidy] Move implicit-cast-in-loop check to upstream.
Summary: This is implemented originally by Alex Pilkiewicz (pilki@google.com). Reviewers: alexfh Subscribers: cfe-commits Patch by Haojian Wu! Differential Revision: http://reviews.llvm.org/D16721 llvm-svn: 259195
This commit is contained in:
parent
865a7d8aab
commit
40d307d120
|
@ -1,6 +1,7 @@
|
|||
set(LLVM_LINK_COMPONENTS support)
|
||||
|
||||
add_clang_library(clangTidyPerformanceModule
|
||||
ImplicitCastInLoopCheck.cpp
|
||||
PerformanceTidyModule.cpp
|
||||
UnnecessaryCopyInitialization.cpp
|
||||
|
||||
|
|
|
@ -0,0 +1,106 @@
|
|||
//===--- ImplicitCastInLoopCheck.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 "ImplicitCastInLoopCheck.h"
|
||||
|
||||
#include "clang/AST/ASTContext.h"
|
||||
#include "clang/AST/Decl.h"
|
||||
#include "clang/ASTMatchers/ASTMatchFinder.h"
|
||||
#include "clang/ASTMatchers/ASTMatchers.h"
|
||||
#include "clang/Lex/Lexer.h"
|
||||
|
||||
namespace clang {
|
||||
|
||||
using namespace ast_matchers;
|
||||
|
||||
namespace tidy {
|
||||
namespace performance {
|
||||
|
||||
namespace {
|
||||
// Checks if the stmt is a ImplicitCastExpr with a CastKind that is not a NoOp.
|
||||
// The subtelty is that in some cases (user defined conversions), we can
|
||||
// get to ImplicitCastExpr inside each other, with the outer one a NoOp. In this
|
||||
// case we skip the first cast expr.
|
||||
bool IsNonTrivialImplicitCast(const Stmt* ST) {
|
||||
if (const auto* ICE = dyn_cast<ImplicitCastExpr>(ST)) {
|
||||
return (ICE->getCastKind() != CK_NoOp) ||
|
||||
IsNonTrivialImplicitCast(ICE->getSubExpr());
|
||||
}
|
||||
return false;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
void ImplicitCastInLoopCheck::registerMatchers(
|
||||
ast_matchers::MatchFinder* Finder) {
|
||||
// We look for const ref loop variables that (optionally inside an
|
||||
// ExprWithCleanup) materialize a temporary, and contain a implicit cast. The
|
||||
// check on the implicit cast is done in check() because we can't access
|
||||
// implicit cast subnode via matchers: has() skips casts and materialize!
|
||||
// We also bind on the call to operator* to get the proper type in the
|
||||
// diagnostic message.
|
||||
// Note that when the implicit cast is done through a user defined cast
|
||||
// operator, the node is a CXXMemberCallExpr, not a CXXOperatorCallExpr, so
|
||||
// it should not get caught by the cxxOperatorCallExpr() matcher.
|
||||
Finder->addMatcher(
|
||||
cxxForRangeStmt(hasLoopVariable(
|
||||
varDecl(hasType(qualType(references(qualType(isConstQualified())))),
|
||||
hasInitializer(expr(hasDescendant(cxxOperatorCallExpr().bind(
|
||||
"operator-call")))
|
||||
.bind("init")))
|
||||
.bind("faulty-var"))),
|
||||
this);
|
||||
}
|
||||
|
||||
void ImplicitCastInLoopCheck::check(
|
||||
const ast_matchers::MatchFinder::MatchResult &Result) {
|
||||
const auto* VD = Result.Nodes.getNodeAs<VarDecl>("faulty-var");
|
||||
const auto* Init = Result.Nodes.getNodeAs<Expr>("init");
|
||||
const auto* OperatorCall =
|
||||
Result.Nodes.getNodeAs<CXXOperatorCallExpr>("operator-call");
|
||||
|
||||
if (const auto* Cleanup = dyn_cast<ExprWithCleanups>(Init)) {
|
||||
Init = Cleanup->getSubExpr();
|
||||
}
|
||||
const auto* Materialized = dyn_cast<MaterializeTemporaryExpr>(Init);
|
||||
if (!Materialized) {
|
||||
return;
|
||||
}
|
||||
|
||||
// We ignore NoOp casts. Those are generated if the * operator on the
|
||||
// iterator returns a value instead of a reference, and the loop variable
|
||||
// is a reference. This situation is fine (it probably produces the same
|
||||
// code at the end).
|
||||
if (IsNonTrivialImplicitCast(Materialized->getTemporary())) {
|
||||
ReportAndFix(Result.Context, VD, OperatorCall);
|
||||
}
|
||||
}
|
||||
|
||||
void ImplicitCastInLoopCheck::ReportAndFix(
|
||||
const ASTContext *Context, const VarDecl *VD,
|
||||
const CXXOperatorCallExpr *OperatorCall) {
|
||||
// We only match on const ref, so we should print a const ref version of the
|
||||
// type.
|
||||
QualType ConstType = OperatorCall->getType().withConst();
|
||||
QualType ConstRefType = Context->getLValueReferenceType(ConstType);
|
||||
const char Message[] =
|
||||
"the type of the loop variable '%0' is different from the one returned "
|
||||
"by the iterator and generates an implicit cast; you can either "
|
||||
"change the type to the correct one ('%1' but 'const auto&' is always a "
|
||||
"valid option) or remove the reference to make it explicit that you are "
|
||||
"creating a new value";
|
||||
PrintingPolicy Policy(Context->getLangOpts());
|
||||
Policy.SuppressTagKeyword = true;
|
||||
|
||||
diag(VD->getLocStart(), Message) << VD->getName()
|
||||
<< ConstRefType.getAsString(Policy);
|
||||
}
|
||||
|
||||
} // namespace performance
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
|
@ -0,0 +1,37 @@
|
|||
//===--- ImplicitCastInLoopCheck.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_PERFORMANCE_IMPLICIT_CAST_IN_LOOP_CHECK_H_
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_IMPLICIT_CAST_IN_LOOP_CHECK_H_
|
||||
|
||||
#include "../ClangTidy.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace performance {
|
||||
|
||||
// Checks that in a for range loop, if the provided type is a reference, then
|
||||
// the underlying type is the one returned by the iterator (i.e. that there
|
||||
// isn't any implicit conversion).
|
||||
class ImplicitCastInLoopCheck : public ClangTidyCheck {
|
||||
public:
|
||||
using ClangTidyCheck::ClangTidyCheck;
|
||||
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
|
||||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
|
||||
|
||||
private:
|
||||
void ReportAndFix(const ASTContext *Context, const VarDecl *VD,
|
||||
const CXXOperatorCallExpr *OperatorCall);
|
||||
};
|
||||
|
||||
} // namespace performance
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
||||
|
||||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_IMPLICIT_CAST_IN_LOOP_CHECK_H_
|
|
@ -11,6 +11,7 @@
|
|||
#include "../ClangTidyModule.h"
|
||||
#include "../ClangTidyModuleRegistry.h"
|
||||
|
||||
#include "ImplicitCastInLoopCheck.h"
|
||||
#include "UnnecessaryCopyInitialization.h"
|
||||
|
||||
namespace clang {
|
||||
|
@ -20,6 +21,8 @@ namespace performance {
|
|||
class PerformanceModule : public ClangTidyModule {
|
||||
public:
|
||||
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
|
||||
CheckFactories.registerCheck<ImplicitCastInLoopCheck>(
|
||||
"performance-implicit-cast-in-loop");
|
||||
CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
|
||||
"performance-unnecessary-copy-initialization");
|
||||
}
|
||||
|
|
|
@ -76,6 +76,7 @@ Clang-Tidy Checks
|
|||
modernize-use-default
|
||||
modernize-use-nullptr
|
||||
modernize-use-override
|
||||
performance-implicit-cast-in-loop
|
||||
readability-braces-around-statements
|
||||
readability-container-size-empty
|
||||
readability-else-after-return
|
||||
|
|
|
@ -0,0 +1,19 @@
|
|||
performance-implicit-cast-in-loop
|
||||
=================================
|
||||
|
||||
This warning appears in range-based loop with loop variable of const ref type
|
||||
where the type of the variable does not match the one returned by the iterator.
|
||||
This means that an implicit cast has been added, which can for example result in
|
||||
expensive deep copies.
|
||||
|
||||
Example:
|
||||
|
||||
.. code:: c++
|
||||
|
||||
map<int, vector<string>> my_map;
|
||||
for (const pair<int, vector<string>>& p : my_map) {}
|
||||
// The iterator type is in fact pair<const int, vector<string>>, which means
|
||||
// that the compiler added a cast, resulting in a copy of the vectors.
|
||||
|
||||
The easiest solution is usually to use "const auto&" instead of writing the type
|
||||
manually.
|
|
@ -0,0 +1,161 @@
|
|||
// RUN: %check_clang_tidy %s performance-implicit-cast-in-loop %t
|
||||
|
||||
// ---------- Classes used in the tests ----------
|
||||
|
||||
// Iterator returning by value.
|
||||
template <typename T>
|
||||
struct Iterator {
|
||||
void operator++();
|
||||
T operator*();
|
||||
bool operator!=(const Iterator& other);
|
||||
};
|
||||
|
||||
// Iterator returning by reference.
|
||||
template <typename T>
|
||||
struct RefIterator {
|
||||
void operator++();
|
||||
T& operator*();
|
||||
bool operator!=(const RefIterator& other);
|
||||
};
|
||||
|
||||
// The template argument is an iterator type, and a view is an object you can
|
||||
// run a for loop on.
|
||||
template <typename T>
|
||||
struct View {
|
||||
T begin();
|
||||
T end();
|
||||
};
|
||||
|
||||
// With this class, the implicit cast is a call to the (implicit) constructor of
|
||||
// the class.
|
||||
template <typename T>
|
||||
class ImplicitWrapper {
|
||||
public:
|
||||
// Implicit!
|
||||
ImplicitWrapper(const T& t);
|
||||
};
|
||||
|
||||
// With this class, the implicit cast is a call to the conversion operators of
|
||||
// SimpleClass and ComplexClass.
|
||||
template <typename T>
|
||||
class OperatorWrapper {
|
||||
public:
|
||||
explicit OperatorWrapper(const T& t);
|
||||
};
|
||||
|
||||
struct SimpleClass {
|
||||
int foo;
|
||||
operator OperatorWrapper<SimpleClass>();
|
||||
};
|
||||
|
||||
// The materialize expression is not the same when the class has a destructor,
|
||||
// so we make sure we cover that case too.
|
||||
class ComplexClass {
|
||||
public:
|
||||
ComplexClass();
|
||||
~ComplexClass();
|
||||
operator OperatorWrapper<ComplexClass>();
|
||||
};
|
||||
|
||||
typedef View<Iterator<SimpleClass>> SimpleView;
|
||||
typedef View<RefIterator<SimpleClass>> SimpleRefView;
|
||||
typedef View<Iterator<ComplexClass>> ComplexView;
|
||||
typedef View<RefIterator<ComplexClass>> ComplexRefView;
|
||||
|
||||
// ---------- The test themselves ----------
|
||||
// For each test we do, in the same order, const ref, non const ref, const
|
||||
// value, non const value.
|
||||
|
||||
void SimpleClassIterator() {
|
||||
for (const SimpleClass& foo : SimpleView()) {}
|
||||
// This line does not compile because a temporary cannot be assigned to a non
|
||||
// const reference.
|
||||
// for (SimpleClass& foo : SimpleView()) {}
|
||||
for (const SimpleClass foo : SimpleView()) {}
|
||||
for (SimpleClass foo : SimpleView()) {}
|
||||
}
|
||||
|
||||
void SimpleClassRefIterator() {
|
||||
for (const SimpleClass& foo : SimpleRefView()) {}
|
||||
for (SimpleClass& foo : SimpleRefView()) {}
|
||||
for (const SimpleClass foo : SimpleRefView()) {}
|
||||
for (SimpleClass foo : SimpleRefView()) {}
|
||||
}
|
||||
|
||||
void ComplexClassIterator() {
|
||||
for (const ComplexClass& foo : ComplexView()) {}
|
||||
// for (ComplexClass& foo : ComplexView()) {}
|
||||
for (const ComplexClass foo : ComplexView()) {}
|
||||
for (ComplexClass foo : ComplexView()) {}
|
||||
}
|
||||
|
||||
void ComplexClassRefIterator() {
|
||||
for (const ComplexClass& foo : ComplexRefView()) {}
|
||||
for (ComplexClass& foo : ComplexRefView()) {}
|
||||
for (const ComplexClass foo : ComplexRefView()) {}
|
||||
for (ComplexClass foo : ComplexRefView()) {}
|
||||
}
|
||||
|
||||
void ImplicitSimpleClassIterator() {
|
||||
for (const ImplicitWrapper<SimpleClass>& foo : SimpleView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the loop variable 'foo' is different from the one returned by the iterator and generates an implicit cast; you can either change the type to the correct one ('const SimpleClass &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [performance-implicit-cast-in-loop]
|
||||
// for (ImplicitWrapper<SimpleClass>& foo : SimpleView()) {}
|
||||
for (const ImplicitWrapper<SimpleClass> foo : SimpleView()) {}
|
||||
for (ImplicitWrapper<SimpleClass>foo : SimpleView()) {}
|
||||
}
|
||||
|
||||
void ImplicitSimpleClassRefIterator() {
|
||||
for (const ImplicitWrapper<SimpleClass>& foo : SimpleRefView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}}
|
||||
// for (ImplicitWrapper<SimpleClass>& foo : SimpleRefView()) {}
|
||||
for (const ImplicitWrapper<SimpleClass> foo : SimpleRefView()) {}
|
||||
for (ImplicitWrapper<SimpleClass>foo : SimpleRefView()) {}
|
||||
}
|
||||
|
||||
void ImplicitComplexClassIterator() {
|
||||
for (const ImplicitWrapper<ComplexClass>& foo : ComplexView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}}
|
||||
// for (ImplicitWrapper<ComplexClass>& foo : ComplexView()) {}
|
||||
for (const ImplicitWrapper<ComplexClass> foo : ComplexView()) {}
|
||||
for (ImplicitWrapper<ComplexClass>foo : ComplexView()) {}
|
||||
}
|
||||
|
||||
void ImplicitComplexClassRefIterator() {
|
||||
for (const ImplicitWrapper<ComplexClass>& foo : ComplexRefView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}}
|
||||
// for (ImplicitWrapper<ComplexClass>& foo : ComplexRefView()) {}
|
||||
for (const ImplicitWrapper<ComplexClass> foo : ComplexRefView()) {}
|
||||
for (ImplicitWrapper<ComplexClass>foo : ComplexRefView()) {}
|
||||
}
|
||||
|
||||
void OperatorSimpleClassIterator() {
|
||||
for (const OperatorWrapper<SimpleClass>& foo : SimpleView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}}
|
||||
// for (OperatorWrapper<SimpleClass>& foo : SimpleView()) {}
|
||||
for (const OperatorWrapper<SimpleClass> foo : SimpleView()) {}
|
||||
for (OperatorWrapper<SimpleClass>foo : SimpleView()) {}
|
||||
}
|
||||
|
||||
void OperatorSimpleClassRefIterator() {
|
||||
for (const OperatorWrapper<SimpleClass>& foo : SimpleRefView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}}
|
||||
// for (OperatorWrapper<SimpleClass>& foo : SimpleRefView()) {}
|
||||
for (const OperatorWrapper<SimpleClass> foo : SimpleRefView()) {}
|
||||
for (OperatorWrapper<SimpleClass>foo : SimpleRefView()) {}
|
||||
}
|
||||
|
||||
void OperatorComplexClassIterator() {
|
||||
for (const OperatorWrapper<ComplexClass>& foo : ComplexView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}}
|
||||
// for (OperatorWrapper<ComplexClass>& foo : ComplexView()) {}
|
||||
for (const OperatorWrapper<ComplexClass> foo : ComplexView()) {}
|
||||
for (OperatorWrapper<ComplexClass>foo : ComplexView()) {}
|
||||
}
|
||||
|
||||
void OperatorComplexClassRefIterator() {
|
||||
for (const OperatorWrapper<ComplexClass>& foo : ComplexRefView()) {}
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}}
|
||||
// for (OperatorWrapper<ComplexClass>& foo : ComplexRefView()) {}
|
||||
for (const OperatorWrapper<ComplexClass> foo : ComplexRefView()) {}
|
||||
for (OperatorWrapper<ComplexClass>foo : ComplexRefView()) {}
|
||||
}
|
Loading…
Reference in New Issue