Summary
Several hot paths copy an entire std::vector / std::set where a move would do. None are correctness bugs, but they defeat the library's "make it fast" goal and are easy wins. The codebase is also internally inconsistent — some sites already move correctly.
1. Copy-instead-of-move when returning wrapper objects
Many non-mutating methods build a local std::vector / std::set and then pass it as an lvalue into a wrapper constructor that has both a copy (const T&) and a move (T&&) overload. Passing the lvalue selects the copy constructor, so the whole container is copied even though the local is about to be destroyed.
include/vector.h
filtered — return vector(filtered_vector); (725)
sorted — return vector(sorted_vector); (1020)
removing_at — return vector(copy); (1213)
removing_range — return vector(shorter_vector); (1311)
inserting_back(T) — return vector(augmented_vector); (1468)
replacing_range_at_imp — return vector(replaced_vector); (2045)
- (and the other
inserting_* / removing_* copies)
include/set.h
difference_with — return set(diff); (533)
union_with — return set(combined); (560)
intersect_with — return set(intersection); (587)
filtered — return set(copy); (841)
removing / inserting — return set(copy); (969 / 999)
zip_impl — return set<...>(combined_set); (1199)
Fix: std::move the local into the constructor, e.g. return vector(std::move(filtered_vector));.
Note — the code already does this correctly in places, e.g. inserting_at_impl (vector.h:2008: return vector(std::move(augmented_vector));) and lazy_vector::get (vector.h:2067). So this is just a matter of consistency.
2. keys() uses a pessimizing move
Location: include/set.h:939
return std::move(vec); // defeats NRVO, triggers -Wpessimizing-move
The map equivalents (include/map.h:498, 517) correctly return vec;. Drop the std::move so NRVO applies.
3. insert_back(T value) double-copies the element
Location: include/vector.h:1437-1441 (and inserting_back(T value), 1464-1469)
vector& insert_back(T value)
{
m_vector.push_back(value); // value is an lvalue here -> copies again
return *this;
}
The parameter is taken by value (one copy/move) and then push_backed as an lvalue (a second copy). Use m_vector.push_back(std::move(value));.
Summary
Several hot paths copy an entire
std::vector/std::setwhere a move would do. None are correctness bugs, but they defeat the library's "make it fast" goal and are easy wins. The codebase is also internally inconsistent — some sites already move correctly.1. Copy-instead-of-move when returning wrapper objects
Many non-mutating methods build a local
std::vector/std::setand then pass it as an lvalue into a wrapper constructor that has both a copy (const T&) and a move (T&&) overload. Passing the lvalue selects the copy constructor, so the whole container is copied even though the local is about to be destroyed.include/vector.hfiltered—return vector(filtered_vector);(725)sorted—return vector(sorted_vector);(1020)removing_at—return vector(copy);(1213)removing_range—return vector(shorter_vector);(1311)inserting_back(T)—return vector(augmented_vector);(1468)replacing_range_at_imp—return vector(replaced_vector);(2045)inserting_*/removing_*copies)include/set.hdifference_with—return set(diff);(533)union_with—return set(combined);(560)intersect_with—return set(intersection);(587)filtered—return set(copy);(841)removing/inserting—return set(copy);(969 / 999)zip_impl—return set<...>(combined_set);(1199)Fix:
std::movethe local into the constructor, e.g.return vector(std::move(filtered_vector));.Note — the code already does this correctly in places, e.g.
inserting_at_impl(vector.h:2008:return vector(std::move(augmented_vector));) andlazy_vector::get(vector.h:2067). So this is just a matter of consistency.2.
keys()uses a pessimizing moveLocation:
include/set.h:939The
mapequivalents (include/map.h:498,517) correctlyreturn vec;. Drop thestd::moveso NRVO applies.3.
insert_back(T value)double-copies the elementLocation:
include/vector.h:1437-1441(andinserting_back(T value), 1464-1469)The parameter is taken by value (one copy/move) and then
push_backed as an lvalue (a second copy). Usem_vector.push_back(std::move(value));.