[PATCH] D87540: [clang-tidy] Fix false positive issue in performance-unnecessary-value-param for arguments being moved in the function body.

[Sukraat Ahluwalia via Phabricator via cfe-commits]

sukraat91 created this revision.
sukraat91 added a reviewer: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
sukraat91 requested review of this revision.

This patch solves a bug (https://bugs.llvm.org/show_bug.cgi?id=44598) that was filed for a false positive in performance-unnecessary-value-param. This was in the case of template functions, where the argument was an expensive to copy type argument and the check would convert the argument to a const T&, even if it was being moved in the function body.

For example, this sort of code -

#include <string>
template<class T>
static T* Create(std::string s) {

  return new T(std::move(s));

}

Leads to the check converting the argument to a const std::string&, even when it is being moved in the body. We saw the same behavior in my company codebase, where there were many false positives being reported.

We ran the modified check based on this submission, on tens of thousands of files to see those false positives not being reported any more. The modifications to the checker are -

1. For an expensive to copy type, before checking it is const qualified, check to see if it is being moved in the function body.

2. The argument and the AST context are passed on to a helper function. The function uses a matcher to check whether the argument is part of a std::move() call in the function body.

3. If true then ignore.

I am submitting the patch for review, along with new regression tests to verify that the check is ok if it sees an expensive to copy argument being moved, and does not recommend to change it to const T&. I have most recently applied the patch to LLVM master as of 09/10/2020, with no issues in build and all tests passing (using make -j10 check-clang-tools).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87540

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87540.291309.patch
Type: text/x-patch
Size: 4142 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200911/00a31812/attachment-0001.bin>