autocxx
autocxx copied to clipboard
google
Avoid the `std::move` for each value parameter
It would be good to impl ValueParam<T> for any New<Output=T>.
In addition, somewhere deep in the generated bindings, we still currently call std::move on the C++ side. This is probably not desirable.
One bad thing that the std::move does: C++ methods which take a non-const reference don't work. I get compile errors like this:
bazel-out/k8-fastbuild/bin/aos/events/event_loop_rs__gen__dir/gen0.h:64:160: error:
non-const lvalue reference to type 'flatbuffers::Verifier' cannot bind to a temporary of type
'typename std::remove_reference<Verifier &>::type' (aka 'flatbuffers::Verifier')
...arg1) { return std::move(autocxx_gen_this).Verify(std::move(arg1)); }
The method signature being wrapped is bool Verify(flatbuffers::Verifier &verifier) const;.
Half of this is now done in #847; the remaining half is to remove the std::move.
@bsilver8192 Could you perhaps add a test case for the problem you report above? I tried in https://github.com/google/autocxx/pull/900 but it all seems to work, so I must be doing something slightly differently.
Ok, added a PR. The tricky thing is it works if it's the only method with a given name, because then it doesn't get an autocxx-generated C++ wrapper, and bindgen/cxxbridge manages calling it fine.
Not sure you want to just fix it immediately, or disable those tests, or add duplicated tests that are disabled, up to you.