baloghadamsoftware updated this revision to Diff 168274.
baloghadamsoftware added a comment.
Herald added a subscriber: mgorny.
Updated according to the comments.
https://reviews.llvm.org/D52727
Files:
clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tidy/performance/ForRangeCopyCheck.h
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.h
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tidy/performance/UnnecessaryValueParamCheck.h
clang-tidy/utils/CMakeLists.txt
clang-tidy/utils/Matchers.cpp
clang-tidy/utils/Matchers.h
docs/clang-tidy/checks/performance-for-range-copy.rst
docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
docs/clang-tidy/checks/performance-unnecessary-value-param.rst
test/clang-tidy/performance-for-range-copy-allowed-types.cpp
test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
Index: test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+ ~SmartPointer();
+};
+
+struct smart_pointer {
+ ~smart_pointer();
+};
+
+struct SmartPtr {
+ ~SmartPtr();
+};
+
+struct smart_ptr {
+ ~smart_ptr();
+};
+
+struct SmartReference {
+ ~SmartReference();
+};
+
+struct smart_reference {
+ ~smart_reference();
+};
+
+struct SmartRef {
+ ~SmartRef();
+};
+
+struct smart_ref {
+ ~smart_ref();
+};
+
+struct OtherType {
+ ~OtherType();
+};
+
+void negativeSmartPointer(SmartPointer P) {
+}
+
+void negative_smart_pointer(smart_pointer p) {
+}
+
+void negativeSmartPtr(SmartPtr P) {
+}
+
+void negative_smart_ptr(smart_ptr p) {
+}
+
+void negativeSmartReference(SmartReference R) {
+}
+
+void negative_smart_reference(smart_reference r) {
+}
+
+void negativeSmartRef(SmartRef R) {
+}
+
+void negative_smart_ref(smart_ref r) {
+}
+
+void positiveOtherType(OtherType O) {
+ // CHECK-MESSAGES: [[@LINE-1]]:34: warning: the parameter 'O' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
+ // CHECK-FIXES: void positiveOtherType(const OtherType& O) {
+}
Index: test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
@@ -0,0 +1,85 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t -- -config="{CheckOptions: [{key: performance-unnecessary-copy-initialization.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+ ~SmartPointer();
+};
+
+struct smart_pointer {
+ ~smart_pointer();
+};
+
+struct SmartPtr {
+ ~SmartPtr();
+};
+
+struct smart_ptr {
+ ~smart_ptr();
+};
+
+struct SmartReference {
+ ~SmartReference();
+};
+
+struct smart_reference {
+ ~smart_reference();
+};
+
+struct SmartRef {
+ ~SmartRef();
+};
+
+struct smart_ref {
+ ~smart_ref();
+};
+
+struct OtherType {
+ ~OtherType();
+};
+
+const SmartPointer &getSmartPointer();
+const smart_pointer &get_smart_pointer();
+const SmartPtr &getSmartPtr();
+const smart_ptr &get_smart_ptr();
+const SmartReference &getSmartReference();
+const smart_reference &get_smart_reference();
+const SmartRef &getSmartRef();
+const smart_ref &get_smart_ref();
+const OtherType &getOtherType();
+
+void negativeSmartPointer() {
+ const auto P = getSmartPointer();
+}
+
+void negative_smart_pointer() {
+ const auto p = get_smart_pointer();
+}
+
+void negativeSmartPtr() {
+ const auto P = getSmartPtr();
+}
+
+void negative_smart_ptr() {
+ const auto p = get_smart_ptr();
+}
+
+void negativeSmartReference() {
+ const auto R = getSmartReference();
+}
+
+void negative_smart_reference() {
+ const auto r = get_smart_reference();
+}
+
+void negativeSmartRef() {
+ const auto R = getSmartRef();
+}
+
+void negative_smart_ref() {
+ const auto r = get_smart_ref();
+}
+
+void positiveOtherType() {
+ const auto O = getOtherType();
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
+ // CHECK-FIXES: const auto& O = getOtherType();
+}
Index: test/clang-tidy/performance-for-range-copy-allowed-types.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/performance-for-range-copy-allowed-types.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" -- -std=c++11 -fno-delayed-template-parsing
+
+template <typename T>
+struct Iterator {
+ void operator++() {}
+ const T& operator*() {
+ static T* TT = new T();
+ return *TT;
+ }
+ bool operator!=(const Iterator &) { return false; }
+ typedef const T& const_reference;
+};
+template <typename T>
+struct View {
+ T begin() { return T(); }
+ T begin() const { return T(); }
+ T end() { return T(); }
+ T end() const { return T(); }
+ typedef typename T::const_reference const_reference;
+};
+
+struct SmartPointer {
+ ~SmartPointer();
+};
+
+struct smart_pointer {
+ ~smart_pointer();
+};
+
+struct SmartPtr {
+ ~SmartPtr();
+};
+
+struct smart_ptr {
+ ~smart_ptr();
+};
+
+struct SmartReference {
+ ~SmartReference();
+};
+
+struct smart_reference {
+ ~smart_reference();
+};
+
+struct SmartRef {
+ ~SmartRef();
+};
+
+struct smart_ref {
+ ~smart_ref();
+};
+
+struct OtherType {
+ ~OtherType();
+};
+
+void negativeSmartPointer() {
+ for (auto P : View<Iterator<SmartPointer>>()) {
+ auto P2 = P;
+ }
+}
+
+void negative_smart_pointer() {
+ for (auto p : View<Iterator<smart_pointer>>()) {
+ auto p2 = p;
+ }
+}
+
+void negativeSmartPtr() {
+ for (auto P : View<Iterator<SmartPtr>>()) {
+ auto P2 = P;
+ }
+}
+
+void negative_smart_ptr() {
+ for (auto p : View<Iterator<smart_ptr>>()) {
+ auto p2 = p;
+ }
+}
+
+void negativeSmartReference() {
+ for (auto R : View<Iterator<SmartReference>>()) {
+ auto R2 = R;
+ }
+}
+
+void negative_smart_reference() {
+ for (auto r : View<Iterator<smart_reference>>()) {
+ auto r2 = r;
+ }
+}
+
+void negativeSmartRef() {
+ for (auto R : View<Iterator<SmartRef>>()) {
+ auto R2 = R;
+ }
+}
+
+void negative_smart_ref() {
+ for (auto r : View<Iterator<smart_ref>>()) {
+ auto r2 = r;
+ }
+}
+
+void positiveOtherType() {
+ for (auto O : View<Iterator<OtherType>>()) {
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
+ // CHECK-FIXES: for (const auto& O : View<Iterator<OtherType>>()) {
+ auto O2 = O;
+ }
+}
Index: docs/clang-tidy/checks/performance-unnecessary-value-param.rst
===================================================================
--- docs/clang-tidy/checks/performance-unnecessary-value-param.rst
+++ docs/clang-tidy/checks/performance-unnecessary-value-param.rst
@@ -61,3 +61,9 @@
A string specifying which include-style is used, `llvm` or `google`. Default
is `llvm`.
+
+.. option:: AllowedTypes
+
+ A semicolon-separated list of names of types allowed to be passed by value.
+ Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches every type
+ with suffix `Ref`, `ref`, `Reference` and `reference`. The default is empty.
Index: docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
===================================================================
--- docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
+++ docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
@@ -35,3 +35,13 @@
string UnnecessaryCopy2 = UnnecessaryCopy1;
UnnecessaryCopy2.find("bar");
}
+
+Options
+-------
+
+.. option:: AllowedTypes
+
+ A semicolon-separated list of names of types allowed to be initialized by
+ copying. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
+ every type with suffix `Ref`, `ref`, `Reference` and `reference`. The
+ default is empty.
Index: docs/clang-tidy/checks/performance-for-range-copy.rst
===================================================================
--- docs/clang-tidy/checks/performance-for-range-copy.rst
+++ docs/clang-tidy/checks/performance-for-range-copy.rst
@@ -25,3 +25,10 @@
When non-zero, warns on any use of `auto` as the type of the range-based for
loop variable. Default is `0`.
+
+.. option:: AllowedTypes
+
+ A semicolon-separated list of names of types allowed to be copied in each
+ iteration. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches
+ every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default
+ is empty.
Index: clang-tidy/utils/Matchers.h
===================================================================
--- clang-tidy/utils/Matchers.h
+++ clang-tidy/utils/Matchers.h
@@ -48,6 +48,11 @@
return referenceType(pointee(qualType(isConstQualified())));
}
+using ast_matchers::internal::Matcher;
+
+Matcher<NamedDecl>
+matchesAnyListedName(const std::vector<std::string> &NameList);
+
} // namespace matchers
} // namespace tidy
} // namespace clang
Index: clang-tidy/utils/Matchers.cpp
===================================================================
--- /dev/null
+++ clang-tidy/utils/Matchers.cpp
@@ -0,0 +1,34 @@
+//===--- Matchers.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 "Matchers.h"
+
+namespace clang {
+namespace tidy {
+namespace matchers {
+
+using ast_matchers::matchesName;
+
+Matcher<NamedDecl>
+matchesAnyListedName(const std::vector<std::string> &NameList) {
+ SmallString<256> NameRegEx;
+ llvm::raw_svector_ostream NameOut(NameRegEx);
+ for (const std::string &Name : NameList) {
+ if (!NameRegEx.empty())
+ NameOut << '|';
+ NameOut << "(" << Name << ")";
+ }
+ if (NameRegEx.empty())
+ NameOut << "^(?!.*)";
+ return matchesName(NameRegEx.str());
+}
+
+} // namespace matchers
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/utils/CMakeLists.txt
===================================================================
--- clang-tidy/utils/CMakeLists.txt
+++ clang-tidy/utils/CMakeLists.txt
@@ -10,6 +10,7 @@
IncludeInserter.cpp
IncludeSorter.cpp
LexerUtils.cpp
+ Matchers.cpp
NamespaceAliaser.cpp
OptionsUtils.cpp
TypeTraits.cpp
Index: clang-tidy/performance/UnnecessaryValueParamCheck.h
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ clang-tidy/performance/UnnecessaryValueParamCheck.h
@@ -40,6 +40,7 @@
MutationAnalyzers;
std::unique_ptr<utils::IncludeInserter> Inserter;
const utils::IncludeSorter::IncludeStyle IncludeStyle;
+ const std::vector<std::string> AllowedTypes;
};
} // namespace performance
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -12,6 +12,7 @@
#include "../utils/DeclRefExprUtils.h"
#include "../utils/FixItHintUtils.h"
#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "../utils/TypeTraits.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Lexer.h"
@@ -68,17 +69,22 @@
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
- Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+ Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
+ AllowedTypes(
+ utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
// This check is specific to C++ and doesn't apply to languages like
// Objective-C.
if (!getLangOpts().CPlusPlus)
return;
- const auto ExpensiveValueParamDecl =
- parmVarDecl(hasType(hasCanonicalType(allOf(
- unless(referenceType()), matchers::isExpensiveToCopy()))),
- decl().bind("param"));
+ const auto ExpensiveValueParamDecl = parmVarDecl(
+ hasType(hasCanonicalType(allOf(
+ unless(anyOf(referenceType(),
+ hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(AllowedTypes))))),
+ matchers::isExpensiveToCopy()))),
+ decl().bind("param"));
Finder->addMatcher(
functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
@@ -173,6 +179,8 @@
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IncludeStyle",
utils::IncludeSorter::toString(IncludeStyle));
+ Options.store(Opts, "AllowedTypes",
+ utils::options::serializeStringList(AllowedTypes));
}
void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
Index: clang-tidy/performance/UnnecessaryCopyInitialization.h
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -26,18 +26,19 @@
// const references, and const pointers to const.
class UnnecessaryCopyInitialization : public ClangTidyCheck {
public:
- UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context);
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
private:
void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
bool IssueFix, const VarDecl *ObjectArg,
ASTContext &Context);
void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
const Stmt &BlockStmt, bool IssueFix,
ASTContext &Context);
+ const std::vector<std::string> AllowedTypes;
};
} // namespace performance
Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -12,6 +12,7 @@
#include "../utils/DeclRefExprUtils.h"
#include "../utils/FixItHintUtils.h"
#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
namespace clang {
namespace tidy {
@@ -30,6 +31,12 @@
using namespace ::clang::ast_matchers;
using utils::decl_ref_expr::isOnlyUsedAsConst;
+UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ AllowedTypes(
+ utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
+
void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
auto ConstReference = referenceType(pointee(qualType(isConstQualified())));
auto ConstOrConstReference =
@@ -50,12 +57,16 @@
callExpr(callee(functionDecl(returns(ConstReference))),
unless(callee(cxxMethodDecl())));
- auto localVarCopiedFrom = [](const internal::Matcher<Expr> &CopyCtorArg) {
+ auto localVarCopiedFrom = [this](const internal::Matcher<Expr> &CopyCtorArg) {
return compoundStmt(
forEachDescendant(
declStmt(
has(varDecl(hasLocalStorage(),
- hasType(matchers::isExpensiveToCopy()),
+ hasType(hasCanonicalType(
+ allOf(matchers::isExpensiveToCopy(),
+ unless(hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(
+ AllowedTypes))))))),
unless(isImplicit()),
hasInitializer(
cxxConstructExpr(
@@ -84,6 +95,7 @@
const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>("objectArg");
const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+
// Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
// since we cannot place them correctly.
bool IssueFix =
@@ -144,6 +156,12 @@
recordFixes(NewVar, Context, Diagnostic);
}
+void UnnecessaryCopyInitialization::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "AllowedTypes",
+ utils::options::serializeStringList(AllowedTypes));
+}
+
} // namespace performance
} // namespace tidy
} // namespace clang
Index: clang-tidy/performance/ForRangeCopyCheck.h
===================================================================
--- clang-tidy/performance/ForRangeCopyCheck.h
+++ clang-tidy/performance/ForRangeCopyCheck.h
@@ -40,6 +40,7 @@
ASTContext &Context);
const bool WarnOnAllAutoCopies;
+ const std::vector<std::string> AllowedTypes;
};
} // namespace performance
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -10,6 +10,8 @@
#include "ForRangeCopyCheck.h"
#include "../utils/DeclRefExprUtils.h"
#include "../utils/FixItHintUtils.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
#include "../utils/TypeTraits.h"
#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
@@ -21,26 +23,34 @@
ForRangeCopyCheck::ForRangeCopyCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)) {}
+ WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)),
+ AllowedTypes(
+ utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
void ForRangeCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "WarnOnAllAutoCopies", WarnOnAllAutoCopies);
+ Options.store(Opts, "AllowedTypes",
+ utils::options::serializeStringList(AllowedTypes));
}
void ForRangeCopyCheck::registerMatchers(MatchFinder *Finder) {
// Match loop variables that are not references or pointers or are already
// initialized through MaterializeTemporaryExpr which indicates a type
// conversion.
auto LoopVar = varDecl(
- hasType(hasCanonicalType(unless(anyOf(referenceType(), pointerType())))),
+ hasType(hasCanonicalType(
+ unless(anyOf(referenceType(), pointerType(),
+ hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(AllowedTypes))))))),
unless(hasInitializer(expr(hasDescendant(materializeTemporaryExpr())))));
Finder->addMatcher(cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
.bind("forRange"),
this);
}
void ForRangeCopyCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Var = Result.Nodes.getNodeAs<VarDecl>("loopVar");
+
// Ignore code in macros since we can't place the fixes correctly.
if (Var->getBeginLoc().isMacroID())
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
