From b3a2855b3300b177e21a192f2fa0e604e8065d2f Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Wed, 24 Jan 2024 13:14:46 -0800 Subject: [PATCH 1/9] fix syntax errors --- src/embind/emval.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/embind/emval.js b/src/embind/emval.js index 8d22ae24839ad..c94d66fc3311c 100644 --- a/src/embind/emval.js +++ b/src/embind/emval.js @@ -103,7 +103,11 @@ var LibraryEmVal = { _emval_decref__deps: ['$emval_freelist', '$emval_handles'], _emval_decref: (handle) => { +<<<<<<< HEAD if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}} && 0 === --emval_handles[handle + 1]) { +======= + if (handle >= emval_handles_reserved && 0 === --emval_handles[handle * 2 + 1]) { +>>>>>>> d90ee0942 (fix syntax errors) #if ASSERTIONS assert(emval_handles[handle] !== undefined, `Decref for unallocated handle.`); #endif From f3583eba5667b86243442337f5a5bca2c41f01a6 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Wed, 7 Feb 2024 11:33:30 -0800 Subject: [PATCH 2/9] Simplify handle code paths even if it means halving the address space. --- system/include/emscripten/val.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 9721f66f7999e..822f52fd28319 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -392,7 +392,7 @@ class val { } ~val() { - if (uses_ref_count()) { + if (handle) { internal::_emval_decref(as_handle()); handle = 0; } From 02b4f8b9530f7bab7e287d8d0f95c9838240888f Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Fri, 16 Feb 2024 14:36:17 -0800 Subject: [PATCH 3/9] Skip incref/decref for reserved handles. --- system/include/emscripten/val.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 822f52fd28319..b6449c4e64900 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -392,7 +392,7 @@ class val { } ~val() { - if (handle) { + if (uses_refcount()) { internal::_emval_decref(as_handle()); handle = 0; } From 6317db6d54d2203be76cb0b7c38d173cc3778c33 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Wed, 21 Feb 2024 08:55:55 -0800 Subject: [PATCH 4/9] First draft of unique_val --- system/include/emscripten/val.h | 431 ++++++++++++++++++++------------ 1 file changed, 276 insertions(+), 155 deletions(-) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index b6449c4e64900..5dfb6c1f0de25 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -284,7 +284,9 @@ struct WireTypePack { static const char name##_symbol[] = #name; \ static const ::emscripten::internal::symbol_registrar name##_registrar -class val { +class unique_val; +class val; +class base_val { public: // missing operators: // * ~ - + ++ -- @@ -296,102 +298,31 @@ class val { // exposing void, comma, and conditional is unnecessary // same with: = += -= *= /= %= <<= >>= >>>= &= ^= |= - static val array() { - return val(internal::_emval_new_array()); - } + static unique_val array(); template - static val array(Iter begin, Iter end) { -#if __cplusplus >= 202002L - if constexpr (std::contiguous_iterator && - internal::typeSupportsMemoryView< - typename std::iterator_traits::value_type>()) { - val view{ typed_memory_view(std::distance(begin, end), std::to_address(begin)) }; - return val(internal::_emval_new_array_from_memory_view(view.as_handle())); - } - // For numeric arrays, following codes are unreachable and the compiler - // will do 'dead code elimination'. - // Others fallback old way. -#endif - val new_array = array(); - for (auto it = begin; it != end; ++it) { - new_array.call("push", *it); - } - return new_array; - } + static unique_val array(Iter begin, Iter end); template - static val array(const std::vector& vec) { - if constexpr (internal::typeSupportsMemoryView()) { - // for numeric types, pass memory view and copy in JS side one-off - val view{ typed_memory_view(vec.size(), vec.data()) }; - return val(internal::_emval_new_array_from_memory_view(view.as_handle())); - } else { - return array(vec.begin(), vec.end()); - } - } + static unique_val array(const std::vector& vec); - static val object() { - return val(internal::_emval_new_object()); - } + static unique_val object(); - static val u8string(const char* s) { - return val(internal::_emval_new_u8string(s)); - } + static unique_val u8string(const char* s); - static val u16string(const char16_t* s) { - return val(internal::_emval_new_u16string(s)); - } + static unique_val u16string(const char16_t* s); - static val undefined() { - return val(EM_VAL(internal::_EMVAL_UNDEFINED)); - } + static unique_val undefined(); - static val null() { - return val(EM_VAL(internal::_EMVAL_NULL)); - } - - static val take_ownership(EM_VAL e) { - return val(e); - } - - static val global(const char* name = 0) { - return val(internal::_emval_get_global(name)); - } - - static val module_property(const char* name) { - return val(internal::_emval_get_module_property(name)); - } + static unique_val null(); - template - explicit val(T&& value) { - using namespace internal; + static unique_val take_ownership(EM_VAL e); - WireTypePack argv(std::forward(value)); - new (this) val(_emval_take_value(internal::TypeID::get(), argv)); - } + static unique_val global(const char* name = 0); - val() : val(EM_VAL(internal::_EMVAL_UNDEFINED)) {} + static unique_val module_property(const char* name); - explicit val(const char* v) - : val(internal::_emval_new_cstring(v)) - {} - - // Note: unlike other constructors, this doesn't use as_handle() because - // it just moves a value and doesn't need to go via incref/decref. - // This means it's safe to move values across threads - an error will - // only arise if you access or free it from the wrong thread later. - val(val&& v) : handle(v.handle), thread(v.thread) { - v.handle = 0; - } - - val(const val& v) : val(v.as_handle()) { - if (uses_ref_count()) { - internal::_emval_incref(handle); - } - } - - ~val() { + ~base_val() { if (uses_refcount()) { internal::_emval_decref(as_handle()); handle = 0; @@ -406,26 +337,13 @@ class val { } // Takes ownership of the handle away from, and invalidates, this instance. - EM_VAL release_ownership() { + EM_VAL take_handle() { EM_VAL taken = as_handle(); handle = 0; return taken; } - val& operator=(val&& v) & { - val tmp(std::move(v)); - this->~val(); - new (this) val(std::move(tmp)); - return *this; - } - - val& operator=(const val& v) & { - return *this = val(v); - } - - bool hasOwnProperty(const char* key) const { - return val::global("Object")["prototype"]["hasOwnProperty"].call("call", *this, val(key)); - } + bool hasOwnProperty(const char* key) const; bool isNull() const { return as_handle() == EM_VAL(internal::_EMVAL_NULL); @@ -451,39 +369,37 @@ class val { return internal::_emval_is_string(as_handle()); } - bool isArray() const { - return instanceof(global("Array")); - } + bool isArray() const; - bool equals(const val& v) const { + bool equals(const base_val& v) const { return internal::_emval_equals(as_handle(), v.as_handle()); } - bool operator==(const val& v) const { + bool operator==(const base_val& v) const { return equals(v); } - bool operator!=(const val& v) const { + bool operator!=(const base_val& v) const { return !equals(v); } - bool strictlyEquals(const val& v) const { + bool strictlyEquals(const base_val& v) const { return internal::_emval_strictly_equals(as_handle(), v.as_handle()); } - bool operator>(const val& v) const { + bool operator>(const base_val& v) const { return internal::_emval_greater_than(as_handle(), v.as_handle()); } - bool operator>=(const val& v) const { + bool operator>=(const base_val& v) const { return (*this > v) || (*this == v); } - bool operator<(const val& v) const { + bool operator<(const base_val& v) const { return internal::_emval_less_than(as_handle(), v.as_handle()); } - bool operator<=(const val& v) const { + bool operator<=(const base_val& v) const { return (*this < v) || (*this == v); } @@ -492,9 +408,7 @@ class val { } template - val operator[](const T& key) const { - return val(internal::_emval_get_property(as_handle(), val_ref(key).as_handle())); - } + unique_val operator[](const T& key) const; template void set(const K& key, const V& value) { @@ -507,18 +421,10 @@ class val { } template - val new_(Args&&... args) const { - using namespace internal; - - return internalCall(_emval_call, std::forward(args)...); - } + unique_val new_(Args&&... args) const; template - val operator()(Args&&... args) const { - using namespace internal; - - return internalCall(_emval_call, std::forward(args)...); - } + unique_val operator()(Args&&... args) const; template ReturnValue call(const char* name, Args&&... args) const { @@ -593,23 +499,19 @@ class val { } // Prefer calling val::typeOf() over val::typeof(), since this form works in both C++11 and GNU++11 build modes. "typeof" is a reserved word in GNU++11 extensions. - val typeOf() const { - return val(internal::_emval_typeof(as_handle())); - } + unique_val typeOf() const; // If code is not being compiled with GNU extensions enabled, typeof() is a valid identifier, so support that as a member function. #if __is_identifier(typeof) [[deprecated("Use typeOf() instead.")]] - val typeof() const { - return typeOf(); - } + unique_val typeof() const; #endif - bool instanceof(const val& v) const { + bool instanceof(const base_val& v) const { return internal::_emval_instanceof(as_handle(), v.as_handle()); } - bool in(const val& v) const { + bool in(const base_val& v) const { return internal::_emval_in(as_handle(), v.as_handle()); } @@ -617,9 +519,7 @@ class val { internal::_emval_throw(as_handle()); } - val await() const { - return val(internal::_emval_await(as_handle())); - } + unique_val await() const; struct iterator; @@ -634,21 +534,30 @@ class val { class promise_type; #endif -private: +protected: // takes ownership, assumes handle already incref'd and lives on the same thread - explicit val(EM_VAL handle) + explicit base_val(EM_VAL handle) : handle(handle), thread(pthread_self()) {} + base_val(EM_VAL handle, pthread_t thread) + : handle(handle), thread(thread) + {} + + // Note: unlike other constructors, this doesn't use as_handle() because + // it just moves a value and doesn't need to go via incref/decref. + // This means it's safe to move values across threads - an error will + // only arise if you access or free it from the wrong thread later. + explicit base_val(base_val&& v) : handle(v.handle), thread(v.thread) { + v.handle = 0; + } + // Whether this value is a uses incref/decref (true) or is a special reserved // value (false). - bool uses_ref_count() const { + bool uses_refcount() const { return handle > reinterpret_cast(internal::_EMVAL_LAST_RESERVED_HANDLE); } - template - friend val internal::wrapped_extend(const std::string& , const val& ); - template Ret internalCall(Implementation impl, Args&&... args) const { using namespace internal; @@ -665,8 +574,14 @@ class val { } template - val val_ref(const T& v) const { - return val(v); + val val_ref(const T& v) const; + + const base_val& val_ref(const base_val& v) const { + return v; + } + + const unique_val& val_ref(const unique_val& v) const { + return v; } const val& val_ref(const val& v) const { @@ -675,15 +590,221 @@ class val { pthread_t thread; EM_VAL handle; +}; + +class unique_val : public base_val { + public: + unique_val() : base_val(EM_VAL(internal::_EMVAL_UNDEFINED)) {} + + // takes ownership, assumes handle already incref'd and lives on the same thread + explicit unique_val(EM_VAL handle) : base_val(handle) {} + + explicit unique_val(const char* v) + : base_val(internal::_emval_new_cstring(v)) + {} + + template + explicit unique_val(T&& value) : unique_val() { + using namespace internal; + + WireTypePack argv(std::forward(value)); + new (this) unique_val(_emval_take_value(internal::TypeID::get(), argv)); + } + + // unique_val doesn't allow copy, as that would require incref/decref. + unique_val(const unique_val& v) = delete; + unique_val& operator=(const unique_val& v) = delete; + + unique_val(base_val&& v) : base_val(std::move(v)) {} + unique_val(unique_val&& v) : base_val(std::move(v)) {} + + unique_val& operator=(base_val&& v) & { + unique_val tmp(std::move(v)); + this->~unique_val(); + new (this) unique_val(std::move(tmp)); + return *this; + } + + unique_val& operator=(unique_val&& v) & { + unique_val tmp(std::move(v)); + this->~unique_val(); + new (this) unique_val(std::move(tmp)); + return *this; + } +}; + +class val : public base_val { + public: + val() : base_val(EM_VAL(internal::_EMVAL_UNDEFINED)) {} + + // takes ownership, assumes handle already incref'd and lives on the same thread + explicit val(EM_VAL handle) : base_val(handle) {} + + explicit val(const char* v) + : base_val(internal::_emval_new_cstring(v)) + {} + + // Note: unlike other constructors, this doesn't use as_handle() because + // it just moves a value and doesn't need to go via incref/decref. + // This means it's safe to move values across threads - an error will + // only arise if you access or free it from the wrong thread later. + val(base_val&& v) : base_val(std::move(v)) { } + + val(const val& v) : base_val(v.as_handle()) { + if (uses_refcount()) { + internal::_emval_incref(handle); + } + } + + val(const base_val& v) : base_val(v.as_handle()) { + if (uses_refcount()) { + internal::_emval_incref(handle); + } + } + + template + explicit val(T&& value) : val() { + using namespace internal; + + WireTypePack argv(std::forward(value)); + new (this) val(_emval_take_value(internal::TypeID::get(), argv)); + } + + val& operator=(base_val&& v) & { + val tmp(std::move(v)); + this->~val(); + new (this) val(std::move(tmp)); + return *this; + } + + val& operator=(const val& v) & { + return *this = val(v); + } + + template + friend val internal::wrapped_extend(const std::string& , const val& ); friend struct internal::BindingType; }; -struct val::iterator { +inline unique_val base_val::array() { + return unique_val(internal::_emval_new_array()); +} + +template +inline unique_val base_val::array(Iter begin, Iter end) { +#if __cplusplus >= 202002L + if constexpr (std::contiguous_iterator && + internal::typeSupportsMemoryView< + typename std::iterator_traits::value_type>()) { + unique_val view{ typed_memory_view(std::distance(begin, end), std::to_address(begin)) }; + return unique_val(internal::_emval_new_array_from_memory_view(view.as_handle())); + } + // For numeric arrays, following codes are unreachable and the compiler + // will do 'dead code elimination'. + // Others fallback old way. +#endif + unique_val new_array = array(); + for (auto it = begin; it != end; ++it) { + new_array.call("push", *it); + } + return std::move(new_array); +} + +template +inline unique_val base_val::array(const std::vector& vec) { + if constexpr (internal::typeSupportsMemoryView()) { + // for numeric types, pass memory view and copy in JS side one-off + unique_val view{ typed_memory_view(vec.size(), vec.data()) }; + return unique_val(internal::_emval_new_array_from_memory_view(view.as_handle())); + } else { + return array(vec.begin(), vec.end()); + } +} + +inline unique_val base_val::object() { + return unique_val(internal::_emval_new_object()); +} + +inline unique_val base_val::u8string(const char* s) { + return unique_val(internal::_emval_new_u8string(s)); +} + +inline unique_val base_val::u16string(const char16_t* s) { + return unique_val(internal::_emval_new_u16string(s)); +} + +inline unique_val base_val::undefined() { + return unique_val(EM_VAL(internal::_EMVAL_UNDEFINED)); +} + +inline unique_val base_val::null() { + return unique_val(EM_VAL(internal::_EMVAL_NULL)); +} + +inline unique_val base_val::take_ownership(EM_VAL e) { + return unique_val(e); +} + +inline unique_val base_val::global(const char* name) { + return unique_val(internal::_emval_get_global(name)); +} + +inline unique_val base_val::module_property(const char* name) { + return unique_val(internal::_emval_get_module_property(name)); +} + +inline bool base_val::hasOwnProperty(const char* key) const { + return base_val::global("Object")["prototype"]["hasOwnProperty"].call("call", val(*this), val(key)); +} + +inline bool base_val::isArray() const { + return instanceof(global("Array")); +} + +template +inline unique_val base_val::operator[](const T& key) const { + return unique_val(internal::_emval_get_property(as_handle(), val_ref(key).as_handle())); +} + +template +inline unique_val base_val::new_(Args&&... args) const { + using namespace internal; + + return internalCall(_emval_call, std::forward(args)...); +} + +template +inline unique_val base_val::operator()(Args&&... args) const { + using namespace internal; + + return internalCall(_emval_call, std::forward(args)...); +} + +inline unique_val base_val::typeOf() const { + return val(internal::_emval_typeof(as_handle())); +} + +#if __is_identifier(typeof) +inline unique_val base_val::typeof() const { + return typeOf(); +} +#endif + +inline unique_val base_val::await() const { + return unique_val(internal::_emval_await(as_handle())); +} + +template +inline val base_val::val_ref(const T& v) const { + return val(v); +} + +struct base_val::iterator { iterator() = delete; // Make sure iterator is only moveable, not copyable as it represents a mutable state. iterator(iterator&&) = default; - iterator(const val& v) : iter(internal::_emval_iter_begin(v.as_handle())) { + iterator(const base_val& v) : iter(internal::_emval_iter_begin(v.as_handle())) { this->operator++(); } val&& operator*() { return std::move(cur_value); } @@ -696,7 +817,7 @@ struct val::iterator { val cur_value; }; -inline val::iterator val::begin() const { +inline base_val::iterator base_val::begin() const { return iterator(*this); } @@ -705,12 +826,12 @@ inline val::iterator val::begin() const { // to drive the argument of the `co_await` operator (regardless // of the type of the parent coroutine). // This one is used for Promises represented by the `val` type. -class val::awaiter { +class base_val::awaiter { // State machine holding awaiter's current state. One of: // - initially created with promise // - waiting with a given coroutine handle // - completed with a result - std::variant, val> state; + std::variant, val> state; constexpr static std::size_t STATE_PROMISE = 0; constexpr static std::size_t STATE_CORO = 1; @@ -746,15 +867,15 @@ class val::awaiter { val await_resume() { return std::move(std::get(state)); } }; -inline val::awaiter val::operator co_await() const { +inline val::awaiter base_val::operator co_await() const { return {*this}; } // `promise_type` is a well-known subtype with well-known method names // that compiler uses to drive the coroutine itself // (`T::promise_type` is used for any coroutine with declared return type `T`). -class val::promise_type { - val promise, resolve, reject_with_current_exception; +class base_val::promise_type { + unique_val promise, resolve, reject_with_current_exception; public: // Create a `new Promise` and store it alongside the `resolve` and `reject` @@ -762,9 +883,9 @@ class val::promise_type { promise_type() { EM_VAL resolve_handle; EM_VAL reject_handle; - promise = val(internal::_emval_coro_make_promise(&resolve_handle, &reject_handle)); - resolve = val(resolve_handle); - reject_with_current_exception = val(reject_handle); + promise = unique_val(internal::_emval_coro_make_promise(&resolve_handle, &reject_handle)); + resolve = unique_val(resolve_handle); + reject_with_current_exception = unique_val(reject_handle); } // Return the stored promise as the actual return value of the coroutine. @@ -806,14 +927,14 @@ struct BindingType::value && // Marshall to JS with move semantics when we can invalidate the temporary val // object. static WireType toWireType(val&& v) { - return v.release_ownership(); + return v.take_handle(); } // Marshal to JS with copy semantics when we cannot transfer the val objects // reference count. static WireType toWireType(const val& v) { EM_VAL handle = v.as_handle(); - if (v.uses_ref_count()) { + if (v.uses_refcount()) { _emval_incref(handle); } return handle; From f6b5e60ba95e066d0572d3fc3102b630dd2dd346 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Fri, 23 Feb 2024 16:08:10 -0800 Subject: [PATCH 5/9] unique_val passes test_embind* --- system/include/emscripten/val.h | 69 +++++++++++++++++++++------------ test/test_core.py | 2 +- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 5dfb6c1f0de25..c6267916e58d8 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -337,7 +337,7 @@ class base_val { } // Takes ownership of the handle away from, and invalidates, this instance. - EM_VAL take_handle() { + EM_VAL release_ownership() { EM_VAL taken = as_handle(); handle = 0; return taken; @@ -552,6 +552,15 @@ class base_val { v.handle = 0; } + void move_assignment(base_val&& v) & { + EM_VAL new_handle = v.handle; + pthread_t new_thread = v.thread; + v.handle = 0; + this->release_ownership(); + handle = new_handle; + thread = new_thread; + } + // Whether this value is a uses incref/decref (true) or is a special reserved // value (false). bool uses_refcount() const { @@ -604,33 +613,21 @@ class unique_val : public base_val { {} template - explicit unique_val(T&& value) : unique_val() { - using namespace internal; - - WireTypePack argv(std::forward(value)); - new (this) unique_val(_emval_take_value(internal::TypeID::get(), argv)); - } + explicit unique_val(T&& value); // unique_val doesn't allow copy, as that would require incref/decref. unique_val(const unique_val& v) = delete; unique_val& operator=(const unique_val& v) = delete; - unique_val(base_val&& v) : base_val(std::move(v)) {} + unique_val(val&& v); unique_val(unique_val&& v) : base_val(std::move(v)) {} - unique_val& operator=(base_val&& v) & { - unique_val tmp(std::move(v)); - this->~unique_val(); - new (this) unique_val(std::move(tmp)); - return *this; - } - unique_val& operator=(unique_val&& v) & { - unique_val tmp(std::move(v)); - this->~unique_val(); - new (this) unique_val(std::move(tmp)); + move_assignment(std::move(v)); return *this; } + + unique_val& operator=(val&& v) &; }; class val : public base_val { @@ -648,7 +645,8 @@ class val : public base_val { // it just moves a value and doesn't need to go via incref/decref. // This means it's safe to move values across threads - an error will // only arise if you access or free it from the wrong thread later. - val(base_val&& v) : base_val(std::move(v)) { } + val(unique_val&& v) : base_val(std::move(v)) { } + val(val&& v) : base_val(std::move(v)) { } val(const val& v) : base_val(v.as_handle()) { if (uses_refcount()) { @@ -671,14 +669,20 @@ class val : public base_val { } val& operator=(base_val&& v) & { - val tmp(std::move(v)); - this->~val(); - new (this) val(std::move(tmp)); + move_assignment(std::move(v)); return *this; } val& operator=(const val& v) & { - return *this = val(v); + if (uses_refcount()) { + internal::_emval_decref(handle); + } + handle = v.handle; + thread = v.thread; + if (uses_refcount()) { + internal::_emval_incref(handle); + } + return *this; } template @@ -687,6 +691,23 @@ class val : public base_val { friend struct internal::BindingType; }; +inline unique_val::unique_val(val&& v) : base_val(0) { + move_assignment(std::move(v)); +} + +template +inline unique_val::unique_val(T&& value) : unique_val() { + using namespace internal; + + WireTypePack argv(std::forward(value)); + new (this) unique_val(std::move(_emval_take_value(internal::TypeID::get(), argv))); +} + +inline unique_val& unique_val::operator=(val&& v) & { + move_assignment(std::move(v)); + return *this; +} + inline unique_val base_val::array() { return unique_val(internal::_emval_new_array()); } @@ -927,7 +948,7 @@ struct BindingType::value && // Marshall to JS with move semantics when we can invalidate the temporary val // object. static WireType toWireType(val&& v) { - return v.take_handle(); + return v.release_ownership(); } // Marshal to JS with copy semantics when we cannot transfer the val objects diff --git a/test/test_core.py b/test/test_core.py index 8711e968dcadc..04eeaebfc3e98 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -7457,7 +7457,7 @@ def test_embind_unsigned(self): self.do_run_in_out_file_test('embind/test_unsigned.cpp') def test_embind_val(self): - self.emcc_args += ['-lembind'] + self.emcc_args += ['-lembind', '-g'] self.do_run_in_out_file_test('embind/test_val.cpp') def test_embind_val_read_pointer(self): From 9057058ed97649935ac04e9bef8d2d7127b4af81 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Fri, 23 Feb 2024 16:22:15 -0800 Subject: [PATCH 6/9] Passes test_embind_val_cross_thread --- system/include/emscripten/val.h | 4 ++-- test/test_core.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index c6267916e58d8..59c8b34c50328 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -675,9 +675,9 @@ class val : public base_val { val& operator=(const val& v) & { if (uses_refcount()) { - internal::_emval_decref(handle); + internal::_emval_decref(as_handle()); } - handle = v.handle; + handle = v.as_handle(); thread = v.thread; if (uses_refcount()) { internal::_emval_incref(handle); diff --git a/test/test_core.py b/test/test_core.py index 04eeaebfc3e98..65e070700efd3 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -7470,7 +7470,7 @@ def test_embind_val_assignment(self): @node_pthreads def test_embind_val_cross_thread(self): - self.emcc_args += ['--bind'] + self.emcc_args += ['--bind', '-g'] create_file('test_embind_val_cross_thread.cpp', r''' #include #include From 0b9e498451c06c388753a8aea5ffb4976aa572e5 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Fri, 23 Feb 2024 16:39:16 -0800 Subject: [PATCH 7/9] With logging to be removed. --- src/embind/emval.js | 7 +++---- system/include/emscripten/val.h | 8 ++++---- test/embind/test_val_coro.cpp | 1 + 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/embind/emval.js b/src/embind/emval.js index c94d66fc3311c..f784116bf52c4 100644 --- a/src/embind/emval.js +++ b/src/embind/emval.js @@ -88,6 +88,7 @@ var LibraryEmVal = { const handle = emval_freelist.pop() || emval_handles.length; emval_handles[handle] = value; emval_handles[handle + 1] = 1; + console.error(`${handle} = ${value}`); return handle; } } @@ -98,16 +99,14 @@ var LibraryEmVal = { _emval_incref: (handle) => { if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}}) { emval_handles[handle + 1] += 1; + console.error(`${handle} inc ${emval_handles[handle + 1]}`); } }, _emval_decref__deps: ['$emval_freelist', '$emval_handles'], _emval_decref: (handle) => { -<<<<<<< HEAD + console.error(`${handle} dec ${emval_handles[handle + 1]-1}`); if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}} && 0 === --emval_handles[handle + 1]) { -======= - if (handle >= emval_handles_reserved && 0 === --emval_handles[handle * 2 + 1]) { ->>>>>>> d90ee0942 (fix syntax errors) #if ASSERTIONS assert(emval_handles[handle] !== undefined, `Decref for unallocated handle.`); #endif diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 59c8b34c50328..00be61b06d7cd 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -583,7 +583,7 @@ class base_val { } template - val val_ref(const T& v) const; + unique_val val_ref(const T& v) const; const base_val& val_ref(const base_val& v) const { return v; @@ -817,8 +817,8 @@ inline unique_val base_val::await() const { } template -inline val base_val::val_ref(const T& v) const { - return val(v); +inline unique_val base_val::val_ref(const T& v) const { + return unique_val(v); } struct base_val::iterator { @@ -852,7 +852,7 @@ class base_val::awaiter { // - initially created with promise // - waiting with a given coroutine handle // - completed with a result - std::variant, val> state; + std::variant, unique_val> state; constexpr static std::size_t STATE_PROMISE = 0; constexpr static std::size_t STATE_CORO = 1; diff --git a/test/embind/test_val_coro.cpp b/test/embind/test_val_coro.cpp index 442667e2a56cd..7226ee73dd5b2 100644 --- a/test/embind/test_val_coro.cpp +++ b/test/embind/test_val_coro.cpp @@ -7,6 +7,7 @@ using namespace emscripten; EM_JS(EM_VAL, promise_sleep_impl, (int ms, int result), { + console.error(`promise_sleep_impl ${ms} ${result}`); let promise = new Promise(resolve => setTimeout(resolve, ms, result)); let handle = Emval.toHandle(promise); // FIXME. See https://github.com/emscripten-core/emscripten/issues/16975. From 3c96147ca654bf62738472c0656981ccc6c690b7 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Fri, 23 Feb 2024 22:26:08 -0800 Subject: [PATCH 8/9] No incref for val::awaiter --- src/embind/emval.js | 1 + system/include/emscripten/val.h | 21 ++++++++++++++++----- test/test_core.py | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/embind/emval.js b/src/embind/emval.js index f784116bf52c4..a683eae71a4e1 100644 --- a/src/embind/emval.js +++ b/src/embind/emval.js @@ -100,6 +100,7 @@ var LibraryEmVal = { if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}}) { emval_handles[handle + 1] += 1; console.error(`${handle} inc ${emval_handles[handle + 1]}`); + console.error((new Error('').stack)); } }, diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h index 00be61b06d7cd..41c0ac413ff5f 100644 --- a/system/include/emscripten/val.h +++ b/system/include/emscripten/val.h @@ -529,7 +529,10 @@ class base_val { #if __cplusplus >= 202002L class awaiter; - awaiter operator co_await() const; + awaiter operator co_await() const&; + + class awaiter; + awaiter operator co_await() &&; class promise_type; #endif @@ -859,8 +862,10 @@ class base_val::awaiter { constexpr static std::size_t STATE_RESULT = 2; public: + awaiter(unique_val promise) + : state(std::in_place_index, std::move(promise)) {} awaiter(const val& promise) - : state(std::in_place_index, promise) {} + : state(std::in_place_index, val(promise)) {} // just in case, ensure nobody moves / copies this type around awaiter(awaiter&&) = delete; @@ -888,8 +893,14 @@ class base_val::awaiter { val await_resume() { return std::move(std::get(state)); } }; -inline val::awaiter base_val::operator co_await() const { - return {*this}; +inline val::awaiter base_val::operator co_await() const& { + return {val(*this)}; +} + +inline val::awaiter base_val::operator co_await() && { + unique_val tmp; + tmp.move_assignment(std::move(*this)); + return {std::move(tmp)}; } // `promise_type` is a well-known subtype with well-known method names @@ -910,7 +921,7 @@ class base_val::promise_type { } // Return the stored promise as the actual return value of the coroutine. - val get_return_object() { return promise; } + unique_val get_return_object() { return std::move(promise); } // For similarity with JS async functions, our coroutines are eagerly evaluated. auto initial_suspend() noexcept { return std::suspend_never{}; } diff --git a/test/test_core.py b/test/test_core.py index 65e070700efd3..3870fd0d6fec9 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -7527,7 +7527,7 @@ def test_embind_val_coro(self): create_file('post.js', r'''Module.onRuntimeInitialized = () => { Module.asyncCoro().then(console.log); }''') - self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js'] + self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-g'] self.do_runf('embind/test_val_coro.cpp', '34\n') def test_embind_val_coro_caught(self): From 9b63ac758fef4f4176b6e2b20ac2c3b052311402 Mon Sep 17 00:00:00 2001 From: Mike Rolig Date: Fri, 23 Feb 2024 22:31:59 -0800 Subject: [PATCH 9/9] remove debug --- src/embind/emval.js | 4 ---- test/embind/test_val_coro.cpp | 1 - test/test_core.py | 4 ++-- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/embind/emval.js b/src/embind/emval.js index a683eae71a4e1..8d22ae24839ad 100644 --- a/src/embind/emval.js +++ b/src/embind/emval.js @@ -88,7 +88,6 @@ var LibraryEmVal = { const handle = emval_freelist.pop() || emval_handles.length; emval_handles[handle] = value; emval_handles[handle + 1] = 1; - console.error(`${handle} = ${value}`); return handle; } } @@ -99,14 +98,11 @@ var LibraryEmVal = { _emval_incref: (handle) => { if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}}) { emval_handles[handle + 1] += 1; - console.error(`${handle} inc ${emval_handles[handle + 1]}`); - console.error((new Error('').stack)); } }, _emval_decref__deps: ['$emval_freelist', '$emval_handles'], _emval_decref: (handle) => { - console.error(`${handle} dec ${emval_handles[handle + 1]-1}`); if (handle > {{{ EMVAL_LAST_RESERVED_HANDLE }}} && 0 === --emval_handles[handle + 1]) { #if ASSERTIONS assert(emval_handles[handle] !== undefined, `Decref for unallocated handle.`); diff --git a/test/embind/test_val_coro.cpp b/test/embind/test_val_coro.cpp index 7226ee73dd5b2..442667e2a56cd 100644 --- a/test/embind/test_val_coro.cpp +++ b/test/embind/test_val_coro.cpp @@ -7,7 +7,6 @@ using namespace emscripten; EM_JS(EM_VAL, promise_sleep_impl, (int ms, int result), { - console.error(`promise_sleep_impl ${ms} ${result}`); let promise = new Promise(resolve => setTimeout(resolve, ms, result)); let handle = Emval.toHandle(promise); // FIXME. See https://github.com/emscripten-core/emscripten/issues/16975. diff --git a/test/test_core.py b/test/test_core.py index 3870fd0d6fec9..fb5e87304f4f1 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -7457,7 +7457,7 @@ def test_embind_unsigned(self): self.do_run_in_out_file_test('embind/test_unsigned.cpp') def test_embind_val(self): - self.emcc_args += ['-lembind', '-g'] + self.emcc_args += ['-lembind'] self.do_run_in_out_file_test('embind/test_val.cpp') def test_embind_val_read_pointer(self): @@ -7527,7 +7527,7 @@ def test_embind_val_coro(self): create_file('post.js', r'''Module.onRuntimeInitialized = () => { Module.asyncCoro().then(console.log); }''') - self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js', '-g'] + self.emcc_args += ['-std=c++20', '--bind', '--post-js=post.js'] self.do_runf('embind/test_val_coro.cpp', '34\n') def test_embind_val_coro_caught(self):