[Lldb-commits] [lldb] [clang-tidy] Add performance-move-smart-pointer-contents check. (PR #66139)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 12 16:33:32 PDT 2023
pizzud wrote:
> I don't see problem described by this check as an performance issue. For example:
>
> ```
> std::unique_ptr<std::vector<int>> ptr;
> std::vector<int> local = std::move(*ptr);
> ```
>
> No performance issue here, simply value may need to be moved, do not expect that someone have to always keep object in unique_ptr, at some point it may need to be moved/copied. And some types may be cheap to move.
>
> As for std::shared_ptr, yes this could be a problem.
>
> My suggestion is:
>
> * Part related to moving object from a std::shared_ptr should be a bugprone-moved-from-shared check (or some other name).
> * As for current check, I fear that it may cause lot of false-positives, and basicly it should work only on "heavy" types, but it could be hard to detect such heavy types. But still I don't think why that part should be reduced to just shared ptr, check that would detect std::move calls that are still heavy, simply because moved class is above XYZ bytes in size.
>
@martinboehme internally suggested putting this in the performance category; perhaps he has a more detailed rationale or some relevant thoughts?
> Fix provided by this check `*std::move(p)` is also questionable... Move will still be called, and this check will still detect such issue, in that case fix does not fix anything.
This check won't re-flag it (see the `correctUnique` and `correctShared` test cases)...but by applying this fix, bugprone-use-after-move will recognize the more standard idiom and can catch the issues it looks at. A pattern like
```
void f() {
std::unique_ptr<LargeProtocolBuffer> p = ...;
LargeProtocolBuffer m = std::move(*p);
std::cout << p->some_method() << std::endl;
}
```
slipped through bugprone-use-after-move because it assumes the `*std::move()` pattern. Discussion internally led to this check being proposed.
https://github.com/llvm/llvm-project/pull/66139
More information about the lldb-commits
mailing list