Merge pull request #29 from ubolonton/rework-vector

Rework Vector

Author: ubolonton

.azure-pipelines/main.yml

@@ -25,12 +25,13 @@ strategy:
     macos-10.14-emacs-26.3:
       IMAGE_NAME: 'macos-10.14'
       EVM_EMACS: emacs-26.3
-    macos-10.15-emacs-25.3:
-      IMAGE_NAME: 'macos-10.15'
-      EVM_EMACS: emacs-25.3
-    macos-10.15-emacs-26.3:
-      IMAGE_NAME: 'macos-10.15'
-      EVM_EMACS: emacs-26.3
+    # Emacs is getting "Killed 9" on these agents, probably due to macOS 10.15 updates.
+    # macos-10.15-emacs-25.3:
+    #   IMAGE_NAME: 'macos-10.15'
+    #   EVM_EMACS: emacs-25.3
+    # macos-10.15-emacs-26.3:
+    #   IMAGE_NAME: 'macos-10.15'
+    #   EVM_EMACS: emacs-26.3
 
     ubuntu-16.04-emacs-25.3:
       IMAGE_NAME: 'ubuntu-16.04'

.azure-pipelines/steps/-test.yml

@@ -1,10 +1,13 @@
 steps:
 - powershell: |
+    cargo test --all
     .\bin\test.ps1
   displayName: Run tests (Windows)
   condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
 
 - bash: |
+    set -e
+    cargo test --all
     ./bin/test
   displayName: Run tests
   condition: and(succeeded(), ne(variables['Agent.OS'], 'Windows_NT'))

CHANGELOG.md

@@ -4,6 +4,8 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
+- Reworked `Vector`, making it iterable.
+- Changed `Env::vector` to return a `Value`.
 
 ## [0.13.0] - 2020-03-11
 - Added `GlobalRef`, which allows keeping Lisp values around without an `Env`.

guide/src/calling-lisp.md

