[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGba4411e7c6a5: [clang-tidy]
performance-unnecessary-copy-initialization: Fix false negative. (authored by
courbet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114249/new/
https://reviews.llvm.org/D114249
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -3,11 +3,19 @@
template
struct Iterator {
void operator++();
- const T *() const;
+ T *() const;
bool operator!=(const Iterator &) const;
typedef const T _reference;
};
+template
+struct ConstIterator {
+ void operator++();
+ const T *() const;
+ bool operator!=(const ConstIterator &) const;
+ typedef const T _reference;
+};
+
struct ExpensiveToCopyType {
ExpensiveToCopyType();
virtual ~ExpensiveToCopyType();
@@ -15,8 +23,6 @@
using ConstRef = const ExpensiveToCopyType &;
ConstRef referenceWithAlias() const;
const ExpensiveToCopyType *pointer() const;
- Iterator begin() const;
- Iterator end() const;
void nonConstMethod();
bool constMethod() const;
template
@@ -24,6 +30,21 @@
operator int() const; // Implicit conversion to int.
};
+template
+struct Container {
+ bool empty() const;
+ const T& operator[](int) const;
+ const T& operator[](int);
+ Iterator begin();
+ Iterator end();
+ ConstIterator begin() const;
+ ConstIterator end() const;
+ void nonConstMethod();
+ bool constMethod() const;
+};
+
+using ExpensiveToCopyContainerAlias = Container;
+
struct TrivialToCopyType {
const TrivialToCopyType () const;
};
@@ -138,6 +159,94 @@
VarCopyConstructed.constMethod();
}
+void PositiveOperatorCallConstReferenceParam(const Container ) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParam(const Container C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParamAlias(const ExpensiveToCopyContainerAlias C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
courbet updated this revision to Diff 389197.
courbet added a comment.
add container exclusion tests for operators.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114249/new/
https://reviews.llvm.org/D114249
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -3,11 +3,19 @@
template
struct Iterator {
void operator++();
- const T *() const;
+ T *() const;
bool operator!=(const Iterator &) const;
typedef const T _reference;
};
+template
+struct ConstIterator {
+ void operator++();
+ const T *() const;
+ bool operator!=(const ConstIterator &) const;
+ typedef const T _reference;
+};
+
struct ExpensiveToCopyType {
ExpensiveToCopyType();
virtual ~ExpensiveToCopyType();
@@ -15,8 +23,6 @@
using ConstRef = const ExpensiveToCopyType &;
ConstRef referenceWithAlias() const;
const ExpensiveToCopyType *pointer() const;
- Iterator begin() const;
- Iterator end() const;
void nonConstMethod();
bool constMethod() const;
template
@@ -24,6 +30,21 @@
operator int() const; // Implicit conversion to int.
};
+template
+struct Container {
+ bool empty() const;
+ const T& operator[](int) const;
+ const T& operator[](int);
+ Iterator begin();
+ Iterator end();
+ ConstIterator begin() const;
+ ConstIterator end() const;
+ void nonConstMethod();
+ bool constMethod() const;
+};
+
+using ExpensiveToCopyContainerAlias = Container;
+
struct TrivialToCopyType {
const TrivialToCopyType () const;
};
@@ -138,6 +159,94 @@
VarCopyConstructed.constMethod();
}
+void PositiveOperatorCallConstReferenceParam(const Container ) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParam(const Container C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParamAlias(const ExpensiveToCopyContainerAlias C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
flx accepted this revision.
flx added inline comments.
This revision is now accepted and ready to land.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:228-232
+void PositiveOperatorCallConstValueParam(const Container*
C) {
+ const auto AutoAssigned = (*C)[42];
+ // TODO-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
+ // TODO-FIXES: const auto& AutoAssigned = (*C)[42];
+ AutoAssigned.constMethod();
Would you mind adding this test also to the file testing the exclusion of
containers:
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization-excluded-container-types.cpp
This would cover whether the type matching of pointer types works for excluded
container types.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114249/new/
https://reviews.llvm.org/D114249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
courbet updated this revision to Diff 389184.
courbet added a comment.
Canonicalize more types and add more container tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114249/new/
https://reviews.llvm.org/D114249
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -3,11 +3,19 @@
template
struct Iterator {
void operator++();
- const T *() const;
+ T *() const;
bool operator!=(const Iterator &) const;
typedef const T _reference;
};
+template
+struct ConstIterator {
+ void operator++();
+ const T *() const;
+ bool operator!=(const ConstIterator &) const;
+ typedef const T _reference;
+};
+
struct ExpensiveToCopyType {
ExpensiveToCopyType();
virtual ~ExpensiveToCopyType();
@@ -15,8 +23,6 @@
using ConstRef = const ExpensiveToCopyType &;
ConstRef referenceWithAlias() const;
const ExpensiveToCopyType *pointer() const;
- Iterator begin() const;
- Iterator end() const;
void nonConstMethod();
bool constMethod() const;
template
@@ -24,6 +30,21 @@
operator int() const; // Implicit conversion to int.
};
+template
+struct Container {
+ bool empty() const;
+ const T& operator[](int) const;
+ const T& operator[](int);
+ Iterator begin();
+ Iterator end();
+ ConstIterator begin() const;
+ ConstIterator end() const;
+ void nonConstMethod();
+ bool constMethod() const;
+};
+
+using ExpensiveToCopyContainerAlias = Container;
+
struct TrivialToCopyType {
const TrivialToCopyType () const;
};
@@ -138,6 +159,94 @@
VarCopyConstructed.constMethod();
}
+void PositiveOperatorCallConstReferenceParam(const Container ) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParam(const Container C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParamAlias(const ExpensiveToCopyContainerAlias C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning:
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
courbet updated this revision to Diff 388821.
courbet added a comment.
rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114249/new/
https://reviews.llvm.org/D114249
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
---
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -24,6 +24,11 @@
operator int() const; // Implicit conversion to int.
};
+struct ExpensiveToCopyContainer {
+ const ExpensiveToCopyType [](int) const;
+ const ExpensiveToCopyType [](int);
+};
+
struct TrivialToCopyType {
const TrivialToCopyType () const;
};
@@ -138,6 +143,50 @@
VarCopyConstructed.constMethod();
}
+void PositiveOperatorCallConstReferenceParam(const ExpensiveToCopyContainer
) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParam(const ExpensiveToCopyContainer C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
void PositiveLocalConstValue() {
const ExpensiveToCopyType Obj;
const auto UnnecessaryCopy = Obj.reference();
Index:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -83,13 +83,19 @@
// variable being declared. The assumption is that the const reference being
// returned either points to a global static variable or to a member of the
// called object.
- return cxxMemberCallExpr(
- callee(cxxMethodDecl(
- returns(hasCanonicalType(matchers::isReferenceToConst(
- .bind(MethodDeclId)),
- on(declRefExpr(to(varDecl().bind(ObjectArgId,
- thisPointerType(namedDecl(
- unless(matchers::matchesAnyListedName(ExcludedContainerTypes);
+ const auto MethodDecl =
+ cxxMethodDecl(returns(hasCanonicalType(matchers::isReferenceToConst(
+ .bind(MethodDeclId);
+ const auto ReceiverExpr = declRefExpr(to(varDecl().bind(ObjectArgId)));
+ const auto ReceiverTypeDecl =
+
namedDecl(unless(matchers::matchesAnyListedName(ExcludedContainerTypes)));
+
+ return expr(anyOf(
+ cxxMemberCallExpr(callee(MethodDecl), on(ReceiverExpr),
+thisPointerType(ReceiverTypeDecl)),
+ cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, ReceiverExpr),
+ hasArgument(0, hasType(recordType(hasDeclaration(
+ ReceiverTypeDecl)));
}
AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) {
Index:
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
courbet added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:97 + cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, ReceiverExpr), + hasArgument(0, hasType(recordType(hasDeclaration( + ReceiverTypeDecl))); flx wrote: > Does this work if if the object argument is a pointer or a type alias? Could > you add a test to confirm? It does not work with a pointer. I plan to make it work in a separate cl, but first I need to fix `isOnlyUsedAsConst` to make more general (specifically, handle unary `*`). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114249/new/ https://reviews.llvm.org/D114249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
flx added a comment. Thank you for catching and fixing this! Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:97 + cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, ReceiverExpr), + hasArgument(0, hasType(recordType(hasDeclaration( + ReceiverTypeDecl))); Does this work if if the object argument is a pointer or a type alias? Could you add a test to confirm? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114249/new/ https://reviews.llvm.org/D114249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D114249: [clang-tidy] performance-unnecessary-copy-initialization: Fix false negative.
courbet created this revision.
courbet added reviewers: flx, aaron.ballman.
Herald added subscribers: carlosgalvezp, xazax.hun.
courbet requested review of this revision.
Herald added a project: clang-tools-extra.
`isConstRefReturningMethodCall` should be considering
`CXXOperatorCallExpr` in addition to `CXXMemberCallExpr`. Clang considers
these to be distinct (`CXXOperatorCallExpr` derives from `CallExpr`, not
`CXXMemberCallExpr`), but we don't care in the context of this
check.
This is important because of
`std::vector::operator[](size_t) const`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114249
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
---
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -24,6 +24,11 @@
operator int() const; // Implicit conversion to int.
};
+struct ExpensiveToCopyContainer {
+ const ExpensiveToCopyType [](int) const;
+ const ExpensiveToCopyType [](int);
+};
+
struct TrivialToCopyType {
const TrivialToCopyType () const;
};
@@ -138,6 +143,50 @@
VarCopyConstructed.constMethod();
}
+void PositiveOperatorCallConstReferenceParam(const ExpensiveToCopyContainer
) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
+void PositiveOperatorCallConstValueParam(const ExpensiveToCopyContainer C) {
+ const auto AutoAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = C[42];
+ AutoAssigned.constMethod();
+
+ const auto AutoCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed(C[42]);
+ AutoCopyConstructed.constMethod();
+
+ const ExpensiveToCopyType VarAssigned = C[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C[42];
+ VarAssigned.constMethod();
+
+ const ExpensiveToCopyType VarCopyConstructed(C[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C[42]);
+ VarCopyConstructed.constMethod();
+}
+
void PositiveLocalConstValue() {
const ExpensiveToCopyType Obj;
const auto UnnecessaryCopy = Obj.reference();
Index:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -83,15 +83,19 @@
// variable being declared. The assumption is that the const reference being
// returned either points to a global static variable or to a member of the
// called object.
- return cxxMemberCallExpr(
- callee(cxxMethodDecl(
- returns(hasCanonicalType(matchers::isReferenceToConst(
- .bind(MethodDeclId)),
- on(declRefExpr(to(
- varDecl(
-
unless(hasType(qualType(hasCanonicalType(hasDeclaration(namedDecl(
- matchers::matchesAnyListedName(ExcludedContainerTypes
- .bind(ObjectArgId);
+ const auto MethodDecl =
+ cxxMethodDecl(returns(hasCanonicalType(matchers::isReferenceToConst(
+ .bind(MethodDeclId);
+ const auto ReceiverExpr = declRefExpr(to(varDecl().bind(ObjectArgId)));
+ const auto ReceiverTypeDecl =
+
namedDecl(unless(matchers::matchesAnyListedName(ExcludedContainerTypes)));
+
+ return expr(anyOf(
+
