From e3880c20557e8e7ff1e20d4dc0a657209113103e Mon Sep 17 00:00:00 2001 From: Andries Hiemstra Date: Mon, 19 Apr 2021 17:10:17 +0200 Subject: [PATCH] see #44 --- CHANGELOG.md | 1 + src/droppable_value.rs | 50 --------------------- src/lib.rs | 1 - src/quickjs_utils/atoms.rs | 12 ++++- src/quickjs_utils/mod.rs | 1 + src/quickjs_utils/objects.rs | 80 ++++++++++----------------------- src/quickjs_utils/properties.rs | 49 ++++++++++++++++++++ 7 files changed, 85 insertions(+), 109 deletions(-) delete mode 100644 src/droppable_value.rs create mode 100644 src/quickjs_utils/properties.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b96708330..ac692b3855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * moved reflection code to reflection/mod.rs (should not affect api) * toPrimitive for Proxy classes (do stuff like console.log('got: ' + MyProxyInstanceOrClass)) +* removed droppablevalue, replaced with JSPropertyEnumRef # 0.4.1 diff --git a/src/droppable_value.rs b/src/droppable_value.rs deleted file mode 100644 index e1bfb9fa97..0000000000 --- a/src/droppable_value.rs +++ /dev/null @@ -1,50 +0,0 @@ -/// A small wrapper that frees resources that have to be freed -/// automatically when they go out of scope. - -// todo i'm guesing i can replace this with a auto freeing AtomRef - -pub struct DroppableValue -where - F: FnMut(&mut T), -{ - value: T, - drop_fn: F, -} - -impl DroppableValue -where - F: FnMut(&mut T), -{ - pub fn new(value: T, drop_fn: F) -> Self { - Self { value, drop_fn } - } -} - -impl Drop for DroppableValue -where - F: FnMut(&mut T), -{ - fn drop(&mut self) { - (self.drop_fn)(&mut self.value); - } -} - -impl std::ops::Deref for DroppableValue -where - F: FnMut(&mut T), -{ - type Target = T; - - fn deref(&self) -> &T { - &self.value - } -} - -impl std::ops::DerefMut for DroppableValue -where - F: FnMut(&mut T), -{ - fn deref_mut(&mut self) -> &mut T { - &mut self.value - } -} diff --git a/src/lib.rs b/src/lib.rs index f6be5dea4c..576f3ac024 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,7 +62,6 @@ macro_rules! es_args { }; } -mod droppable_value; pub mod eserror; pub mod esruntime; pub mod esruntime_utils; diff --git a/src/quickjs_utils/atoms.rs b/src/quickjs_utils/atoms.rs index 4d2fd0aae0..c6707190fb 100644 --- a/src/quickjs_utils/atoms.rs +++ b/src/quickjs_utils/atoms.rs @@ -4,7 +4,7 @@ use crate::quickjs_utils::primitives; use crate::quickjscontext::QuickJsContext; use crate::valueref::JSValueRef; use libquickjs_sys as q; -use std::ffi::CString; +use std::ffi::{CStr, CString}; #[allow(clippy::upper_case_acronyms)] pub struct JSAtomRef { @@ -62,6 +62,16 @@ pub unsafe fn to_string2(context: *mut q::JSContext, atom: &q::JSAtom) -> Result primitives::to_string(context, &val_ref) } +/// # Safety +/// When passing a context pointer please make sure the corresponding QuickJsContext is still valid +pub unsafe fn to_str(context: *mut q::JSContext, atom: &q::JSAtom) -> Result<&str, EsError> { + let c_string = q::JS_AtomToCString(context, *atom); + let c_str = CStr::from_ptr(c_string); + c_str + .to_str() + .map_err(|e| EsError::new_string(format!("{}", e))) +} + pub fn from_string_q(q_ctx: &QuickJsContext, string: &str) -> Result { unsafe { from_string(q_ctx.context, string) } } diff --git a/src/quickjs_utils/mod.rs b/src/quickjs_utils/mod.rs index a642dd3749..82dd1a1ad4 100644 --- a/src/quickjs_utils/mod.rs +++ b/src/quickjs_utils/mod.rs @@ -14,6 +14,7 @@ pub mod modules; pub mod objects; pub mod primitives; pub mod promises; +pub mod properties; pub mod sets; pub mod typedarrays; diff --git a/src/quickjs_utils/objects.rs b/src/quickjs_utils/objects.rs index 840429336b..91a4534775 100644 --- a/src/quickjs_utils/objects.rs +++ b/src/quickjs_utils/objects.rs @@ -1,13 +1,12 @@ //! Utils for working with objects -use crate::droppable_value::DroppableValue; use crate::eserror::EsError; +use crate::quickjs_utils::properties::JSPropertyEnumRef; use crate::quickjs_utils::{atoms, functions, get_constructor, get_global}; use crate::quickjscontext::QuickJsContext; use crate::quickjsruntime::{make_cstring, QuickJsRuntime}; use crate::valueref::JSValueRef; use libquickjs_sys as q; -use std::collections::HashMap; /// get a namespace object /// this is used to get nested object properties which are used as namespaces @@ -370,6 +369,7 @@ pub unsafe fn get_property( Ok(prop_ref) } +/// get the names of all properties of an object pub fn get_property_names_q( q_ctx: &QuickJsContext, obj_ref: &JSValueRef, @@ -377,12 +377,15 @@ pub fn get_property_names_q( unsafe { get_property_names(q_ctx.context, obj_ref) } } +/// get the names of all properties of an object /// # Safety /// When passing a context pointer please make sure the corresponding QuickJsContext is still valid pub unsafe fn get_property_names( context: *mut q::JSContext, obj_ref: &JSValueRef, ) -> Result, EsError> { + // todo wrap this in a get_own_property_names which returns the JSPropertyEnumRef + let mut properties: *mut q::JSPropertyEnum = std::ptr::null_mut(); let mut count: u32 = 0; @@ -398,43 +401,25 @@ pub unsafe fn get_property_names( return Err(EsError::new_str("Could not get object properties")); } - let properties = DroppableValue::new(properties, |&mut properties| { - for index in 0..count { - let prop = properties.offset(index as isize); - - q::JS_FreeAtom(context, (*prop).atom); - } + let enum_ref = JSPropertyEnumRef::new(context, properties, count); - q::js_free(context, properties as *mut std::ffi::c_void); - }); + let mut names = vec![]; - let mut res: Vec = vec![]; for index in 0..count { - let prop = (*properties).offset(index as isize); - - let key_value = q::JS_AtomToString(context, (*prop).atom); - let key_ref = JSValueRef::new( - context, - key_value, - false, - true, - "objects::get_property_names key_value", - ); - if key_ref.is_exception() { - return Err(EsError::new_str("Could not get object property name")); - } + let atom = enum_ref.get_atom_raw(index) as q::JSAtom; + let name = atoms::to_str(context, &atom)?; - let key_str = crate::quickjs_utils::primitives::to_string(context, &key_ref)?; - res.push(key_str); + names.push(name.to_string()); } - Ok(res) + + Ok(names) } pub fn traverse_properties_q( q_ctx: &QuickJsContext, obj_ref: &JSValueRef, visitor: V, -) -> Result, EsError> +) -> Result, EsError> where V: Fn(&str, JSValueRef) -> Result, { @@ -447,7 +432,7 @@ pub unsafe fn traverse_properties( context: *mut q::JSContext, obj_ref: &JSValueRef, visitor: V, -) -> Result, EsError> +) -> Result, EsError> where V: Fn(&str, JSValueRef) -> Result, { @@ -466,24 +451,18 @@ where return Err(EsError::new_str("Could not get object properties")); } - let properties = DroppableValue::new(properties, |&mut properties| { - for index in 0..count { - let prop = properties.offset(index as isize); + let enum_ref = JSPropertyEnumRef::new(context, properties, count); - q::JS_FreeAtom(context, (*prop).atom); - } - - q::js_free(context, properties as *mut std::ffi::c_void); - }); - - let mut map = HashMap::new(); + let mut result = vec![]; for index in 0..count { - let prop = (*properties).offset(index as isize); + let atom = enum_ref.get_atom_raw(index) as q::JSAtom; + let prop_name = atoms::to_str(context, &atom)?; + let raw_value = q::JS_GetPropertyInternal( context, *obj_ref.borrow_value(), - (*prop).atom, + atom, *obj_ref.borrow_value(), 0, ); @@ -498,25 +477,12 @@ where return Err(EsError::new_str("Could not get object property")); } - let key_value = q::JS_AtomToString(context, (*prop).atom); - let key_ref = JSValueRef::new( - context, - key_value, - false, - true, - "objects::traverse_properties key_value", - ); - if key_ref.is_exception() { - return Err(EsError::new_str("Could not get object property name")); - } - - let key_str = crate::quickjs_utils::primitives::to_string(context, &key_ref)?; - let r = visitor(key_str.as_str(), prop_val_ref)?; + let r = visitor(prop_name, prop_val_ref)?; - map.insert(key_str, r); + result.push(r); } - Ok(map) + Ok(result) } pub fn is_instance_of_q( diff --git a/src/quickjs_utils/properties.rs b/src/quickjs_utils/properties.rs new file mode 100644 index 0000000000..0de90832e8 --- /dev/null +++ b/src/quickjs_utils/properties.rs @@ -0,0 +1,49 @@ +use crate::quickjs_utils::atoms::JSAtomRef; +use libquickjs_sys as q; + +pub struct JSPropertyEnumRef { + context: *mut q::JSContext, + property_enum: *mut q::JSPropertyEnum, + length: u32, +} + +impl JSPropertyEnumRef { + pub fn new( + context: *mut q::JSContext, + property_enum: *mut q::JSPropertyEnum, + length: u32, + ) -> Self { + Self { + context, + property_enum, + length, + } + } + pub unsafe fn get_atom_raw(&self, index: u32) -> *mut q::JSAtom { + if index >= self.length { + panic!("index out of bounds"); + } + let prop: *mut q::JSPropertyEnum = self.property_enum.offset(index as isize); + let atom: *mut q::JSAtom = (*prop).atom as *mut q::JSAtom; + atom + } + pub fn get_atom(&self, index: u32) -> JSAtomRef { + let atom: *mut q::JSAtom = unsafe { self.get_atom_raw(index) }; + let atom_ref = JSAtomRef::new(self.context, atom as q::JSAtom); + atom_ref.increment_ref_ct(); + return atom_ref; + } +} + +impl Drop for JSPropertyEnumRef { + fn drop(&mut self) { + unsafe { + for index in 0..self.length { + let prop: *mut q::JSPropertyEnum = self.property_enum.offset(index as isize); + q::JS_FreeAtom(self.context, (*prop).atom); + } + + q::js_free(self.context, self.property_enum as *mut std::ffi::c_void); + } + } +}