emscripten-core / emscripten-core/emscripten

Avoid copying rvalue object passed to Embind

Open
#25,412 1 comment 1 reaction 1 assignee Claimed by @brendandahl View on GitHub
Dominant language
C++
Stars
27.6k
Forks
3.6k
Avg merge
1d 1h
Merged PRs (30d)
105

Description

Say I have the following code:

```cpp
#include
#include
#include
#include
#include

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(largeData.data())));
}
};

EMSCRIPTEN_BINDINGS(NoCopy) {
emscripten::class_("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::toWireType' requested here
240 | writeGenericWireType(cursor, BindingType::toWireType(std::forward(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' requested here
248 | writeGenericWireTypes(cursor, std::forward(args)...);
| ^
/home/swdv/emsdk/upstream/emscripten/cache/sysroot/include/emscripten/val.h:588:27: note: in instantiation of member function 'emscripten::internal::WireTypePack::WireTypePack' requested here
588 | WireTypePack argv(std::forward(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::val, NoCopy>' requested here
348 | new (this) val(internalCall, val>(nullptr, nullptr, std::forward(value)));
| ^
/home/swdv/webtest/main.cpp:26:9: note: in instantiation of function template specialization 'emscripten::val::val' 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::toWireType`, even though it is passed as an rvalue. I'd expect it to be moved instead.

GenericBindingType declaration

https://github.com/emscripten-core/emscripten/blob/96371ed7888fc78c040179f4d4faa82a6a07a116/system/include/emscripten/wire.h#L381-L404

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 (96371ed7888fc78c040179f4d4faa82a6a07a116)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.