[clang-tidy] Fix false in unnecessary-value-param inside templates (#98488)
Summary:
If callExpr is type dependent, there is no way to analyze individual
arguments until template specialization. Before this diff only calls
with dependent callees were skipped so unnecessary-value-param was
processing arguments that had non-dependent type that gave false
positives because the call was not fully resolved till specialization.
So now instead of checking type dependent callee, the whole expression
will be checked for type dependent.
Test Plan: check-clang-tools
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 697b514..a23483e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -450,7 +450,8 @@
<clang-tidy/checks/performance/unnecessary-value-param>` check
detecting more cases for template functions including lambdas with ``auto``.
E.g., ``std::sort(a.begin(), a.end(), [](auto x, auto y) { return a > b; });``
- will be detected for expensive to copy types.
+ will be detected for expensive to copy types. Fixed false positives for
+ dependent call expressions.
- Improved :doc:`readability-avoid-return-with-void-value
<clang-tidy/checks/readability/avoid-return-with-void-value>` check by adding
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
index 0dffaef..7c7ae43 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
@@ -2,6 +2,31 @@
// CHECK-FIXES: #include <utility>
+namespace std {
+template <typename>
+struct remove_reference;
+
+template <typename _Tp>
+struct remove_reference {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &> {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &&> {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
+ return static_cast<typename std::remove_reference<_Tp>::type &&>(__t);
+}
+} // namespace std
+
struct ExpensiveToCopyType {
const ExpensiveToCopyType & constReference() const {
return *this;
@@ -357,3 +382,12 @@
ExpensiveToCopyType E;
NegativeUsingConstructor S(E);
}
+
+struct B {
+ static void bar(ExpensiveMovableType a, ExpensiveMovableType b);
+};
+
+template <typename T>
+void NegativeCallWithDependentAndNondependentArgs(ExpensiveMovableType a, T b) {
+ B::bar(std::move(a), b);
+}
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 3b3782fa..6d726ae 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -404,25 +404,24 @@
memberExpr(hasObjectExpression(canResolveToExpr(Exp)))),
nonConstReferenceType());
const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
- const auto TypeDependentCallee =
- callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
- cxxDependentScopeMemberExpr(),
- hasType(templateTypeParmType()), isTypeDependent())));
- const auto AsNonConstRefArg = anyOf(
- callExpr(NonConstRefParam, NotInstantiated),
- cxxConstructExpr(NonConstRefParam, NotInstantiated),
- callExpr(TypeDependentCallee, hasAnyArgument(canResolveToExpr(Exp))),
- cxxUnresolvedConstructExpr(hasAnyArgument(canResolveToExpr(Exp))),
- // Previous False Positive in the following Code:
- // `template <typename T> void f() { int i = 42; new Type<T>(i); }`
- // Where the constructor of `Type` takes its argument as reference.
- // The AST does not resolve in a `cxxConstructExpr` because it is
- // type-dependent.
- parenListExpr(hasDescendant(expr(canResolveToExpr(Exp)))),
- // If the initializer is for a reference type, there is no cast for
- // the variable. Values are cast to RValue first.
- initListExpr(hasAnyInit(expr(canResolveToExpr(Exp)))));
+ const auto AsNonConstRefArg =
+ anyOf(callExpr(NonConstRefParam, NotInstantiated),
+ cxxConstructExpr(NonConstRefParam, NotInstantiated),
+ // If the call is type-dependent, we can't properly process any
+ // argument because required type conversions and implicit casts
+ // will be inserted only after specialization.
+ callExpr(isTypeDependent(), hasAnyArgument(canResolveToExpr(Exp))),
+ cxxUnresolvedConstructExpr(hasAnyArgument(canResolveToExpr(Exp))),
+ // Previous False Positive in the following Code:
+ // `template <typename T> void f() { int i = 42; new Type<T>(i); }`
+ // Where the constructor of `Type` takes its argument as reference.
+ // The AST does not resolve in a `cxxConstructExpr` because it is
+ // type-dependent.
+ parenListExpr(hasDescendant(expr(canResolveToExpr(Exp)))),
+ // If the initializer is for a reference type, there is no cast for
+ // the variable. Values are cast to RValue first.
+ initListExpr(hasAnyInit(expr(canResolveToExpr(Exp)))));
// Captured by a lambda by reference.
// If we're initializing a capture with 'Exp' directly then we're initializing
