Merge pull request #755 from skinny121/int_new_args

OddCoincidence · web-flow · commit c5c91818a561 · 2019-03-29T09:13:25.000-07:00
Use new style args for int.__new__
diff --git a/derive/src/lib.rs b/derive/src/lib.rs
@@ -3,31 +3,198 @@ extern crate proc_macro;
 use proc_macro::TokenStream;
 use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
-use syn::{Data, DeriveInput, Fields};
+use syn::{Attribute, Data, DeriveInput, Expr, Field, Fields, Ident, Lit, Meta, NestedMeta};
 
-#[proc_macro_derive(FromArgs)]
+#[proc_macro_derive(FromArgs, attributes(pyarg))]
 pub fn derive_from_args(input: TokenStream) -> TokenStream {
     let ast: DeriveInput = syn::parse(input).unwrap();
 
     let gen = impl_from_args(&ast);
     gen.to_string().parse().unwrap()
 }
 
+/// The kind of the python parameter, this corresponds to the value of Parameter.kind
+/// (https://docs.python.org/3/library/inspect.html#inspect.Parameter.kind)
+enum ParameterKind {
+    PositionalOnly,
+    PositionalOrKeyword,
+    KeywordOnly,
+}
+
+impl ParameterKind {
+    fn from_ident(ident: &Ident) -> ParameterKind {
+        if ident == "positional_only" {
+            ParameterKind::PositionalOnly
+        } else if ident == "positional_or_keyword" {
+            ParameterKind::PositionalOrKeyword
+        } else if ident == "keyword_only" {
+            ParameterKind::KeywordOnly
+        } else {
+            panic!("Unrecognised attribute")
+        }
+    }
+}
+
+struct ArgAttribute {
+    kind: ParameterKind,
+    default: Option<Expr>,
+    optional: bool,
+}
+
+impl ArgAttribute {
+    fn from_attribute(attr: &Attribute) -> Option<ArgAttribute> {
+        if !attr.path.is_ident("pyarg") {
+            return None;
+        }
+
+        match attr.parse_meta().unwrap() {
+            Meta::List(list) => {
+                let mut iter = list.nested.iter();
+                let first_arg = iter.next().expect("at least one argument in pyarg list");
+                let kind = match first_arg {
+                    NestedMeta::Meta(Meta::Word(ident)) => ParameterKind::from_ident(ident),
+                    _ => panic!("Bad syntax for first pyarg attribute argument"),
+                };
+
+                let mut attribute = ArgAttribute {
+                    kind,
+                    default: None,
+                    optional: false,
+                };
+
+                while let Some(arg) = iter.next() {
+                    attribute.parse_argument(arg);
+                }
+
+                assert!(
+                    attribute.default.is_none() || !attribute.optional,
+                    "Can't set both a default value and optional"
+                );
+
+                Some(attribute)
+            }
+            _ => panic!("Bad syntax for pyarg attribute"),
+        }
+    }
+
+    fn parse_argument(&mut self, arg: &NestedMeta) {
+        match arg {
+            NestedMeta::Meta(Meta::Word(ident)) => {
+                if ident == "default" {
+                    assert!(self.default.is_none(), "Default already set");
+                    let expr = syn::parse_str::<Expr>("Default::default()").unwrap();
+                    self.default = Some(expr);
+                } else if ident == "optional" {
+                    self.optional = true;
+                } else {
+                    panic!("Unrecognised pyarg attribute '{}'", ident);
+                }
+            }
+            NestedMeta::Meta(Meta::NameValue(name_value)) => {
+                if name_value.ident == "default" {
+                    assert!(self.default.is_none(), "Default already set");
+
+                    match name_value.lit {
+                        Lit::Str(ref val) => {
+                            let expr = val
+                                .parse::<Expr>()
+                                .expect("a valid expression for default argument");
+                            self.default = Some(expr);
+                        }
+                        _ => panic!("Expected string value for default argument"),
+                    }
+                } else if name_value.ident == "optional" {
+                    match name_value.lit {
+                        Lit::Bool(ref val) => {
+                            self.optional = val.value;
+                        }
+                        _ => panic!("Expected boolean value for optional argument"),
+                    }
+                } else {
+                    panic!("Unrecognised pyarg attribute '{}'", name_value.ident);
+                }
+            }
+            _ => panic!("Bad syntax for first pyarg attribute argument"),
+        };
+    }
+}
+
+fn generate_field(field: &Field) -> TokenStream2 {
+    let mut pyarg_attrs = field
+        .attrs
+        .iter()
+        .filter_map(ArgAttribute::from_attribute)
+        .collect::<Vec<_>>();
+    let attr = if pyarg_attrs.is_empty() {
+        ArgAttribute {
+            kind: ParameterKind::PositionalOrKeyword,
+            default: None,
+            optional: false,
+        }
+    } else if pyarg_attrs.len() == 1 {
+        pyarg_attrs.remove(0)
+    } else {
+        panic!(
+            "Multiple pyarg attributes on field '{}'",
+            field.ident.as_ref().unwrap()
+        );
+    };
+
+    let name = &field.ident;
+    let middle = quote! {
+        .map(|x| crate::pyobject::TryFromObject::try_from_object(vm, x)).transpose()?
+    };
+    let ending = if let Some(default) = attr.default {
+        quote! {
+            .unwrap_or_else(|| #default)
+        }
+    } else if attr.optional {
+        quote! {
+            .map(crate::function::OptionalArg::Present)
+            .unwrap_or(crate::function::OptionalArg::Missing)
+        }
+    } else {
+        let err = match attr.kind {
+            ParameterKind::PositionalOnly | ParameterKind::PositionalOrKeyword => quote! {
+                crate::function::ArgumentError::TooFewArgs
+            },
+            ParameterKind::KeywordOnly => quote! {
+                crate::function::ArgumentError::RequiredKeywordArgument(tringify!(#name))
+            },
+        };
+        quote! {
+            .ok_or_else(|| #err)?
+        }
+    };
+
+    match attr.kind {
+        ParameterKind::PositionalOnly => {
+            quote! {
+                #name: args.take_positional()#middle#ending,
+            }
+        }
+        ParameterKind::PositionalOrKeyword => {
+            quote! {
+                #name: args.take_positional_keyword(stringify!(#name))#middle#ending,
+            }
+        }
+        ParameterKind::KeywordOnly => {
+            quote! {
+                #name: args.take_keyword(stringify!(#name))#middle#ending,
+            }
+        }
+    }
+}
+
 fn impl_from_args(input: &DeriveInput) -> TokenStream2 {
     // FIXME: This references types using `crate` instead of `rustpython_vm`
     //        so that it can be used in the latter. How can we support both?
+    //        Can use extern crate self as rustpython_vm; once in stable.
+    //        https://github.com/rust-lang/rust/issues/56409
     let fields = match input.data {
         Data::Struct(ref data) => {
             match data.fields {
-                Fields::Named(ref fields) => fields.named.iter().map(|field| {
-                    let name = &field.ident;
-                    quote! {
-                        #name: crate::pyobject::TryFromObject::try_from_object(
-                            vm,
-                            args.take_keyword(stringify!(#name)).unwrap_or_else(|| vm.ctx.none())
-                        )?,
-                    }
-                }),
+                Fields::Named(ref fields) => fields.named.iter().map(generate_field),
                 Fields::Unnamed(_) | Fields::Unit => unimplemented!(), // TODO: better error message
             }
         }
diff --git a/tests/snippets/ints.py b/tests/snippets/ints.py
@@ -1,4 +1,4 @@
-from testutils import assert_raises
+from testutils import assert_raises, assertRaises
 
 # int to int comparisons
 
@@ -58,3 +58,19 @@
 assert (2).__rmul__(1.0) == NotImplemented
 assert (2).__truediv__(1.0) == NotImplemented
 assert (2).__rtruediv__(1.0) == NotImplemented
+
+
+assert int() == 0
+assert int("101", 2) == 5
+assert int("101", base=2) == 5
+assert int(1) == 1
+
+with assertRaises(TypeError):
+    int(base=2)
+
+with assertRaises(TypeError):
+    int(1, base=2)
+
+with assertRaises(TypeError):
+    # check that first parameter is truly positional only
+    int(val_options=1)
diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs
@@ -537,8 +537,11 @@ fn builtin_pow(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
 
 #[derive(Debug, FromArgs)]
 pub struct PrintOptions {
+    #[pyarg(keyword_only, default = "None")]
     sep: Option<PyStringRef>,
+    #[pyarg(keyword_only, default = "None")]
     end: Option<PyStringRef>,
+    #[pyarg(keyword_only, default = "false")]
     flush: bool,
 }
 
diff --git a/vm/src/function.rs b/vm/src/function.rs
@@ -1,12 +1,11 @@
 use std::collections::HashMap;
 use std::ops::RangeInclusive;
 
-use crate::obj::objtype;
+use crate::obj::objtype::{isinstance, PyClassRef};
 use crate::pyobject::{IntoPyObject, PyObjectRef, PyResult, TryFromObject, TypeProtocol};
 use crate::vm::VirtualMachine;
 
 use self::OptionalArg::*;
-use crate::obj::objtype::PyClassRef;
 
 /// The `PyFuncArgs` struct is one of the most used structs then creating
 /// a rust function that can be called from python. It holds both positional
@@ -95,7 +94,7 @@ impl PyFuncArgs {
     ) -> Result<Option<PyObjectRef>, PyObjectRef> {
         match self.get_optional_kwarg(key) {
             Some(kwarg) => {
-                if objtype::isinstance(&kwarg, &ty) {
+                if isinstance(&kwarg, &ty) {
                     Ok(Some(kwarg))
                 } else {
                     let expected_ty_name = vm.to_pystr(&ty)?;
@@ -110,14 +109,18 @@ impl PyFuncArgs {
         }
     }
 
-    pub fn next_positional(&mut self) -> Option<PyObjectRef> {
+    pub fn take_positional(&mut self) -> Option<PyObjectRef> {
         if self.args.is_empty() {
             None
         } else {
             Some(self.args.remove(0))
         }
     }
 
+    pub fn take_positional_keyword(&mut self, name: &str) -> Option<PyObjectRef> {
+        self.take_positional().or_else(|| self.take_keyword(name))
+    }
+
     pub fn take_keyword(&mut self, name: &str) -> Option<PyObjectRef> {
         // TODO: change kwarg representation so this scan isn't necessary
         if let Some(index) = self
@@ -164,6 +167,9 @@ impl PyFuncArgs {
             Err(ArgumentError::InvalidKeywordArgument(name)) => {
                 return Err(vm.new_type_error(format!("{} is an invalid keyword argument", name)));
             }
+            Err(ArgumentError::RequiredKeywordArgument(name)) => {
+                return Err(vm.new_type_error(format!("Required keyqord only argument {}", name)));
+            }
             Err(ArgumentError::Exception(ex)) => {
                 return Err(ex);
             }
@@ -192,6 +198,8 @@ pub enum ArgumentError {
     TooManyArgs,
     /// The function doesn't accept a keyword argument with the given name.
     InvalidKeywordArgument(String),
+    /// The function require a keyword argument with the given name, but one wasn't provided
+    RequiredKeywordArgument(String),
     /// An exception was raised while binding arguments to the function
     /// parameters.
     Exception(PyObjectRef),
@@ -272,7 +280,7 @@ where
 {
     fn from_args(vm: &VirtualMachine, args: &mut PyFuncArgs) -> Result<Self, ArgumentError> {
         let mut varargs = Vec::new();
-        while let Some(value) = args.next_positional() {
+        while let Some(value) = args.take_positional() {
             varargs.push(T::try_from_object(vm, value)?);
         }
         Ok(Args(varargs))
@@ -297,7 +305,7 @@ where
     }
 
     fn from_args(vm: &VirtualMachine, args: &mut PyFuncArgs) -> Result<Self, ArgumentError> {
-        if let Some(value) = args.next_positional() {
+        if let Some(value) = args.take_positional() {
             Ok(T::try_from_object(vm, value)?)
         } else {
             Err(ArgumentError::TooFewArgs)
@@ -308,6 +316,7 @@ where
 /// An argument that may or may not be provided by the caller.
 ///
 /// This style of argument is not possible in pure Python.
+#[derive(Debug)]
 pub enum OptionalArg<T = PyObjectRef> {
     Present(T),
     Missing,
@@ -340,7 +349,7 @@ where
     }
 
     fn from_args(vm: &VirtualMachine, args: &mut PyFuncArgs) -> Result<Self, ArgumentError> {
-        if let Some(value) = args.next_positional() {
+        if let Some(value) = args.take_positional() {
             Ok(Present(T::try_from_object(vm, value)?))
         } else {
             Ok(Missing)
diff --git a/vm/src/obj/objint.rs b/vm/src/obj/objint.rs
