[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added a comment.

In D137205#3915526 <https://reviews.llvm.org/D137205#3915526>, @Skylion007 wrote:

> Okay, now I am getting what I believe to be segfaults:
>
>   #0 0x0000564383482be4 PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
>   #1 0x0000564383480464 SignalHandler(int) Signals.cpp:0:0
>   #2 0x00007f7c275c9420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
>   #3 0x00005643804b0ea5 clang::tidy::performance::UnnecessaryCopyOnLastUseCheck::check(clang::ast_matchers::MatchFinder::MatchResult const&) (.cold) UnnecessaryCopyOnLastUseCheck.cpp:0:0
>   #4 0x000056438262bba1 clang::ast_matchers::internal::(anonymous namespace)::MatchASTVisitor::MatchVisitor::visitMatch(clang::ast_matchers::BoundNodes const&) ASTMatchFinder.cpp:0:0
>   #5 0x00005643826818df clang::ast_matchers::internal::BoundNodesTreeBuilder::visitMatches(clang::ast_matchers::internal::BoundNodesTreeBuilder::Visitor*) (/home/aaron/git/llvm-project/build/bin/clang-tidy+0x2d258df)
>
> It worked before this PR, now it just crashes on a lot of real world codebases including just trying to run it for a few files on LLVM's own codebase.

I thought I had fixed it with my newest diff. 
The reason for the segfaults are, that I removed the `llvm::Expected` and replaced it with asserts. But there are cases, where the asserts can be false with regular (currently unhandled code).

I also found issues:

- a fixup is proposed even when the copy-parameter is not mentioned in the code. This is the case for capturing lambdas `... = [=]{...};` -> `... = [std::move(=)]{...};`
- a fixup is proposed for trivial types like std::pair<unsigned, unsigned>


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137205/new/

https://reviews.llvm.org/D137205