PiotrZSL added a comment.
I run this check on project that I work for, here my comments:
False-positives (for me)
std::optional<Type>&& obj + std::move(opj.value());
std::pair<Type2, Type1>&& obj+ std::forward<std::pair<Type2, Type1>>(obj);
Type1&& factory + factory.create() [on initialization list] (create is not &&
method)
Type&& + member(std:move(obj.member)) [on initialization list]
Type1&& + member(obj) + when Type1 its POD
Type&& + x.emplace_back(std::move(obj.x)); y.emplace_back(std::move(obj.y));
- as function
Type&& (unnamed) + throw UnimplementedException in virtual function
X& operator=(Y&&) - when we create locally X object and assign it via
operator=(X&&), Y used only to access other data
X&& + a = std::move(obj.x), b = std::move(obj.y) (any access to obj member is
wraped via std::move, but not all members are moved)
^ same as above but some members are just used, for example instead of
move std::optional<int> we just copy value (if initialized)
^ same on initialization list
XYZ&& as lambda argument (explicit) + std::forward<XYZ>(obj) in lambda with
"using XYZ = std::vector<..>".
X&& obj as argument + for(auto& v : obj) { something(std::move(v)); }
X&& obj as argument + for(auto& v : obj) { something(std::move(v.x)); }
X&& obj (as function argument)+ lambda called in function [&](auto& v) { v.x
= std::move(obj.x); }
std::optional<std::vector<std::uint8_t>>&& obj + xyz.push_back({other.a,
other.b, std::move(obj), other.c}); - vector type has no constructor, fields
initialization
container&& from + for(auto&& value : from) { if (x) { a = std::move(value);
} else {dest.emplace_back(std::move(value)); } }
const Type&& obj + std::move(obj) in function
FlatMap&& obj + if (const auto it = obj.find(xyz)) { something =
std::move(it->second); } + obj.clear() at end
template<X> SomeType serialize(const X&, OtherType&&, bool isSomething); (in
header, private template function declaration with definition in cpp that also
got warning)
template <typename U> Type& operator=(std::shared_ptr<U>&& u) {
assign(std::forward<U>(u)); return *this; }
template<typename U> void XYZ::swap(XYZ&& other) { something.reset();
other.something.reset(); T::swap(other); (T is template parameter, for example
std::map);
Map&& xyz, auto&& found = xyz.find(value); if (found != xyz.cend()) {
something.merge(found->second); }
Map&& xyz, for(auto& [x,y] : xyz) { something.merge(y); }:
constructor: explicit Something(T&& value) : value(std::forward<T>(value))
{}, where T is class template argument
Overall 167 issues, half false-positive, but I already use other check for
adding std::move, still had some good finding.
For some of the issues message is wrong, for example it could suggest moving
some members (that were not moved) or use std::move instead of std::forward.
But for both cases probably other (new) checks could be better.
In summary:
- does not understand std::forward
- does not understand partial std::move
- does not understand ::swap and ::merge
- does not understand unused (unnamed) arguments
- some of use cases are && on purpose, just to hint user that passed value
should be temporary, with const & it would accept also lvalue, such variables
usually are created in place via constructor call or just initialization with
{x, y, z}
I would say, do check more restricted, better is find less issues, than raising
warnings on correct use-cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141569/new/
https://reviews.llvm.org/D141569
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
