From 1d18398c660047b2c1306511a218dc059437002f Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Feb 2021 10:09:42 -0600 Subject: [PATCH 1/5] Handle splat args in bases/kwargs for __build_class__ --- compiler/src/compile.rs | 180 +++++++++++++++++++--------------------- 1 file changed, 87 insertions(+), 93 deletions(-) diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 4775dc183..626bec72e 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -1158,6 +1158,8 @@ impl Compiler { ) -> CompileResult<()> { self.prepare_decorators(decorator_list)?; + self.emit(Instruction::LoadBuildClass); + let prev_ctx = self.ctx; self.ctx = CompileContext { func: FunctionContext::NoFunction, @@ -1173,7 +1175,6 @@ impl Compiler { Some(qualified_name.clone()), ); - self.emit(Instruction::LoadBuildClass); self.push_output(bytecode::CodeFlags::empty(), 0, 0, 0, name.to_owned()); let (new_body, doc_str) = get_doc(body); @@ -1241,35 +1242,7 @@ impl Compiler { value: qualified_name, }); - for base in bases { - self.compile_expression(base)?; - } - - if !keywords.is_empty() { - let mut kwarg_names = vec![]; - for keyword in keywords { - if let Some(name) = &keyword.node.arg { - kwarg_names.push(ConstantData::Str { - value: name.to_owned(), - }); - } else { - // This means **kwargs! - panic!("name must be set"); - } - self.compile_expression(&keyword.node.value)?; - } - - self.emit_constant(ConstantData::Tuple { - elements: kwarg_names, - }); - self.emit(Instruction::CallFunctionKeyword { - nargs: (2 + keywords.len() + bases.len()) as u32, - }); - } else { - self.emit(Instruction::CallFunctionPositional { - nargs: (2 + bases.len()) as u32, - }); - } + self.compile_call(2, bases, keywords)?; self.apply_decorators(decorator_list); @@ -1821,7 +1794,10 @@ impl Compiler { func, args, keywords, - } => self.compile_call(func, args, keywords)?, + } => { + self.compile_expression(func)?; + self.compile_call(0, args, keywords)?; + } BoolOp { op, values } => self.compile_bool_op(op, values)?, BinOp { left, op, right } => { self.compile_expression(left)?; @@ -1864,28 +1840,16 @@ impl Compiler { self.emit_constant(compile_constant(value)); } List { elts, .. } => { - let size = elts.len() as u32; - let must_unpack = self.gather_elements(elts)?; - self.emit(Instruction::BuildList { - size, - unpack: must_unpack, - }); + let (size, unpack) = self.gather_elements(0, elts)?; + self.emit(Instruction::BuildList { size, unpack }); } Tuple { elts, .. } => { - let size = elts.len() as u32; - let must_unpack = self.gather_elements(elts)?; - self.emit(Instruction::BuildTuple { - size, - unpack: must_unpack, - }); + let (size, unpack) = self.gather_elements(0, elts)?; + self.emit(Instruction::BuildTuple { size, unpack }); } Set { elts, .. } => { - let size = elts.len() as u32; - let must_unpack = self.gather_elements(elts)?; - self.emit(Instruction::BuildSet { - size, - unpack: must_unpack, - }); + let (size, unpack) = self.gather_elements(0, elts)?; + self.emit(Instruction::BuildSet { size, unpack }); } Dict { keys, values } => { self.compile_dict(keys, values)?; @@ -2139,81 +2103,111 @@ impl Compiler { fn compile_call( &mut self, - function: &ast::Expr, + additional_positional: u32, args: &[ast::Expr], keywords: &[ast::Keyword], ) -> CompileResult<()> { - self.compile_expression(function)?; - let count = (args.len() + keywords.len()) as u32; + let count = (args.len() + keywords.len()) as u32 + additional_positional; // Normal arguments: - let must_unpack = self.gather_elements(args)?; + let (size, unpack) = self.gather_elements(additional_positional, args)?; let has_double_star = keywords.iter().any(|k| k.node.arg.is_none()); - if must_unpack || has_double_star { + if unpack || has_double_star { // Create a tuple with positional args: - self.emit(Instruction::BuildTuple { - size: args.len() as u32, - unpack: must_unpack, - }); + self.emit(Instruction::BuildTuple { size, unpack }); // Create an optional map with kw-args: - if !keywords.is_empty() { + let has_kwargs = !keywords.is_empty(); + if has_kwargs { self.compile_keywords(keywords)?; - self.emit(Instruction::CallFunctionEx { has_kwargs: true }); - } else { - self.emit(Instruction::CallFunctionEx { has_kwargs: false }); } - } else { - // Keyword arguments: - if !keywords.is_empty() { - let mut kwarg_names = vec![]; - for keyword in keywords { - if let Some(name) = &keyword.node.arg { - kwarg_names.push(ConstantData::Str { - value: name.to_owned(), - }); - } else { - // This means **kwargs! - panic!("name must be set"); - } - self.compile_expression(&keyword.node.value)?; + self.emit(Instruction::CallFunctionEx { has_kwargs }); + } else if !keywords.is_empty() { + let mut kwarg_names = vec![]; + for keyword in keywords { + if let Some(name) = &keyword.node.arg { + kwarg_names.push(ConstantData::Str { + value: name.to_owned(), + }); + } else { + // This means **kwargs! + panic!("name must be set"); } - - self.emit_constant(ConstantData::Tuple { - elements: kwarg_names, - }); - self.emit(Instruction::CallFunctionKeyword { nargs: count }); - } else { - self.emit(Instruction::CallFunctionPositional { nargs: count }); + self.compile_expression(&keyword.node.value)?; } + + self.emit_constant(ConstantData::Tuple { + elements: kwarg_names, + }); + self.emit(Instruction::CallFunctionKeyword { nargs: count }); + } else { + self.emit(Instruction::CallFunctionPositional { nargs: count }); } + Ok(()) } // Given a vector of expr / star expr generate code which gives either // a list of expressions on the stack, or a list of tuples. - fn gather_elements(&mut self, elements: &[ast::Expr]) -> CompileResult { + fn gather_elements( + &mut self, + before: u32, + elements: &[ast::Expr], + ) -> CompileResult<(u32, bool)> { // First determine if we have starred elements: let has_stars = elements .iter() .any(|e| matches!(e.node, ast::ExprKind::Starred { .. })); - for element in elements { - if let ast::ExprKind::Starred { value, .. } = &element.node { - self.compile_expression(value)?; - } else { - self.compile_expression(element)?; - if has_stars { + let size = if has_stars { + let mut size = 0; + + if before > 0 { + self.emit(Instruction::BuildTuple { + size: before, + unpack: false, + }); + size += 1; + } + + let groups = elements + .iter() + .map(|element| { + if let ast::ExprKind::Starred { value, .. } = &element.node { + (true, value.as_ref()) + } else { + (false, element) + } + }) + .group_by(|(starred, _)| *starred); + + for (starred, run) in &groups { + let mut run_size = 0; + for (_, value) in run { + self.compile_expression(value)?; + run_size += 1 + } + if starred { + size += run_size + } else { self.emit(Instruction::BuildTuple { - size: 1, + size: run_size, unpack: false, }); + size += 1 } } - } - Ok(has_stars) + size + } else { + for element in elements { + self.compile_expression(element)?; + } + before + elements.len() as u32 + }; + + Ok((size, has_stars)) } fn compile_comprehension_element(&mut self, element: &ast::Expr) -> CompileResult<()> { From 8f68959a984d2abb21266c0adfb9bb0875931a5a Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Feb 2021 10:12:40 -0600 Subject: [PATCH 2/5] Add snippet test --- extra_tests/snippets/class.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/extra_tests/snippets/class.py b/extra_tests/snippets/class.py index 227051253..829ebe10b 100644 --- a/extra_tests/snippets/class.py +++ b/extra_tests/snippets/class.py @@ -258,4 +258,10 @@ else: class A(): a: int -assert A.__annotations__['a'] == int \ No newline at end of file +assert A.__annotations__['a'] == int + +class A: pass +class B: pass + +class C(*(A, B), **{"metaclass": type}): + pass From b191c68b9c4fbce8eae1ffc9f878110d7b22e57d Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Feb 2021 10:16:22 -0600 Subject: [PATCH 3/5] Pass kwargs to __prepare__ --- vm/src/builtins/make_module.rs | 5 ++++- vm/src/function.rs | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index cf6c4129e..c415be762 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -857,7 +857,10 @@ mod decl { // Prepare uses full __getattribute__ resolution chain. let prepare = vm.get_attribute(metaclass.clone().into_object(), "__prepare__")?; - let namespace = vm.invoke(&prepare, vec![name_obj.clone(), bases.clone()])?; + let namespace = vm.invoke( + &prepare, + FuncArgs::new(vec![name_obj.clone(), bases.clone()], kwargs.clone()), + )?; let namespace = PyDictRef::try_from_object(vm, namespace)?; diff --git a/vm/src/function.rs b/vm/src/function.rs index bad40909a..6428fd18d 100644 --- a/vm/src/function.rs +++ b/vm/src/function.rs @@ -313,6 +313,7 @@ impl FromArgOptional for T { /// KwArgs is only for functions that accept arbitrary keyword arguments. For /// functions that accept only *specific* named arguments, a rust struct with /// an appropriate FromArgs implementation must be created. +#[derive(Clone)] pub struct KwArgs(IndexMap); impl KwArgs { From 0bdc4bc8d8715a4365a575b89f609d8a3d26e454 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Feb 2021 18:29:54 -0600 Subject: [PATCH 4/5] Accept kwargs in type.__prepare__ --- vm/src/builtins/pytype.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vm/src/builtins/pytype.rs b/vm/src/builtins/pytype.rs index 8d2b6e358..20c045d83 100644 --- a/vm/src/builtins/pytype.rs +++ b/vm/src/builtins/pytype.rs @@ -330,7 +330,12 @@ impl PyType { } #[pymethod(magic)] - fn prepare(_name: PyStrRef, _bases: PyObjectRef, vm: &VirtualMachine) -> PyDictRef { + fn prepare( + _name: PyStrRef, + _bases: PyObjectRef, + _kwargs: KwArgs, + vm: &VirtualMachine, + ) -> PyDictRef { vm.ctx.new_dict() } From db282446f06ed96fbe945aa9e2b70b4724d25059 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Wed, 3 Feb 2021 18:59:04 -0600 Subject: [PATCH 5/5] Unskip/fix tests in test_subclassinit --- Lib/test/test_subclassinit.py | 4 ---- vm/src/builtins/pytype.rs | 40 +++++++++++++++++++++-------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/Lib/test/test_subclassinit.py b/Lib/test/test_subclassinit.py index 623790e4e..c007476e0 100644 --- a/Lib/test/test_subclassinit.py +++ b/Lib/test/test_subclassinit.py @@ -197,8 +197,6 @@ class Test(unittest.TestCase): self.assertIs(B.meta_owner, B) self.assertEqual(B.name, 'd') - #TODO: RUSTPYTHON - @unittest.skip("infinite loops") def test_set_name_modifying_dict(self): notified = [] class Descriptor: @@ -215,8 +213,6 @@ class Test(unittest.TestCase): self.assertCountEqual(notified, ['a', 'b', 'c', 'd', 'e']) - #TODO: RUSTPYTHON - @unittest.expectedFailure def test_errors(self): class MyMeta(type): pass diff --git a/vm/src/builtins/pytype.rs b/vm/src/builtins/pytype.rs index 20c045d83..a335901f4 100644 --- a/vm/src/builtins/pytype.rs +++ b/vm/src/builtins/pytype.rs @@ -11,7 +11,7 @@ use super::int::PyInt; use super::list::PyList; use super::mappingproxy::PyMappingProxy; use super::object; -use super::pystr::PyStrRef; +use super::pystr::{PyStr, PyStrRef}; use super::staticmethod::PyStaticMethod; use super::tuple::PyTuple; use super::weakref::PyWeak; @@ -496,21 +496,29 @@ impl PyType { vm.ctx.add_tp_new_wrapper(&typ); - for (name, obj) in typ.attributes.read().clone().iter() { - if let Some(meth) = vm.get_method(obj.clone(), "__set_name__") { - let set_name = meth?; - vm.invoke(&set_name, (typ.clone(), name.clone())) - .map_err(|e| { - let err = vm.new_runtime_error(format!( - "Error calling __set_name__ on '{}' instance {} in '{}'", - obj.class().name, - name, - typ.name - )); - err.set_cause(Some(e)); - err - })?; - } + // avoid deadlock + let attributes = typ + .attributes + .read() + .iter() + .filter_map(|(name, obj)| { + vm.get_method(obj.clone(), "__set_name__").map(|res| { + res.map(|meth| (obj.clone(), PyStr::from(name.clone()).into_ref(vm), meth)) + }) + }) + .collect::>>()?; + for (obj, name, set_name) in attributes { + vm.invoke(&set_name, (typ.clone(), name.clone())) + .map_err(|e| { + let err = vm.new_runtime_error(format!( + "Error calling __set_name__ on '{}' instance {} in '{}'", + obj.class().name, + name, + typ.name + )); + err.set_cause(Some(e)); + err + })?; } if let Some(initter) = typ.get_super_attr("__init_subclass__") {