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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
