Skip to content

Commit

Permalink
[clang-tidy] Fix false in unnecessary-value-param inside templates
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dmpolukhin committed Jul 11, 2024
1 parent ce92b2f commit a05c4ca
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -402,3 +427,12 @@ int templateSpecializationFunction(ExpensiveToCopyType E) {
// CHECK-FIXES-NOT: int templateSpecializationFunction(const ExpensiveToCopyType& E) {
return 0;
}

struct B {
static void bar(ExpensiveMovableType a, ExpensiveMovableType b);
};

template <typename T>
void NegativeCallWithDependentAndNondependentArgs(ExpensiveMovableType a, T b) {
B::bar(std::move(a), b);
}
9 changes: 4 additions & 5 deletions clang/lib/Analysis/ExprMutationAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,15 +404,14 @@ ExprMutationAnalyzer::Analyzer::findDirectMutation(const Expr *Exp) {
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))),
// 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); }`
Expand Down

0 comments on commit a05c4ca

Please sign in to comment.