Refactor Zend strings (#77)

* Call zval destructor when changing zval type and dropping

* Remove `ZendHashTable` wrapper

Replaced by returning references to the actual hashtable, and having a
new type `OwnedHashTable` when creating a new hashtable.

* Refactor `ZendString` into a borrowed and owned variant

`&ZendStr` is now equivalent to `&str`, while `ZendString` is equivalent
to `String`.

* Tidy up `ZendString`, add `Debug` implementation

Author: davidcole1340

Cargo.toml

@@ -12,6 +12,7 @@ categories = ["api-bindings"]
 
 [dependencies]
 bitflags = "1.2.1"
+parking_lot = "0.11.2"
 ext-php-rs-derive = { version = "=0.4.0", path = "./ext-php-rs-derive" }
 
 [build-dependencies]

build.rs

@@ -85,6 +85,7 @@ fn main() {
         .parse_callbacks(Box::new(bindgen::CargoCallbacks))
         .rustfmt_bindings(true)
         .no_copy("_zend_value")
+        .no_copy("_zend_string")
         .layout_tests(env::var("EXT_PHP_RS_TEST").is_ok());
 
     for binding in ALLOWED_BINDINGS.iter() {

example/skel/src/lib.rs

@@ -112,5 +112,10 @@ pub fn closure_count() -> Closure {
 
 #[php_module]
 pub fn module(module: ModuleBuilder) -> ModuleBuilder {
-    module
+    module.function(
+        FunctionBuilder::new("test_zval", test_zval)
+            .arg(Arg::new("test", DataType::Array))
+            .build()
+            .unwrap(),
+    )
 }

example/skel/test.php

@@ -1,6 +1,18 @@
 <?php
 
+<<<<<<< HEAD
+<<<<<<< HEAD
+<<<<<<< HEAD
 test_zval(['hello' => 'world']);
+=======
+test_zval();
+>>>>>>> 712aded56 (Call zval destructor when changing zval type and dropping)
+=======
+test_zval(['hello' => 'world']);
+>>>>>>> 3646da741 (Remove `ZendHashTable` wrapper)
+=======
+test_zval([]);
+>>>>>>> 7ba9ee9bf (Refactor `ZendString` into a borrowed and owned variant)
 //include 'vendor/autoload.php';
 
 //$ext = new ReflectionExtension('skel');

src/php/class.rs

@@ -38,10 +38,10 @@ impl ClassEntry {
     /// not be found or the class table has not been initialized.
     pub fn try_find(name: &str) -> Option<&'static Self> {
         ExecutorGlobals::get().class_table()?;
-        let name = ZendString::new(name, false).ok()?;
+        let mut name = ZendString::new(name, false).ok()?;
 
         unsafe {
-            crate::bindings::zend_lookup_class_ex(name.borrow_ptr(), std::ptr::null_mut(), 0)
+            crate::bindings::zend_lookup_class_ex(name.as_mut_zend_str(), std::ptr::null_mut(), 0)
                 .as_ref()
         }
     }
@@ -115,8 +115,7 @@ impl ClassEntry {
         if self.flags().contains(ClassFlags::ResolvedParent) {
             unsafe { self.__bindgen_anon_1.parent.as_ref() }
         } else {
-            let name =
-                unsafe { ZendString::from_ptr(self.__bindgen_anon_1.parent_name, false) }.ok()?;
+            let name = unsafe { self.__bindgen_anon_1.parent_name.as_ref()? };
             Self::try_find(name.as_str()?)
         }
     }
@@ -288,7 +287,7 @@ impl ClassBuilder {
     ///
     /// Returns an [`Error`] variant if the class could not be registered.
     pub fn build(mut self) -> Result<&'static mut ClassEntry> {
-        self.ptr.name = ZendString::new_interned(&self.name)?.release();
+        self.ptr.name = ZendString::new_interned(&self.name, true)?.into_inner();
 
         self.methods.push(FunctionEntry::end());
         let func = Box::into_raw(self.methods.into_boxed_slice()) as *const FunctionEntry;
@@ -349,9 +348,9 @@ impl ClassBuilder {
 
 impl Debug for ClassEntry {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let name: String = unsafe { ZendString::from_ptr(self.name, false) }
-            .and_then(|str| str.try_into())
-            .map_err(|_| std::fmt::Error)?;
+        let name: String = unsafe { self.name.as_ref() }
+            .and_then(|s| s.try_into().ok())
+            .ok_or(std::fmt::Error)?;
 
         f.debug_struct("ClassEntry")
             .field("name", &name)

src/php/types/array.rs

@@ -24,10 +24,7 @@ use crate::{
     php::enums::DataType,
 };
 
-use super::{
-    string::ZendString,
-    zval::{FromZval, IntoZval, Zval},
-};
+use super::zval::{FromZval, IntoZval, Zval};
 
 /// PHP array, which is represented in memory as a hashtable.
 pub use crate::bindings::HashTable;
@@ -239,9 +236,7 @@ impl<'a> Iterator for Iter<'a> {
         }
 
         let bucket = unsafe { pos.as_ref() };
-        let key = unsafe { ZendString::from_ptr(bucket.key, false) }
-            .and_then(|s| s.try_into())
-            .ok();
+        let key = unsafe { bucket.key.as_ref() }.and_then(|s| s.try_into().ok());
 
         self.pos = NonNull::new(unsafe { pos.as_ptr().offset(1) });
 
@@ -272,9 +267,7 @@ impl<'a> DoubleEndedIterator for Iter<'a> {
 
         let new_end = NonNull::new(unsafe { end.as_ptr().offset(-1) })?;
         let bucket = unsafe { new_end.as_ref() };
-        let key = unsafe { ZendString::from_ptr(bucket.key, false) }
-            .and_then(|s| s.try_into())
-            .ok();
+        let key = unsafe { bucket.key.as_ref() }.and_then(|s| s.try_into().ok());
         self.end = Some(new_end);
 
         Some((bucket.h, key, &bucket.val))
@@ -370,7 +363,11 @@ impl OwnedHashTable {
         }
     }
 
-    /// Returns the inner pointer to the hashtable, without destroying the
+    /// Converts the owned Zend hashtable into the internal pointer, bypassing the [`Drop`]
+    /// implementation.
+    ///
+    /// The caller is responsible for freeing the resulting pointer using the `zend_array_destroy`
+    /// function.
     pub fn into_inner(self) -> *mut HashTable {
         let this = ManuallyDrop::new(self);
         this.ptr.as_ptr()

src/php/types/object.rs

@@ -45,14 +45,13 @@ pub enum PropertyQuery {
 impl ZendObject {
     /// Attempts to retrieve the class name of the object.
     pub fn get_class_name(&self) -> Result<String> {
-        let name = unsafe {
-            ZendString::from_ptr(
-                self.handlers()?.get_class_name.ok_or(Error::InvalidScope)?(self),
-                false,
-            )
-        }?;
-
-        name.try_into()
+        unsafe {
+            self.handlers()?
+                .get_class_name
+                .and_then(|f| f(self).as_ref())
+                .ok_or(Error::InvalidScope)
+                .and_then(|s| s.try_into())
+        }
     }
 
     /// Checks if the given object is an instance of a registered class with Rust
@@ -74,13 +73,13 @@ impl ZendObject {
             return Err(Error::InvalidProperty);
         }
 
-        let name = ZendString::new(name, false)?;
+        let mut name = ZendString::new(name, false)?;
         let mut rv = Zval::new();
 
         unsafe {
             self.handlers()?.read_property.ok_or(Error::InvalidScope)?(
                 self.mut_ptr(),
-                name.borrow_ptr(),
+                name.as_mut_zend_str(),
                 1,
                 std::ptr::null_mut(),
                 &mut rv,
@@ -98,13 +97,13 @@ impl ZendObject {
     /// * `name` - The name of the property.
     /// * `value` - The value to set the property to.
     pub fn set_property(&mut self, name: &str, value: impl IntoZval) -> Result<&Zval> {
-        let name = ZendString::new(name, false)?;
+        let mut name = ZendString::new(name, false)?;
         let mut value = value.into_zval(false)?;
 
         unsafe {
             self.handlers()?.write_property.ok_or(Error::InvalidScope)?(
                 self,
-                name.borrow_ptr(),
+                name.as_mut_zend_str(),
                 &mut value,
                 std::ptr::null_mut(),
             )
@@ -127,7 +126,7 @@ impl ZendObject {
         Ok(unsafe {
             self.handlers()?.has_property.ok_or(Error::InvalidScope)?(
                 self.mut_ptr(),
-                name.borrow_ptr(),
+                name.deref() as *const _ as *mut _,
                 query as _,
                 std::ptr::null_mut(),
             )