f00kat created this revision.
f00kat added reviewers: aaron.ballman, alexfh.
f00kat added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Existing 'modernize-pass-by-value' check works only with non template values in
initializers.
Now we also skip cases when the value has type 'TemplateSpecializedType' inside
'cxxConstructExpr'.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75159
Files:
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -118,6 +118,26 @@
J<Movable> j1(Movable());
J<NotMovable> j2(NotMovable());
+template<class T>
+struct MovableTemplateT
+{
+ MovableTemplateT() {}
+ MovableTemplateT(const MovableTemplateT& o) { }
+ MovableTemplateT(MovableTemplateT&& o) { }
+};
+
+template <class T>
+struct J2 {
+ J2(const MovableTemplateT<T>& A);
+ // CHECK-FIXES: J2(const MovableTemplateT<T>& A);
+ MovableTemplateT<T> M;
+};
+
+template <class T>
+J2<T>::J2(const MovableTemplateT<T>& A) : M(A) {}
+// CHECK-FIXES: J2<T>::J2(const MovableTemplateT<T>& A) : M(A) {}
+J2<int> j3(MovableTemplateT<int>{});
+
struct K_Movable {
K_Movable() = default;
K_Movable(const K_Movable &) = default;
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -46,8 +46,9 @@
}
} // namespace
-static TypeMatcher constRefType() {
- return lValueReferenceType(pointee(isConstQualified()));
+static TypeMatcher notTemplateSpecConstRefType() {
+ return lValueReferenceType(
+ pointee(unless(templateSpecializationType()), isConstQualified()));
}
static TypeMatcher nonConstValueType() {
@@ -145,16 +146,18 @@
// ParenListExpr is generated instead of a CXXConstructExpr,
// filtering out templates automatically for us.
withInitializer(cxxConstructExpr(
- has(ignoringParenImpCasts(declRefExpr(to(
- parmVarDecl(
- hasType(qualType(
- // Match only const-ref or a non-const value
- // parameters. Rvalues and const-values
- // shouldn't be modified.
- ValuesOnly ? nonConstValueType()
- : anyOf(constRefType(),
- nonConstValueType()))))
- .bind("Param"))))),
+ has(ignoringParenImpCasts(declRefExpr(
+ to(parmVarDecl(
+ hasType(qualType(
+ // Match only const-ref or a non-const
+ // value parameters. Rvalues,
+ // TemplateSpecializationValues and
+ // const-values shouldn't be modified.
+ ValuesOnly
+ ? nonConstValueType()
+ : anyOf(notTemplateSpecConstRefType(),
+ nonConstValueType()))))
+ .bind("Param"))))),
hasDeclaration(cxxConstructorDecl(
isCopyConstructor(), unless(isDeleted()),
hasDeclContext(
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -118,6 +118,26 @@
J<Movable> j1(Movable());
J<NotMovable> j2(NotMovable());
+template<class T>
+struct MovableTemplateT
+{
+ MovableTemplateT() {}
+ MovableTemplateT(const MovableTemplateT& o) { }
+ MovableTemplateT(MovableTemplateT&& o) { }
+};
+
+template <class T>
+struct J2 {
+ J2(const MovableTemplateT<T>& A);
+ // CHECK-FIXES: J2(const MovableTemplateT<T>& A);
+ MovableTemplateT<T> M;
+};
+
+template <class T>
+J2<T>::J2(const MovableTemplateT<T>& A) : M(A) {}
+// CHECK-FIXES: J2<T>::J2(const MovableTemplateT<T>& A) : M(A) {}
+J2<int> j3(MovableTemplateT<int>{});
+
struct K_Movable {
K_Movable() = default;
K_Movable(const K_Movable &) = default;
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -46,8 +46,9 @@
}
} // namespace
-static TypeMatcher constRefType() {
- return lValueReferenceType(pointee(isConstQualified()));
+static TypeMatcher notTemplateSpecConstRefType() {
+ return lValueReferenceType(
+ pointee(unless(templateSpecializationType()), isConstQualified()));
}
static TypeMatcher nonConstValueType() {
@@ -145,16 +146,18 @@
// ParenListExpr is generated instead of a CXXConstructExpr,
// filtering out templates automatically for us.
withInitializer(cxxConstructExpr(
- has(ignoringParenImpCasts(declRefExpr(to(
- parmVarDecl(
- hasType(qualType(
- // Match only const-ref or a non-const value
- // parameters. Rvalues and const-values
- // shouldn't be modified.
- ValuesOnly ? nonConstValueType()
- : anyOf(constRefType(),
- nonConstValueType()))))
- .bind("Param"))))),
+ has(ignoringParenImpCasts(declRefExpr(
+ to(parmVarDecl(
+ hasType(qualType(
+ // Match only const-ref or a non-const
+ // value parameters. Rvalues,
+ // TemplateSpecializationValues and
+ // const-values shouldn't be modified.
+ ValuesOnly
+ ? nonConstValueType()
+ : anyOf(notTemplateSpecConstRefType(),
+ nonConstValueType()))))
+ .bind("Param"))))),
hasDeclaration(cxxConstructorDecl(
isCopyConstructor(), unless(isDeleted()),
hasDeclContext(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
