windelbouwman · Aug 29, 2018 · Aug 28, 2018 · Aug 28, 2018 · Aug 28, 2018 · Aug 28, 2018
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/parser/src/ast.rs b/parser/src/ast.rs
@@ -95,12 +95,12 @@ pub enum Statement {
     ClassDef {
         name: String,
         body: Vec<LocatedStatement>,
-        args: Vec<String>,
+        args: Vec<(String, Option<Expression>)>,
         // TODO: docstring: String,
     },
     FunctionDef {
         name: String,
-        args: Vec<String>,
+        args: Vec<(String, Option<Expression>)>,
         // docstring: String,
         body: Vec<LocatedStatement>,
     },
@@ -161,7 +161,7 @@ pub enum Expression {
         name: String,
     },
     Lambda {
-        args: Vec<String>,
+        args: Vec<(String, Option<Expression>)>,
         body: Box<Expression>,
     },
     True,

diff --git a/parser/src/parser.rs b/parser/src/parser.rs
@@ -193,7 +193,7 @@ mod tests {
                 location: ast::Location::new(1, 1),
                 node: ast::Statement::Expression {
                     expression: ast::Expression::Lambda {
-                        args: vec![String::from("x"), String::from("y")],
+                        args: vec![(String::from("x"), None), (String::from("y"), None)],
                         body: Box::new(ast::Expression::Binop {
                             a: Box::new(ast::Expression::Identifier {
                                 name: String::from("x"),
@@ -211,25 +211,46 @@ mod tests {
 
     #[test]
     fn test_parse_class() {
-        let source = String::from("class Foo(A, B):\n def __init__(self):\n  pass\n");
+        let source = String::from("class Foo(A, B):\n def __init__(self):\n  pass\n def method_with_default(self, arg='default'):\n  pass\n");
         assert_eq!(
             parse_statement(&source),
             Ok(ast::LocatedStatement {
                 location: ast::Location::new(1, 1),
                 node: ast::Statement::ClassDef {
                     name: String::from("Foo"),
-                    args: vec![String::from("A"), String::from("B")],
-                    body: vec![ast::LocatedStatement {
-                        location: ast::Location::new(2, 2),
-                        node: ast::Statement::FunctionDef {
-                            name: String::from("__init__"),
-                            args: vec![String::from("self")],
-                            body: vec![ast::LocatedStatement {
-                                location: ast::Location::new(3, 3),
-                                node: ast::Statement::Pass,
-                            }],
+                    args: vec![(String::from("A"), None), (String::from("B"), None)],
+                    body: vec![
+                        ast::LocatedStatement {
+                            location: ast::Location::new(2, 2),
+                            node: ast::Statement::FunctionDef {
+                                name: String::from("__init__"),
+                                args: vec![(String::from("self"), None)],
+                                body: vec![ast::LocatedStatement {
+                                    location: ast::Location::new(3, 3),
+                                    node: ast::Statement::Pass,
+                                }],
+                            }
+                        },
+                        ast::LocatedStatement {
+                            location: ast::Location::new(4, 2),
+                            node: ast::Statement::FunctionDef {
+                                name: String::from("method_with_default"),
+                                args: vec![
+                                    (String::from("self"), None),
+                                    (
+                                        String::from("arg"),
+                                        Some(ast::Expression::String {
+                                            value: "default".to_string()
+                                        })
+                                    )
+                                ],
+                                body: vec![ast::LocatedStatement {
+                                    location: ast::Location::new(5, 3),
+                                    node: ast::Statement::Pass,
+                                }],
+                            }
                         }
-                    }],
+                    ],
                 }
             })
         )

diff --git a/parser/src/python.lalrpop b/parser/src/python.lalrpop
@@ -308,12 +308,19 @@ FuncDef: ast::LocatedStatement = {
   },
 };
 
-Parameters: Vec<String> = {
+Parameters: Vec<(String, Option<ast::Expression>)> = {
   "(" <a: TypedArgsList> ")" => a,
 };
 
-TypedArgsList: Vec<String> = {
-  <a: Comma<Identifier>> => a,
+// parameters are (String, None), kwargs are (String, Some(Test)) where Test is
+// the default
+TypedArgsList: Vec<(String, Option<ast::Expression>)> = {
+  <a: Comma<Parameter>> => a
+};
+
+Parameter: (String, Option<ast::Expression>) = {
+    <i:Identifier> => (i.clone(), None),
+    <i:Identifier> "=" <e:Expression> => (i.clone(), Some(e.clone())),
 };
 
 ClassDef: ast::LocatedStatement = {

diff --git a/tests/snippets/func_defaults.py b/tests/snippets/func_defaults.py
@@ -0,0 +1,66 @@
+def no_args():
+    pass
+
+no_args()
+
+try:
+    no_args('one_arg')
+except TypeError:
+    pass
+else:
+    assert False, 'no TypeError raised: 1 arg to no_args'
+
+
+def one_arg(arg):
+    pass
+
+one_arg('one_arg')
+
+try:
+    one_arg()
+except TypeError:
+    pass
+else:
+    assert False, 'no TypeError raised: no args to one_arg'
+
+try:
+    one_arg('one_arg', 'two_arg')
+except TypeError:
+    pass
+else:
+    assert False, 'no TypeError raised: two args to one_arg'
+
+
+def one_default_arg(arg="default"):
+    return arg
+
+assert 'default' == one_default_arg()
+assert 'arg' == one_default_arg('arg')
+
+try:
+    one_default_arg('one_arg', 'two_arg')
+except TypeError:
+    pass
+else:
+    assert False, 'no TypeError raised: two args to one_default_arg'
+
+
+def one_normal_one_default_arg(pos, arg="default"):
+    return pos, arg
+
+assert ('arg', 'default') == one_normal_one_default_arg('arg')
+assert ('arg', 'arg2') == one_normal_one_default_arg('arg', 'arg2')
+
+try:
+    one_normal_one_default_arg()
+except TypeError:
+    pass
+else:
+    assert False, 'no TypeError raised: two args to one_default_arg'
+
+try:
+    one_normal_one_default_arg('one', 'two', 'three')
+except TypeError:
+    pass
+else:
+    assert False, 'no TypeError raised: two args to one_default_arg'
diff --git a/vm/Cargo.toml b/vm/Cargo.toml
@@ -4,6 +4,7 @@ version = "0.1.0"
 authors = ["Shing Lyu <[email protected]>"]
 
 [dependencies]
+bitflags = "1.0.4"
 log = "0.3"
 rustpython_parser = {path = "../parser"}
 serde = "1.0.66"

diff --git a/vm/src/bytecode.rs b/vm/src/bytecode.rs
@@ -37,6 +37,12 @@ impl CodeObject {
     }
 }
 
+bitflags! {
+    pub struct FunctionOpArg: u8 {
+        const HAS_DEFAULTS = 0x01;
+    }
+}
+
 pub type Label = usize;
 
 #[derive(Debug, Clone, PartialEq)]
@@ -88,7 +94,9 @@ pub enum Instruction {
     JumpIfFalse {
         target: Label,
     },
-    MakeFunction,
+    MakeFunction {
+        flags: FunctionOpArg,
+    },
     CallFunction {
         count: usize,
     },

diff --git a/vm/src/compile.rs b/vm/src/compile.rs
@@ -350,8 +350,28 @@ impl Compiler {
             }
             ast::Statement::FunctionDef { name, args, body } => {
                 // Create bytecode for this function:
-                self.code_object_stack
-                    .push(CodeObject::new(args.to_vec(), None));
+                let mut names = vec![];
+                let mut default_elements = vec![];
+
+                for (name, default) in args {
+                    names.push(name.clone());
+                    if let Some(default) = default {
+                        default_elements.push(default.clone());
+                    } else {
+                        if default_elements.len() > 0 {
+                            // Once we have started with defaults, all remaining arguments must
+                            // have defaults
+                            panic!("non-default argument follows default argument: {}", name);
+                        }
+                    }
+                }
+
+                let have_kwargs = default_elements.len() > 0;
+                self.compile_expression(&ast::Expression::Tuple {
+                    elements: default_elements,
+                });
+
+                self.code_object_stack.push(CodeObject::new(names, None));
                 self.compile_statements(body);
 
                 // Emit None at end:
@@ -371,7 +391,11 @@ impl Compiler {
                 });
 
                 // Turn code object into function object:
-                self.emit(Instruction::MakeFunction);
+                let mut flags = bytecode::FunctionOpArg::empty();
+                if have_kwargs {
+                    flags = flags | bytecode::FunctionOpArg::HAS_DEFAULTS;
+                }
+                self.emit(Instruction::MakeFunction { flags: flags });
                 self.emit(Instruction::StoreName {
                     name: name.to_string(),
                 });
@@ -400,7 +424,9 @@ impl Compiler {
                     },
                 });
                 // Turn code object into function object:
-                self.emit(Instruction::MakeFunction);
+                self.emit(Instruction::MakeFunction {
+                    flags: bytecode::FunctionOpArg::empty(),
+                });
 
                 self.emit(Instruction::LoadConst {
                     value: bytecode::Constant::String {
@@ -409,7 +435,9 @@ impl Compiler {
                 });
 
                 for base in args {
-                    self.emit(Instruction::LoadName { name: base.clone() });
+                    self.emit(Instruction::LoadName {
+                        name: base.0.clone(),
+                    });
                 }
                 self.emit(Instruction::CallFunction {
                     count: 2 + args.len(),
@@ -720,8 +748,10 @@ impl Compiler {
                 });
             }
             ast::Expression::Lambda { args, body } => {
-                self.code_object_stack
-                    .push(CodeObject::new(args.to_vec(), None));
+                self.code_object_stack.push(CodeObject::new(
+                    args.iter().map(|(name, _default)| name.clone()).collect(),
+                    None,
+                ));
                 self.compile_expression(body);
                 self.emit(Instruction::ReturnValue);
                 let code = self.code_object_stack.pop().unwrap();
@@ -734,7 +764,9 @@ impl Compiler {
                     },
                 });
                 // Turn code object into function object:
-                self.emit(Instruction::MakeFunction);
+                self.emit(Instruction::MakeFunction {
+                    flags: bytecode::FunctionOpArg::empty(),
+                });
             }
         }
     }

diff --git a/vm/src/lib.rs b/vm/src/lib.rs
@@ -1,4 +1,6 @@
 #[macro_use]
+extern crate bitflags;
+#[macro_use]
 extern crate log;
 // extern crate env_logger;
 extern crate serde;

diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs
@@ -267,11 +267,17 @@ impl PyContext {
         )
     }
 
-    pub fn new_function(&self, code_obj: PyObjectRef, scope: PyObjectRef) -> PyObjectRef {
+    pub fn new_function(
+        &self,
+        code_obj: PyObjectRef,
+        scope: PyObjectRef,
+        defaults: PyObjectRef,
+    ) -> PyObjectRef {
         PyObject::new(
             PyObjectKind::Function {
                 code: code_obj,
                 scope: scope,
+                defaults: defaults,
             },
             self.function_type(),
         )
@@ -566,6 +572,7 @@ pub enum PyObjectKind {
     Function {
         code: PyObjectRef,
         scope: PyObjectRef,
+        defaults: PyObjectRef,
     },
     BoundMethod {
         function: PyObjectRef,
@@ -613,7 +620,7 @@ impl fmt::Debug for PyObjectKind {
             } => write!(f, "slice"),
             &PyObjectKind::NameError { name: _ } => write!(f, "NameError"),
             &PyObjectKind::Code { ref code } => write!(f, "code: {:?}", code),
-            &PyObjectKind::Function { code: _, scope: _ } => write!(f, "function"),
+            &PyObjectKind::Function { .. } => write!(f, "function"),
             &PyObjectKind::BoundMethod {
                 ref function,
                 ref object,
@@ -683,7 +690,7 @@ impl PyObject {
             } => format!("<class '{}'>", name),
             PyObjectKind::Instance { dict: _ } => format!("<instance>"),
             PyObjectKind::Code { code: _ } => format!("<code>"),
-            PyObjectKind::Function { code: _, scope: _ } => format!("<func>"),
+            PyObjectKind::Function { .. } => format!("<func>"),
             PyObjectKind::BoundMethod { .. } => format!("<bound-method>"),
             PyObjectKind::RustFunction { function: _ } => format!("<rustfunc>"),
             PyObjectKind::Module { ref name, dict: _ } => format!("<module '{}'>", name),