From 4e822ed6cdf02a7826f733a5edf0c5e4f4f6765b Mon Sep 17 00:00:00 2001 From: Joey Hain Date: Sun, 3 Mar 2019 12:26:50 -0800 Subject: [PATCH] Improve error messages and add docs for new native function machinery --- vm/src/function.rs | 46 ++++++++++- vm/src/pyobject.rs | 192 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 222 insertions(+), 16 deletions(-) diff --git a/vm/src/function.rs b/vm/src/function.rs index 434f995bb5..131fce129d 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -2,17 +2,48 @@ use std::marker::PhantomData; use std::ops::Deref; use crate::obj::objtype; -use crate::pyobject::{PyObjectPayload2, PyObjectRef, PyResult, TryFromObject}; +use crate::pyobject::{ + IntoPyObject, PyContext, PyObject, PyObjectPayload, PyObjectPayload2, PyObjectRef, PyResult, + TryFromObject, TypeProtocol, +}; use crate::vm::VirtualMachine; // TODO: Move PyFuncArgs, FromArgs, etc. here +// TODO: `PyRef` probably actually belongs in the pyobject module. + +/// A reference to the payload of a built-in object. +/// +/// Note that a `PyRef` can only deref to a shared / immutable reference. +/// It is the payload type's responsibility to handle (possibly concurrent) +/// mutability with locks or concurrent data structures if required. +/// +/// A `PyRef` can be directly returned from a built-in function to handle +/// situations (such as when implementing in-place methods such as `__iadd__`) +/// where a reference to the same object must be returned. pub struct PyRef { // invariant: this obj must always have payload of type T obj: PyObjectRef, _payload: PhantomData, } +impl PyRef +where + T: PyObjectPayload2, +{ + pub fn new(ctx: &PyContext, payload: T) -> Self { + PyRef { + obj: PyObject::new( + PyObjectPayload::AnyRustValue { + value: Box::new(payload), + }, + T::required_type(ctx), + ), + _payload: PhantomData, + } + } +} + impl Deref for PyRef where T: PyObjectPayload2, @@ -35,7 +66,18 @@ where _payload: PhantomData, }) } else { - Err(vm.new_type_error("wrong type".to_string())) // TODO: better message + let expected_type = vm.to_pystr(&T::required_type(&vm.ctx))?; + let actual_type = vm.to_pystr(&obj.typ())?; + Err(vm.new_type_error(format!( + "Expected type {}, not {}", + expected_type, actual_type, + ))) } } } + +impl IntoPyObject for PyRef { + fn into_pyobject(self, _ctx: &PyContext) -> PyResult { + Ok(self.obj) + } +} diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index c39aa71ae6..cb5bd7ed95 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -2,6 +2,7 @@ use std::cell::{Cell, RefCell}; use std::collections::HashMap; use std::fmt; use std::iter; +use std::ops::RangeInclusive; use std::rc::{Rc, Weak}; use crate::bytecode; @@ -937,6 +938,8 @@ impl PyFuncArgs { } } + /// Serializes these arguments into an iterator starting with the positional + /// arguments followed by keyword arguments. fn into_iter(self) -> impl Iterator { self.args.into_iter().map(PyArg::Positional).chain( self.kwargs @@ -945,30 +948,82 @@ impl PyFuncArgs { ) } + /// Binds these arguments to their respective values. + /// + /// If there is an insufficient number of arguments, there are leftover + /// arguments after performing the binding, or if an argument is not of + /// the expected type, a TypeError is raised. + /// + /// If the given `FromArgs` includes any conversions, exceptions raised + /// during the conversion will halt the binding and return the error. fn bind(self, vm: &mut VirtualMachine) -> PyResult { + let given_args = self.args.len(); let mut args = self.into_iter().peekable(); - let bound = T::from_args(vm, &mut args)?; + let bound = match T::from_args(vm, &mut args) { + Ok(args) => args, + Err(ArgumentError::TooFewArgs) => { + return Err(vm.new_type_error(format!( + "Expected at least {} arguments ({} given)", + T::arity().start(), + given_args, + ))); + } + Err(ArgumentError::Exception(ex)) => { + return Err(ex); + } + }; - if args.next().is_none() { - Ok(bound) - } else { - Err(vm.new_type_error("too many args".to_string())) // TODO: improve error message + match args.next() { + None => Ok(bound), + Some(PyArg::Positional(_)) => Err(vm.new_type_error(format!( + "Expected at most {} arguments ({} given)", + T::arity().end(), + given_args, + ))), + Some(PyArg::Keyword(name, _)) => { + Err(vm.new_type_error(format!("Unexpected keyword argument {}", name))) + } } } } +/// Implemented by any type that can be accepted as a parameter to a built-in +/// function. +/// pub trait FromArgs: Sized { - fn from_args(vm: &mut VirtualMachine, args: &mut iter::Peekable) -> PyResult + /// The range of positional arguments permitted by the function signature. + /// + /// Returns an empty range if not applicable. + fn arity() -> RangeInclusive { + 0..=0 + } + + /// Extracts this item from the next argument(s). + fn from_args( + vm: &mut VirtualMachine, + args: &mut iter::Peekable, + ) -> Result where I: Iterator; } -pub struct PyIterable { +/// An iterable Python object. +/// +/// `PyIterable` implements `FromArgs` so that a built-in function can accept +/// an object that is required to conform to the Python iterator protocol. +/// +/// PyIterable can optionally perform type checking and conversions on iterated +/// objects using a generic type parameter that implements `TryFromObject`. +pub struct PyIterable { method: PyObjectRef, _item: std::marker::PhantomData, } impl PyIterable { + /// Returns an iterator over this sequence of objects. + /// + /// This operation may fail if an exception is raised while invoking the + /// `__iter__` method of the iterable object. pub fn iter<'a>(&self, vm: &'a mut VirtualMachine) -> PyResult> { let iter_obj = vm.invoke( self.method.clone(), @@ -1037,13 +1092,24 @@ impl TryFromObject for PyObjectRef { } } -pub struct KwArgs(HashMap); +/// A map of keyword arguments to their values. +/// +/// A built-in function with a `KwArgs` parameter is analagous to a Python +/// function with `*kwargs`. All remaining keyword arguments are extracted +/// (and hence the function will permit an arbitrary number of them). +/// +/// `KwArgs` optionally accepts a generic type parameter to allow type checks +/// or conversions of each argument. +pub struct KwArgs(HashMap); impl FromArgs for KwArgs where T: TryFromObject, { - fn from_args(vm: &mut VirtualMachine, args: &mut iter::Peekable) -> PyResult + fn from_args( + vm: &mut VirtualMachine, + args: &mut iter::Peekable, + ) -> Result where I: Iterator, { @@ -1055,13 +1121,24 @@ where } } +/// A list of positional argument values. +/// +/// A built-in function with a `Args` parameter is analagous to a Python +/// function with `*args`. All remaining positional arguments are extracted +/// (and hence the function will permit an arbitrary number of them). +/// +/// `Args` optionally accepts a generic type parameter to allow type checks +/// or conversions of each argument. pub struct Args(Vec); impl FromArgs for Args where T: TryFromObject, { - fn from_args(vm: &mut VirtualMachine, args: &mut iter::Peekable) -> PyResult + fn from_args( + vm: &mut VirtualMachine, + args: &mut iter::Peekable, + ) -> Result where I: Iterator, { @@ -1077,14 +1154,21 @@ impl FromArgs for T where T: TryFromObject, { - fn from_args(vm: &mut VirtualMachine, args: &mut iter::Peekable) -> PyResult + fn arity() -> RangeInclusive { + 1..=1 + } + + fn from_args( + vm: &mut VirtualMachine, + args: &mut iter::Peekable, + ) -> Result where I: Iterator, { if let Some(PyArg::Positional(value)) = args.next() { Ok(T::try_from_object(vm, value)?) } else { - Err(vm.new_type_error("not enough args".to_string())) // TODO: improve error message + Err(ArgumentError::TooFewArgs) } } } @@ -1103,7 +1187,14 @@ impl FromArgs for OptArg where T: TryFromObject, { - fn from_args(vm: &mut VirtualMachine, args: &mut iter::Peekable) -> PyResult + fn arity() -> RangeInclusive { + 0..=1 + } + + fn from_args( + vm: &mut VirtualMachine, + args: &mut iter::Peekable, + ) -> Result where I: Iterator, { @@ -1125,10 +1216,31 @@ pub enum PyArg { Keyword(String, PyObjectRef), } +pub enum ArgumentError { + TooFewArgs, + Exception(PyObjectRef), +} + +impl From for ArgumentError { + fn from(ex: PyObjectRef) -> Self { + ArgumentError::Exception(ex) + } +} + +/// Implemented by any type that can be created from a Python object. +/// +/// Any type that implements `TryFromObject` is automatically `FromArgs`, and +/// so can be accepted as a argument to a built-in function. pub trait TryFromObject: Sized { + /// Attempt to convert a Python object to a value of this type. fn try_from_object(vm: &mut VirtualMachine, obj: PyObjectRef) -> PyResult; } +/// Implemented by any type that can be returned from a built-in Python function. +/// +/// `IntoPyObject` has a blanket implementation for any built-in object payload, +/// and should be implemented by many primitive Rust types, allowing a built-in +/// function to simply return a `bool` or a `usize` for example. pub trait IntoPyObject { fn into_pyobject(self, ctx: &PyContext) -> PyResult; } @@ -1148,12 +1260,22 @@ where } } +// This allows a built-in function to not return a value, mapping to +// Python's behavior of returning `None` in this situation. impl IntoPyObject for () { fn into_pyobject(self, ctx: &PyContext) -> PyResult { Ok(ctx.none()) } } +// TODO: Allow a built-in function to return an `Option`, i.e.: +// +// impl IntoPyObject for Option +// +// Option::None should map to a Python `None`. + +// Allows a built-in function to return any built-in object payload without +// explicitly implementing `IntoPyObject`. impl IntoPyObject for T where T: PyObjectPayload2 + Sized, @@ -1168,13 +1290,33 @@ where } } +// A tuple of types that each implement `FromArgs` represents a sequence of +// arguments that can be bound and passed to a built-in function. +// +// Technically, a tuple can contain tuples, which can contain tuples, and so on, +// so this actually represents a tree of values to be bound from arguments, but +// in practice this is only used for the top-level parameters. macro_rules! tuple_from_py_func_args { ($($T:ident),+) => { impl<$($T),+> FromArgs for ($($T,)+) where $($T: FromArgs),+ { - fn from_args(vm: &mut VirtualMachine, args: &mut iter::Peekable) -> PyResult + fn arity() -> RangeInclusive { + let mut min = 0; + let mut max = 0; + $( + let (start, end) = $T::arity().into_inner(); + min += start; + max += end; + )+ + min..=max + } + + fn from_args( + vm: &mut VirtualMachine, + args: &mut iter::Peekable + ) -> Result where I: Iterator { @@ -1184,14 +1326,32 @@ macro_rules! tuple_from_py_func_args { }; } +// Implement `FromArgs` for up to 5-tuples, allowing built-in functions to bind +// up to 5 top-level parameters (note that `Args`, `KwArgs`, nested tuples, etc. +// count as 1, so this should actually be more than enough). tuple_from_py_func_args!(A); tuple_from_py_func_args!(A, B); tuple_from_py_func_args!(A, B, C); tuple_from_py_func_args!(A, B, C, D); tuple_from_py_func_args!(A, B, C, D, E); +/// A built-in Python function. pub type PyNativeFunc = Box PyResult + 'static>; +/// Implemented by types that are or can generate built-in functions. +/// +/// For example, any function that: +/// +/// - Accepts a sequence of types that implement `FromArgs`, followed by a +/// `&mut VirtualMachine` +/// - Returns some type that implements `IntoPyObject` +/// +/// will generate a `PyNativeFunc` that performs the appropriate type and arity +/// checking, any requested conversions, and then if successful call the function +/// with the bound values. +/// +/// A bare `PyNativeFunc` also implements this trait, allowing the above to be +/// done manually, for rare situations that don't fit into this model. pub trait IntoPyNativeFunc { fn into_func(self) -> PyNativeFunc; } @@ -1211,6 +1371,10 @@ impl IntoPyNativeFunc for PyNativeFunc { } } +// This is the "magic" that allows rust functions of varying signatures to +// generate native python functions. +// +// Note that this could be done without a macro - it is simply to avoid repetition. macro_rules! into_py_native_func_tuple { ($(($n:tt, $T:ident)),+) => { impl IntoPyNativeFunc<($($T,)+), R> for F