search

pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
886 stars 118 forks source link

Add move constructor for v8pp::context #168

Closed YarikTH closed 2 years ago

pmed commented 2 years ago

Thanks a lot! I would also try to add the move assignment operator. Maybe it would be handy to have the movement available other v8pp classes, like module, class_.

YarikTH commented 2 years ago

Only thing that bothers me in this PR - test is ugly. It's just copy of test from above, but with a move. Maybe static_assert(std::is_move_constructable::value); would fit better.

YarikTH commented 2 years ago

Another inconvenient thing - move constructor has to be noexcept(false) because member value v8::Global<> is noexcept(false) at least in v8 6.X. Maybe move constructor should be marked as

constexpr bool is_context_move_noexcept = noexcept(std::declval<v8::Global<v8::Context>>().Pass());

context(context&&) noexcept(is_context_move_noexcept);

looks ugly, but at least it will become noexcept as soon as v8::Global's move will become noexcept.

pmed commented 2 years ago

Merged in 9ee366003dc01858be9e0601b3949d112aa2432e, backported to c++17 branch in 21918e08426ec7d1ed54b678caa25a07737b15dd