feat(iterator): simply stop when there is an exception, do not take it from EG to avoid segfault

Author: joelwurtz

docsrs_bindings.rs

@@ -17,6 +17,7 @@ pub const IS_RESOURCE: u32 = 9;
 pub const IS_REFERENCE: u32 = 10;
 pub const IS_CONSTANT_AST: u32 = 11;
 pub const IS_CALLABLE: u32 = 12;
+pub const IS_ITERABLE: u32 = 13;
 pub const IS_VOID: u32 = 14;
 pub const IS_MIXED: u32 = 16;
 pub const IS_PTR: u32 = 13;
@@ -1455,6 +1456,9 @@ extern "C" {
         named_params: *mut HashTable,
     );
 }
+extern "C" {
+    pub fn zend_is_iterable(iterable: *mut zval) -> bool;
+}
 pub const _zend_expected_type_Z_EXPECTED_LONG: _zend_expected_type = 0;
 pub const _zend_expected_type_Z_EXPECTED_LONG_OR_NULL: _zend_expected_type = 1;
 pub const _zend_expected_type_Z_EXPECTED_BOOL: _zend_expected_type = 2;

guide/src/types/iterable.md

@@ -19,7 +19,7 @@ that implements the `Traversable` interface. This means that any value that can
 # use ext_php_rs::types::Iterable;
 #[php_function]
 pub fn test_iterable(mut iterable: Iterable) {
-    for (k, v) in iterable.iter() {
+    for (k, v) in iterable.iter().expect("cannot get iterable") {
         println!("k: {} v: {}", k, v.string().unwrap());
     }
 }

guide/src/types/iterator.md

@@ -20,7 +20,7 @@ used but also a the result of a `query` call with `PDO`.
 # use ext_php_rs::types::ZendIterator;
 #[php_function]
 pub fn test_iterator(iterator: &mut ZendIterator) {
-    for (k, v) in iterator.iter() {
+    for (k, v) in iterator.iter().expect("cannot get iterator") {
         println!("k: {} v: {}", k, v.string().unwrap());
     }
 }

src/types/iterable.rs

@@ -1,7 +1,6 @@
 use super::array::Iter as ZendHashTableIter;
 use super::iterator::Iter as ZendIteratorIter;
 use crate::convert::FromZval;
-use crate::exception::PhpResult;
 use crate::flags::DataType;
 use crate::types::iterator::IterKey;
 use crate::types::{ZendHashTable, ZendIterator, Zval};
@@ -16,10 +15,10 @@ pub enum Iterable<'a> {
 
 impl<'a> Iterable<'a> {
     /// Creates a new rust iterator from a PHP iterable.
-    pub fn iter(&mut self) -> PhpResult<Iter> {
+    pub fn iter(&mut self) -> Option<Iter> {
         match self {
-            Iterable::Array(array) => Ok(Iter::Array(array.iter())),
-            Iterable::Traversable(traversable) => Ok(Iter::Traversable(traversable.iter()?)),
+            Iterable::Array(array) => Some(Iter::Array(array.iter())),
+            Iterable::Traversable(traversable) => Some(Iter::Traversable(traversable.iter()?)),
         }
     }
 }
@@ -47,11 +46,11 @@ pub enum Iter<'a> {
 }
 
 impl<'a> Iterator for Iter<'a> {
-    type Item = PhpResult<(IterKey, &'a Zval)>;
+    type Item = (IterKey, &'a Zval);
 
     fn next(&mut self) -> Option<Self::Item> {
         match self {
-            Iter::Array(array) => array.next().map(Ok),
+            Iter::Array(array) => array.next(),
             Iter::Traversable(traversable) => traversable.next(),
         }
     }

src/types/iterator.rs

@@ -1,7 +1,6 @@
 use crate::convert::{FromZval, FromZvalMut};
 use crate::ffi::{zend_object_iterator, ZEND_RESULT_CODE_SUCCESS};
 use crate::flags::DataType;
-use crate::prelude::PhpResult;
 use crate::types::Zval;
 use crate::zend::ExecutorGlobals;
 use std::fmt::{Debug, Display, Formatter};
@@ -18,55 +17,70 @@ impl ZendIterator {
     /// # Returns
     ///
     /// Returns a iterator over the zend_object_iterator.
-    pub fn iter(&mut self) -> PhpResult<Iter> {
+    pub fn iter(&mut self) -> Option<Iter> {
         self.index = 0;
-        self.rewind()?;
 
-        Ok(Iter { zi: self })
+        if self.rewind() {
+            return Some(Iter { zi: self });
+        }
+
+        None
     }
 
     /// Check if the current position of the iterator is valid.
     ///
     /// As an example this will call the user defined valid method of the ['\Iterator'] interface.
     /// see <https://www.php.net/manual/en/iterator.valid.php>
-    pub fn valid(&mut self) -> PhpResult<bool> {
+    pub fn valid(&mut self) -> bool {
         if let Some(valid) = unsafe { (*self.funcs).valid } {
             let valid = unsafe { valid(&mut *self) == ZEND_RESULT_CODE_SUCCESS };
 
-            ExecutorGlobals::throw_if_exception()?;
+            if ExecutorGlobals::has_exception() {
+                return false;
+            }
 
-            Ok(valid)
+            valid
         } else {
-            Ok(true)
+            true
         }
     }
 
     /// Rewind the iterator to the first element.
     ///
     /// As an example this will call the user defined rewind method of the ['\Iterator'] interface.
     /// see <https://www.php.net/manual/en/iterator.rewind.php>
-    pub fn rewind(&mut self) -> PhpResult<()> {
+    ///
+    /// # Returns
+    ///
+    /// Returns true if the iterator was successfully rewind, false otherwise. (when there is
+    /// an exception during rewind)
+    pub fn rewind(&mut self) -> bool {
         if let Some(rewind) = unsafe { (*self.funcs).rewind } {
             unsafe {
                 rewind(&mut *self);
             }
         }
 
-        ExecutorGlobals::throw_if_exception()
+        !ExecutorGlobals::has_exception()
     }
 
     /// Move the iterator forward to the next element.
     ///
     /// As an example this will call the user defined next method of the ['\Iterator'] interface.
     /// see <https://www.php.net/manual/en/iterator.next.php>
-    pub fn move_forward(&mut self) -> PhpResult<()> {
+    ///
+    /// # Returns
+    ///
+    /// Returns true if the iterator was successfully move, false otherwise. (when there is
+    /// an exception during next)
+    pub fn move_forward(&mut self) -> bool {
         if let Some(move_forward) = unsafe { (*self.funcs).move_forward } {
             unsafe {
                 move_forward(&mut *self);
             }
         }
 
-        ExecutorGlobals::throw_if_exception()
+        !ExecutorGlobals::has_exception()
     }
 
     /// Get the current data of the iterator.
@@ -75,16 +89,15 @@ impl ZendIterator {
     ///
     /// Returns a reference to the current data of the iterator if available
     /// , ['None'] otherwise.
-    pub fn get_current_data<'a>(&mut self) -> PhpResult<Option<&'a Zval>> {
-        let get_current_data = match unsafe { (*self.funcs).get_current_data } {
-            Some(get_current_data) => get_current_data,
-            None => return Ok(None),
-        };
+    pub fn get_current_data<'a>(&mut self) -> Option<&'a Zval> {
+        let get_current_data = unsafe { (*self.funcs).get_current_data }?;
         let value = unsafe { &*get_current_data(&mut *self) };
 
-        ExecutorGlobals::throw_if_exception()?;
+        if ExecutorGlobals::has_exception() {
+            return None;
+        }
 
-        Ok(Some(value))
+        Some(value)
     }
 
     /// Get the current key of the iterator.
@@ -93,21 +106,19 @@ impl ZendIterator {
     ///
     /// Returns a new ['Zval'] containing the current key of the iterator if available
     /// , ['None'] otherwise.
-    pub fn get_current_key(&mut self) -> PhpResult<Option<Zval>> {
-        let get_current_key = match unsafe { (*self.funcs).get_current_key } {
-            Some(get_current_key) => get_current_key,
-            None => return Ok(None),
-        };
-
+    pub fn get_current_key(&mut self) -> Option<Zval> {
+        let get_current_key = unsafe { (*self.funcs).get_current_key? };
         let mut key = Zval::new();
 
         unsafe {
             get_current_key(&mut *self, &mut key);
         }
 
-        ExecutorGlobals::throw_if_exception()?;
+        if ExecutorGlobals::has_exception() {
+            return None;
+        }
 
-        Ok(Some(key))
+        Some(key)
     }
 }
 
@@ -164,40 +175,31 @@ pub struct Iter<'a> {
 }
 
 impl<'a> Iterator for Iter<'a> {
-    type Item = PhpResult<(IterKey, &'a Zval)>;
+    type Item = (IterKey, &'a Zval);
 
     fn next(&mut self) -> Option<Self::Item> {
         // Call next when index > 0, so next is really called at the start of each iteration, which allow to work better with generator iterator
-        if self.zi.index > 0 {
-            if let Err(err) = self.zi.move_forward() {
-                return Some(Err(err));
-            }
+        if self.zi.index > 0 && !self.zi.move_forward() {
+            return None;
         }
 
-        match self.zi.valid() {
-            Err(err) => return Some(Err(err)),
-            Ok(false) => return None,
-            Ok(true) => (),
+        if !self.zi.valid() {
+            return None;
         }
 
         self.zi.index += 1;
 
         let real_index = self.zi.index - 1;
 
         let key = match self.zi.get_current_key() {
-            Err(err) => return Some(Err(err)),
-            Ok(None) => IterKey::Long(real_index),
-            Ok(Some(key)) => match IterKey::from_zval(&key) {
+            None => IterKey::Long(real_index),
+            Some(key) => match IterKey::from_zval(&key) {
                 Some(key) => key,
                 None => IterKey::Long(real_index),
             },
         };
 
-        match self.zi.get_current_data() {
-            Err(err) => Some(Err(err)),
-            Ok(None) => None,
-            Ok(Some(value)) => Some(Ok((key, value))),
-        }
+        self.zi.get_current_data().map(|value| (key, value))
     }
 }
 
@@ -208,3 +210,133 @@ impl<'a> FromZvalMut<'a> for &'a mut ZendIterator {
         zval.object()?.get_class_entry().get_iterator(zval, false)
     }
 }
+
+#[cfg(test)]
+#[cfg(feature = "embed")]
+mod tests {
+    use crate::embed::Embed;
+    use crate::types::iterator::IterKey;
+
+    #[test]
+    fn test_generator() {
+        Embed::run(|| {
+            let result = Embed::run_script("src/types/iterator.test.php");
+
+            assert!(result.is_ok());
+
+            let generator = Embed::eval("$generator;");
+
+            assert!(generator.is_ok());
+
+            let zval = generator.unwrap();
+
+            assert!(zval.is_traversable());
+
+            let iterator = zval.traversable().unwrap();
+
+            assert!(iterator.valid());
+
+            {
+                let mut iter = iterator.iter().unwrap();
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::Long(0));
+                assert!(value.is_long());
+                assert_eq!(value.long().unwrap(), 1);
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::Long(1));
+                assert!(value.is_long());
+                assert_eq!(value.long().unwrap(), 2);
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::Long(2));
+                assert!(value.is_long());
+                assert_eq!(value.long().unwrap(), 3);
+
+                let next = iter.next();
+
+                assert!(next.is_none());
+            }
+        });
+    }
+
+    #[test]
+    fn test_iterator() {
+        Embed::run(|| {
+            let result = Embed::run_script("src/types/iterator.test.php");
+
+            assert!(result.is_ok());
+
+            let generator = Embed::eval("$iterator;");
+
+            assert!(generator.is_ok());
+
+            let zval = generator.unwrap();
+
+            assert!(zval.is_traversable());
+
+            let iterator = zval.traversable().unwrap();
+
+            assert!(iterator.valid());
+
+            {
+                let mut iter = iterator.iter().unwrap();
+
+                let (key, value) = iter.next().unwrap();
+
+                assert!(!key.is_numerical());
+                assert_eq!(key, IterKey::String("key".to_string()));
+                assert!(value.is_string());
+                assert_eq!(value.string().unwrap(), "foo");
+
+                let (key, value) = iter.next().unwrap();
+
+                assert!(key.is_numerical());
+                assert_eq!(key, IterKey::Long(10));
+                assert!(value.is_string());
+                assert_eq!(value.string().unwrap(), "bar");
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::Long(2));
+                assert!(value.is_string());
+                assert_eq!(value.string().unwrap(), "baz");
+
+                let next = iter.next();
+
+                assert!(next.is_none());
+            }
+
+            // Test rewind
+            {
+                let mut iter = iterator.iter().unwrap();
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::String("key".to_string()));
+                assert!(value.is_string());
+                assert_eq!(value.string().unwrap(), "foo");
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::Long(10));
+                assert!(value.is_string());
+                assert_eq!(value.string().unwrap(), "bar");
+
+                let (key, value) = iter.next().unwrap();
+
+                assert_eq!(key, IterKey::Long(2));
+                assert!(value.is_string());
+                assert_eq!(value.string().unwrap(), "baz");
+
+                let next = iter.next();
+
+                assert!(next.is_none());
+            }
+        });
+    }
+}