-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Description
Say I have the following code:
#include <cstdint>
#include <string>
#include <utility>
#include <emscripten/bind.h>
#include <emscripten/val.h>
struct NoCopy {
std::string largeData;
NoCopy(std::string largeData) : largeData(std::move(largeData)) {}
NoCopy(NoCopy &&) = default;
NoCopy &operator=(NoCopy &&) = default;
emscripten::val largeDataView() const {
return emscripten::val(emscripten::typed_memory_view(largeData.size(), reinterpret_cast<const std::uint8_t *>(largeData.data())));
}
};
EMSCRIPTEN_BINDINGS(NoCopy) {
emscripten::class_<NoCopy>("NoCopy")
.function("largeDataView", &NoCopy::largeDataView);
}
int main() {
(void) emscripten::val(NoCopy("AAAAAAA" /*imagine this is a few megabytes*/));
}I do not want the data in NoCopy to be copied, so I deleted the copy constructor by only declaring move operations, and provided a function that JS can call to obtain a view of the memory.
I then construct a val which I may pass to another function or val::set, for example.
However, this produces an error:
Compiler error
/home/swdv/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/wire.h:388:20: error: call to implicitly-deleted copy constructor of 'ActualT' (aka 'NoCopy')
388 | return new ActualT(v);
| ^ ~
/home/swdv/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/val.h:240:52: note: in instantiation of function template specialization 'emscripten::internal::GenericBindingType<NoCopy>::toWireType<NoCopy>' requested here
240 | writeGenericWireType(cursor, BindingType<First>::toWireType(std::forward<First>(first), rvp::default_tag{}));
| ^
/home/swdv/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/val.h:248:5: note: in instantiation of function template specialization 'emscripten::internal::writeGenericWireTypes<NoCopy>' requested here
248 | writeGenericWireTypes(cursor, std::forward<Args>(args)...);
| ^
/home/swdv/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/val.h:588:27: note: in instantiation of member function 'emscripten::internal::WireTypePack<NoCopy>::WireTypePack' requested here
588 | WireTypePack<Args...> argv(std::forward<Args>(args)...);
| ^
/home/swdv/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/val.h:348:20: note: in instantiation of function template specialization 'emscripten::val::internalCall<emscripten::internal::EM_INVOKER_KIND::CAST, emscripten::internal::WithPolicies<>, emscripten::val, NoCopy>' requested here
348 | new (this) val(internalCall<EM_INVOKER_KIND::CAST, WithPolicies<Policies...>, val>(nullptr, nullptr, std::forward<T>(value)));
| ^
/home/swdv/webtest/main.cpp:26:9: note: in instantiation of function template specialization 'emscripten::val::val<NoCopy>' requested here
26 | (void) emscripten::val(NoCopy("AAAAAAA"));
| ^
/home/swdv/webtest/main.cpp:12:2: note: copy constructor is implicitly deleted because 'NoCopy' has a user-declared move constructor
12 | NoCopy(NoCopy &&) = default;
| ^
As it turns out, NoCopy is copied in GenericBindingType<NoCopy>::toWireType, even though it is passed as an rvalue. I'd expect it to be moved instead.
GenericBindingType declaration
emscripten/system/include/emscripten/wire.h
Lines 381 to 404 in 96371ed
| template<typename T> | |
| struct GenericBindingType { | |
| typedef typename std::remove_reference<T>::type ActualT; | |
| typedef ActualT* WireType; | |
| template<typename R> | |
| static WireType toWireType(R&& v, rvp::default_tag) { | |
| return new ActualT(v); | |
| } | |
| template<typename R> | |
| static WireType toWireType(R&& v, rvp::take_ownership) { | |
| return new ActualT(std::move(v)); | |
| } | |
| template<typename R> | |
| static WireType toWireType(R&& v, rvp::reference) { | |
| return &v; | |
| } | |
| static ActualT& fromWireType(WireType p) { | |
| return *p; | |
| } | |
| }; |
Note how, even though toWireType(R&& v, rvp::default_tag) takes a universal reference, v is not forwarded.
For performance reasons, I'd really like an rvalue object argument of class type to be moved.
Related to this issue is that some functions, like val::set and val::array, take their arguments by const lvalue reference, meaning moving is not possible even if the above issue is solved (unless you wrap it in a val before passing it).
Affected version
4.0.14 (96371ed)