baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, flx.
baloghadamsoftware added a project: clang-tools-extra.
baloghadamsoftware added a comment.
This is an alternative approach to https://reviews.llvm.org/D52315.
The three checks mentioned in the Title are two noisy if the code uses custom
(e.g. smart) pointers or references. LLVM/Clang is such a code, it has lots of
such types, e.g. StringRef, SymbolRef or ProgramStateRef, all of them based
llvm::IntrusiveRefCntPtr. Every time such a type is passed as parameter, used
for initialization or in a range-based for loop a false positive warning is
issued together with an (unnecessary) fix.
This patch excludes pointer types (e.g. smart pointers) by their name: if the
name of the type ends on `Pointer`, `pointer`, `Ptr`, `ptr`, `Reference`,
`reference`, `Ref` or `ref` the type is considered a pointer/reference type.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52360
Files:
clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
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.cpp
test/clang-tidy/performance-unnecessary-copy-initialization.cpp
test/clang-tidy/performance-unnecessary-value-param.cpp
Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -55,6 +55,38 @@
~ExpensiveMovableType();
};
+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();
+};
+
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
// CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj);
void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
@@ -381,3 +413,27 @@
// CHECK-FIXES: void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
E.constReference();
}
+
+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) {
+}
Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -20,11 +20,52 @@
bool constMethod() const;
};
+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();
+};
+
ExpensiveToCopyType global_expensive_to_copy_type;
const ExpensiveToCopyType &ExpensiveTypeReference();
const TrivialToCopyType &TrivialTypeReference();
+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();
+
void mutate(ExpensiveToCopyType &);
void mutate(ExpensiveToCopyType *);
void useAsConstPointer(const ExpensiveToCopyType *);
@@ -387,3 +428,35 @@
for (const Element &E : Container()) {
}
}
+
+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();
+}
Index: test/clang-tidy/performance-for-range-copy.cpp
===================================================================
--- test/clang-tidy/performance-for-range-copy.cpp
+++ test/clang-tidy/performance-for-range-copy.cpp
@@ -52,6 +52,38 @@
}
};
+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();
+};
+
void negativeConstReference() {
for (const S &S1 : View<Iterator<S>>()) {
}
@@ -270,3 +302,51 @@
for (auto _ : View<Iterator<S>>()) {
}
}
+
+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;
+ }
+}
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
@@ -10,6 +10,11 @@
which means they are not trivially copyable or have a non-trivial copy
constructor or destructor.
+To avoid too many false-positives the check ignores types considered as pointer
+or reference types (e.g. smart pointers) based on their name. The name of these
+types must end to either `Pointer`, `pointer`, `Ptr`, `ptr`, `Reference`,
+`reference`, `Ref`, `ref`.
+
To ensure that it is safe to replace the value parameter with a const reference
the following heuristic is employed:
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
@@ -7,6 +7,11 @@
constructor of a non-trivially-copyable type but it would suffice to obtain a
const reference.
+To avoid too many false-positives the check ignores types considered as pointer
+or reference types (e.g. smart pointers) based on their name. The name of these
+types must end to either `Pointer`, `pointer`, `Ptr`, `ptr`, `Reference`,
+`reference`, `Ref`, `ref`.
+
The check is only applied if it is safe to replace the copy by a const
reference. This is the case when the variable is const qualified or when it is
only used as a const, i.e. only const methods or operators are invoked on it, or
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
@@ -10,6 +10,11 @@
which means they are not trivially copyable or have a non-trivial copy
constructor or destructor.
+To avoid too many false-positives the check ignores types considered as pointer
+or reference types (e.g. smart pointers) based on their name. The name of these
+types must end to either `Pointer`, `pointer`, `Ptr`, `ptr`, `Reference`,
+`reference`, `Ref`, `ref`.
+
To ensure that it is safe to replace the copy with a const reference the
following heuristic is employed:
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -75,10 +75,13 @@
// 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(matchesName(
+ "[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$"))))),
+ matchers::isExpensiveToCopy()))),
+ decl().bind("param"));
Finder->addMatcher(
functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -55,7 +55,11 @@
forEachDescendant(
declStmt(
has(varDecl(hasLocalStorage(),
- hasType(matchers::isExpensiveToCopy()),
+ hasType(hasCanonicalType(allOf(
+ matchers::isExpensiveToCopy(),
+ unless(hasDeclaration(namedDecl(
+ matchesName("[Pp]ointer$|[Pp]tr$|["
+ "Rr]ef(erence)?$"))))))),
unless(isImplicit()),
hasInitializer(
cxxConstructExpr(
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -32,7 +32,10 @@
// 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(matchesName(
+ "[Pp]ointer$|[Pp]tr$|[Rr]ef(erence)?$"))))))),
unless(hasInitializer(expr(hasDescendant(materializeTemporaryExpr())))));
Finder->addMatcher(cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
.bind("forRange"),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