@@ -50,8 +50,8 @@ env.call("add-hook", [
 #[defun]
 fn listify_vec(vector: Vector) -> Result<Value> {
     let mut args = vec![];
-    for i in 0..vector.size()? {
-        args.push(vector.get(i)?)
+    for e in vector {
+        args.push(e)
     }
     vector.0.env.call("list", &args)
 }

guide/src/functions.md

@@ -34,8 +34,8 @@ Each parameter must be one of the following:
     #[defun(user_ptr)]
     fn to_rust_vec_string(input: Vector) -> Result<Vec<String>> {
         let mut vec = vec![];
-        for i in 0..input.size()? {
-            vec.push(input.get(i)?.into_rust()?);
+        for e in input {
+            vec.push(e.into_rust()?);
         }
         Ok(vec)
     }

src/call.rs

@@ -25,8 +25,7 @@ impl<'e> Value<'e> {
     /// # use emacs::{defun, Value, Result, Vector};
     /// #[defun]
     /// fn mapc_enumerate_vec(function: Value, vector: Vector) -> Result<()> {
-    ///     for nth in 0..vector.size()? {
-    ///         let elem: Value = vector.get(nth)?;
+    ///     for (nth, elem) in vector.into_iter().enumerate() {
     ///         function.call((nth, elem))?;
     ///     }
     ///     Ok(())
@@ -65,10 +64,10 @@ impl Env {
     /// # use emacs::{defun, Value, Result, Vector};
     /// #[defun]
     /// fn listify_vec(vector: Vector) -> Result<Value> {
-    ///     let env = vector.0.env;
+    ///     let env = vector.value().env;
     ///     let mut args = vec![];
-    ///     for i in 0..vector.size()? {
-    ///         args.push(vector.get(i)?)
+    ///     for elem in vector {
+    ///         args.push(elem)
     ///     }
     ///     env.call("list", &args)
     /// }

src/types/vector.rs

@@ -3,12 +3,11 @@ use std::convert::TryInto;
 use super::*;
 use crate::{subr, call::IntoLispArgs};
 
-/// A type that represents Lisp vectors. This is a newtype wrapper around [`Value`] that provides
+/// A type that represents Lisp vectors. This is a wrapper around [`Value`] that provides
 /// vector-specific methods.
 ///
-/// Arguments to #[[`defun`]] having this type will be type-checked. This type checking can be
-/// omitted by manually wrapping a [`Value`]. Note that Emacs still does type checking when calling
-/// methods on the vector.
+/// Arguments to #[[`defun`]] having this type will be type-checked. If you want to omit, or delay
+/// this type checking, use [`Value`] instead.
 ///
 /// ```
 /// use emacs::{defun, Value, Vector, Result};
@@ -20,62 +19,121 @@ use crate::{subr, call::IntoLispArgs};
 ///
 /// #[defun]
 /// fn no_type_check(value: Value) -> Result<Vector> {
-///     Ok(Vector(value))
+///     Ok(Vector::from_value_unchecked(value, 0))
 /// }
 /// ```
 ///
 /// [`Value`]: struct.Value.html
 /// [`defun`]: attr.defun.html
 #[derive(Debug, Clone, Copy)]
-pub struct Vector<'e>(pub Value<'e>);
+pub struct Vector<'e> {
+    value: Value<'e>,
+    len: usize,
+}
 
 impl<'e> Vector<'e> {
+    #[doc(hidden)]
+    #[inline]
+    pub fn from_value_unchecked(value: Value<'e>, len: usize) -> Self {
+        Self { value, len }
+    }
+
     pub fn get<T: FromLisp<'e>>(&self, i: usize) -> Result<T> {
-        let v = self.0;
+        let v = self.value;
         let env = v.env;
         // Safety: Same lifetime. Emacs does bound checking.
         unsafe_raw_call_value!(env, vec_get, v.raw, i as isize)?.into_rust()
     }
 
     pub fn set<T: IntoLisp<'e>>(&self, i: usize, value: T) -> Result<()> {
-        let v = self.0;
+        let v = self.value;
         let env = v.env;
         let value = value.into_lisp(env)?;
         // Safety: Same lifetime. Emacs does bound checking.
         unsafe_raw_call!(env, vec_set, v.raw, i as isize, value.raw)
     }
 
+    #[deprecated(since = "0.14.0", note = "Use .len() instead")]
+    #[doc(hidden)]
     pub fn size(&self) -> Result<usize> {
-        let v = self.0;
-        let env = v.env;
-        let result =
-            unsafe_raw_call_no_exit!(env, vec_size, v.raw).try_into().expect("invalid size from Emacs");
-        env.handle_exit(result)
+        Ok(self.len)
+    }
+
+    #[inline]
+    pub fn len(&self) -> usize {
+        self.len
+    }
+
+    #[inline]
+    pub fn value(&self) -> Value<'e> {
+        self.value
     }
 }
 
 impl<'e> FromLisp<'e> for Vector<'e> {
     fn from_lisp(value: Value<'e>) -> Result<Vector<'e>> {
-        let vector = Vector(value);
-        // TODO: Confirm that this is indeed cheaper than calling vectorp and signaling error.
-        vector.size()?;
-        Ok(vector)
+        let env = value.env;
+        let len = unsafe_raw_call!(env, vec_size, value.raw)?.try_into()
+            .expect("Invalid size from Emacs");
+        Ok(Vector { value, len })
     }
 }
 
 impl<'e> IntoLisp<'e> for Vector<'e> {
     #[inline(always)]
     fn into_lisp(self, _: &'e Env) -> Result<Value<'_>> {
-        Ok(self.0)
+        Ok(self.value)
+    }
+}
+
+/// An iterator over the elements of a [`Vector`], as [`Value`] structs.
+///
+/// [`Vector`]: struct.Vector.html
+/// [`Value`]: struct.Value.html
+pub struct IntoIter<'e> {
+    vector: Vector<'e>,
+    i: usize,
+}
+
+impl<'e> Iterator for IntoIter<'e> {
+    type Item = Value<'e>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let i = self.i;
+        if i >= self.vector.len {
+            None
+        } else {
+            self.i += 1;
+            Some(self.vector.get(i).unwrap_or_else(|err| panic!(err)))
+        }
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        let remaining = self.vector.len - self.i;
+        (remaining, Some(remaining))
+    }
+}
+
+impl<'e> ExactSizeIterator for IntoIter<'e> {}
+
+impl<'e> IntoIterator for Vector<'e> {
+    type Item = Value<'e>;
+
+    type IntoIter = IntoIter<'e>;
+
+    #[inline]
+    fn into_iter(self) -> Self::IntoIter {
+        IntoIter { vector: self, i: 0 }
     }
 }
 
 impl Env {
     pub fn make_vector<'e, T: IntoLisp<'e>>(&'e self, length: usize, init: T) -> Result<Vector> {
-        self.call(subr::make_vector, (length, init)).map(Vector)
+        let value = self.call(subr::make_vector, (length, init))?;
+        Ok(Vector::from_value_unchecked(value, length))
     }
 
-    pub fn vector<'e, A: IntoLispArgs<'e>>(&'e self, args: A) -> Result<Vector> {
-        self.call(subr::vector, args).map(Vector)
+    pub fn vector<'e, A: IntoLispArgs<'e>>(&'e self, args: A) -> Result<Value> {
+        self.call(subr::vector, args)
     }
 }

test-module/src/call.rs

@@ -43,8 +43,7 @@ fn value<'e>(function: Value<'e>, arg: Value) -> Result<Value<'e>> {
 
 #[defun]
 fn mapc_vec(function: Value, vector: emacs::Vector) -> Result<()> {
-    for i in 0..vector.size()? as usize {
-        let elem: Value = vector.get(i)?;
+    for (i, elem) in vector.into_iter().enumerate() {
         function.call((i, elem))?;
     }
     Ok(())

test-module/src/test_vector.rs

@@ -3,8 +3,8 @@
 use emacs::{defun, Result, Value, Vector};
 
 #[defun(mod_in_name = false)]
-fn vec_size(v: Vector) -> Result<i64> {
-    v.size().map(|s| s as i64)
+fn vec_size(v: Vector) -> Result<usize> {
+    Ok(v.len())
 }
 
 #[defun(mod_in_name = false)]
@@ -24,7 +24,7 @@ fn identity_if_vector(v: Vector) -> Result<Vector> {
 
 #[defun(mod_in_name = false)]
 fn stringify_num_vector(v: Vector) -> Result<Vector> {
-    for i in 0..v.size()? {
+    for i in 0..v.len() {
         let x: i64 = v.get(i)?;
         v.set(i, format!("{}", x))?;
     }