Hacker News new | ask | show | jobs
by senkora 502 days ago
This bothered me enough to check the source code, because I simply had to know:

    template <layout_or_array A>
    std::pair<ArrowArray*, ArrowSchema*> get_arrow_structures(A& a)
    {
        arrow_proxy& proxy = detail::array_access::get_arrow_proxy(a);
        return std::make_pair(&(proxy.array()), &(proxy.schema()));
    }
https://github.com/man-group/sparrow/blob/c01a768f590ebf3b22...

So the answer is that the `std::move` does nothing and should be omitted, because this function only has one overload, and that overload takes its argument by lvalue-reference.

(And as far as I can tell, `detail::array_access::get_arrow_proxy(a)` eventually just reads a member on `a`, so there's no copying anywhere)

It's a harmless mistake; I'm just surprised it wasn't caught. The authors seem pretty experienced with the language.

2 comments

> So the answer is that the `std::move` does nothing and should be omitted

You can't assign an rvalue into an lvalue reference, precisely to avoid this sort of mistake. If this is the only overload then this just wouldn't compile (e.g. [1]). So the std::move isn't doing nothing, but yes it should be omitted. Maybe it's just a weird typo.

[1] https://ideone.com/4NS5dI

Indeed, I don't even have to compile it, vim (+ Syntastic) tells me as soon as it sees it:

  candidate function not viable: expects an lvalue for 1st argument
Modern C++ in a nutshell

t. C++17 dev and pain connoisseur