From 2c29a33d009d2210b011d414372fea5c9d2ff72c Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Fri, 27 Nov 2020 17:43:03 -0600 Subject: [PATCH 01/14] Add support for deleter on getsets --- derive/src/pyclass.rs | 119 ++++++++++++++++++------- vm/src/builtins/getset.rs | 182 ++++++++++++++++++++++++++++---------- vm/src/pyobject.rs | 4 +- 3 files changed, 223 insertions(+), 82 deletions(-) diff --git a/derive/src/pyclass.rs b/derive/src/pyclass.rs index e8aa209bf6..793b775893 100644 --- a/derive/src/pyclass.rs +++ b/derive/src/pyclass.rs @@ -354,11 +354,10 @@ where let item_attr = args.attrs.remove(self.index()); let item_meta = PropertyItemMeta::from_attr(ident.clone(), &item_attr)?; - let py_name = item_meta.property_name()?; - let setter = item_meta.setter()?; + let (py_name, kind) = item_meta.property_name()?; args.context .getset_items - .add_item(py_name, args.cfgs.to_vec(), setter, ident.clone())?; + .add_item(py_name, args.cfgs.to_vec(), kind, ident.clone())?; Ok(()) } } @@ -478,27 +477,37 @@ where #[derive(Default)] #[allow(clippy::type_complexity)] struct GetSetNursery { - map: HashMap<(String, Vec), (Option, Option)>, + map: HashMap<(String, Vec), (Option, Option, Option)>, validated: bool, } +enum GetSetItemKind { + Get, + Set, + Delete, +} + impl GetSetNursery { fn add_item( &mut self, name: String, cfgs: Vec, - setter: bool, + kind: GetSetItemKind, item_ident: Ident, ) -> Result<()> { assert!(!self.validated, "new item is not allowed after validation"); - if setter && !cfgs.is_empty() { + if !matches!(kind, GetSetItemKind::Get) && !cfgs.is_empty() { return Err(syn::Error::new_spanned( item_ident, - "Property setter does not allow #[cfg]", + "Only the getter can have #[cfg]", )); } let entry = self.map.entry((name.clone(), cfgs)).or_default(); - let func = if setter { &mut entry.1 } else { &mut entry.0 }; + let func = match kind { + GetSetItemKind::Get => &mut entry.0, + GetSetItemKind::Set => &mut entry.1, + GetSetItemKind::Delete => &mut entry.2, + }; if func.is_some() { return Err(syn::Error::new_spanned( item_ident, @@ -511,10 +520,10 @@ impl GetSetNursery { fn validate(&mut self) -> Result<()> { let mut errors = Vec::new(); - for ((name, _cfgs), (getter, setter)) in self.map.iter() { + for ((name, _cfgs), (getter, setter, deleter)) in self.map.iter() { if getter.is_none() { errors.push(syn::Error::new_spanned( - setter.as_ref().unwrap(), + setter.as_ref().or_else(|| deleter.as_ref()).unwrap(), format!("Property '{}' is missing a getter", name), )); }; @@ -528,21 +537,33 @@ impl GetSetNursery { impl ToTokens for GetSetNursery { fn to_tokens(&self, tokens: &mut TokenStream) { assert!(self.validated, "Call `validate()` before token generation"); - tokens.extend(self.map.iter().map(|((name, cfgs), (getter, setter))| { - let (constructor, setter) = match setter { - Some(setter) => (quote! { with_get_set }, quote_spanned! { setter.span() => , &Self::#setter }), - None => (quote! { with_get }, quote! { }), - }; - quote! { - #( #cfgs )* - class.set_str_attr( - #name, - ::rustpython_vm::pyobject::PyObject::new( - ::rustpython_vm::builtins::PyGetSet::#constructor(#name.into(), &Self::#getter #setter), - ctx.types.getset_type.clone(), None) - ); - } - })); + let properties = self + .map + .iter() + .map(|((name, cfgs), (getter, setter, deleter))| { + let setter = match setter { + Some(setter) => quote_spanned! { setter.span()=> .with_set(&Self::#setter)}, + None => quote! {}, + }; + let deleter = match deleter { + Some(deleter) => { + quote_spanned! { deleter.span()=> .with_delete(&Self::#deleter)} + } + None => quote! {}, + }; + quote! { + #( #cfgs )* + class.set_str_attr( + #name, + ::rustpython_vm::pyobject::PyObject::new( + ::rustpython_vm::builtins::PyGetSet::new(#name.into()) + .with_get(&Self::#getter) + #setter #deleter, + ctx.types.getset_type.clone(), None) + ); + } + }); + tokens.extend(properties); } } @@ -580,7 +601,7 @@ impl MethodItemMeta { struct PropertyItemMeta(ItemMetaInner); impl ItemMeta for PropertyItemMeta { - const ALLOWED_NAMES: &'static [&'static str] = &["name", "magic", "setter"]; + const ALLOWED_NAMES: &'static [&'static str] = &["name", "magic", "setter", "deleter"]; fn from_inner(inner: ItemMetaInner) -> Self { Self(inner) @@ -591,10 +612,25 @@ impl ItemMeta for PropertyItemMeta { } impl PropertyItemMeta { - fn property_name(&self) -> Result { + fn property_name(&self) -> Result<(String, GetSetItemKind)> { let inner = self.inner(); let magic = inner._bool("magic")?; let setter = inner._bool("setter")?; + let deleter = inner._bool("deleter")?; + let kind = match (setter, deleter) { + (false, false) => GetSetItemKind::Get, + (true, false) => GetSetItemKind::Set, + (false, true) => GetSetItemKind::Delete, + (true, true) => { + return Err(syn::Error::new_spanned( + &inner.meta_ident, + format!( + "can't have both setter and deleter on a #[{}] fn", + inner.meta_name() + ), + )) + } + }; let name = inner._optional_str("name")?; let py_name = if let Some(name) = name { name @@ -623,6 +659,29 @@ impl PropertyItemMeta { ), )); } + } else if deleter { + if let Some(name) = sig_name.strip_prefix("del_") { + if name.is_empty() { + return Err(syn::Error::new_spanned( + &inner.meta_ident, + format!( + "A #[{}(deleter)] fn with a del_* name must \ + have something after \"del_\"", + inner.meta_name() + ), + )); + } + name.to_string() + } else { + return Err(syn::Error::new_spanned( + &inner.meta_ident, + format!( + "A #[{}(deleter)] fn must either have a `name` \ + parameter or a fn name along the lines of \"del_*\"", + inner.meta_name() + ), + )); + } } else { sig_name }; @@ -632,11 +691,7 @@ impl PropertyItemMeta { name } }; - Ok(py_name) - } - - fn setter(&self) -> Result { - self.inner()._bool("setter") + Ok((py_name, kind)) } } diff --git a/vm/src/builtins/getset.rs b/vm/src/builtins/getset.rs index 48204f613b..5a84c5724e 100644 --- a/vm/src/builtins/getset.rs +++ b/vm/src/builtins/getset.rs @@ -4,8 +4,8 @@ use super::pytype::PyTypeRef; use crate::function::{OwnedParam, RefParam}; use crate::pyobject::{ - IntoPyResult, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, - TypeProtocol, + IntoPyResult, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyThreadingConstraint, + PyValue, TryFromObject, TypeProtocol, }; use crate::slots::SlotDescriptor; use crate::vm::VirtualMachine; @@ -13,9 +13,13 @@ use crate::vm::VirtualMachine; pub type PyGetterFunc = Box PyResult)>; pub type PySetterFunc = Box PyResult<()>)>; +pub type PyDeleterFunc = Box PyResult<()>)>; -pub trait IntoPyGetterFunc { - fn into_getter(self) -> PyGetterFunc; +pub trait IntoPyGetterFunc: PyThreadingConstraint + Sized + 'static { + fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult; + fn into_getter(self) -> PyGetterFunc { + Box::new(move |vm, obj| self.get(obj, vm)) + } } impl IntoPyGetterFunc<(OwnedParam, R, VirtualMachine)> for F @@ -24,11 +28,9 @@ where T: TryFromObject, R: IntoPyResult, { - fn into_getter(self) -> PyGetterFunc { - Box::new(move |vm: &VirtualMachine, obj| { - let obj = T::try_from_object(vm, obj)?; - (self)(obj, vm).into_pyresult(vm) - }) + fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let obj = T::try_from_object(vm, obj)?; + (self)(obj, vm).into_pyresult(vm) } } @@ -38,11 +40,9 @@ where S: PyValue, R: IntoPyResult, { - fn into_getter(self) -> PyGetterFunc { - Box::new(move |vm: &VirtualMachine, obj| { - let zelf = PyRef::::try_from_object(vm, obj)?; - (self)(&zelf, vm).into_pyresult(vm) - }) + fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let zelf = PyRef::::try_from_object(vm, obj)?; + (self)(&zelf, vm).into_pyresult(vm) } } @@ -52,8 +52,9 @@ where T: TryFromObject, R: IntoPyResult, { - fn into_getter(self) -> PyGetterFunc { - IntoPyGetterFunc::into_getter(move |obj, _vm: &VirtualMachine| (self)(obj)) + fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let obj = T::try_from_object(vm, obj)?; + (self)(obj).into_pyresult(vm) } } @@ -63,8 +64,9 @@ where S: PyValue, R: IntoPyResult, { - fn into_getter(self) -> PyGetterFunc { - IntoPyGetterFunc::into_getter(move |zelf: &S, _vm: &VirtualMachine| (self)(zelf)) + fn get(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let zelf = PyRef::::try_from_object(vm, obj)?; + (self)(&zelf).into_pyresult(vm) } } @@ -73,19 +75,24 @@ pub trait IntoPyNoResult { } impl IntoPyNoResult for () { + #[inline] fn into_noresult(self) -> PyResult<()> { Ok(()) } } impl IntoPyNoResult for PyResult<()> { + #[inline] fn into_noresult(self) -> PyResult<()> { self } } -pub trait IntoPySetterFunc { - fn into_setter(self) -> PySetterFunc; +pub trait IntoPySetterFunc: PyThreadingConstraint + Sized + 'static { + fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()>; + fn into_setter(self) -> PySetterFunc { + Box::new(move |vm, obj, value| self.set(obj, value, vm)) + } } impl IntoPySetterFunc<(OwnedParam, V, R, VirtualMachine)> for F @@ -95,12 +102,10 @@ where V: TryFromObject, R: IntoPyNoResult, { - fn into_setter(self) -> PySetterFunc { - Box::new(move |vm: &VirtualMachine, obj, value| { - let obj = T::try_from_object(vm, obj)?; - let value = V::try_from_object(vm, value)?; - (self)(obj, value, vm).into_noresult() - }) + fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let obj = T::try_from_object(vm, obj)?; + let value = V::try_from_object(vm, value)?; + (self)(obj, value, vm).into_noresult() } } @@ -111,12 +116,10 @@ where V: TryFromObject, R: IntoPyNoResult, { - fn into_setter(self) -> PySetterFunc { - Box::new(move |vm: &VirtualMachine, obj, value| { - let zelf = PyRef::::try_from_object(vm, obj)?; - let value = V::try_from_object(vm, value)?; - (self)(&zelf, value, vm).into_noresult() - }) + fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let zelf = PyRef::::try_from_object(vm, obj)?; + let value = V::try_from_object(vm, value)?; + (self)(&zelf, value, vm).into_noresult() } } @@ -127,8 +130,10 @@ where V: TryFromObject, R: IntoPyNoResult, { - fn into_setter(self) -> PySetterFunc { - IntoPySetterFunc::into_setter(move |obj, v, _vm: &VirtualMachine| (self)(obj, v)) + fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let obj = T::try_from_object(vm, obj)?; + let value = V::try_from_object(vm, value)?; + (self)(obj, value).into_noresult() } } @@ -139,8 +144,65 @@ where V: TryFromObject, R: IntoPyNoResult, { - fn into_setter(self) -> PySetterFunc { - IntoPySetterFunc::into_setter(move |zelf: &S, v, _vm: &VirtualMachine| (self)(zelf, v)) + fn set(&self, obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let zelf = PyRef::::try_from_object(vm, obj)?; + let value = V::try_from_object(vm, value)?; + (self)(&zelf, value).into_noresult() + } +} + +pub trait IntoPyDeleterFunc: PyThreadingConstraint + Sized + 'static { + fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()>; + fn into_deleter(self) -> PyDeleterFunc { + Box::new(move |vm, obj| self.delete(obj, vm)) + } +} + +impl IntoPyDeleterFunc<(OwnedParam, R, VirtualMachine)> for F +where + F: Fn(T, &VirtualMachine) -> R + 'static + Send + Sync, + T: TryFromObject, + R: IntoPyNoResult, +{ + fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let obj = T::try_from_object(vm, obj)?; + (self)(obj, vm).into_noresult() + } +} + +impl IntoPyDeleterFunc<(RefParam, R, VirtualMachine)> for F +where + F: Fn(&S, &VirtualMachine) -> R + 'static + Send + Sync, + S: PyValue, + R: IntoPyNoResult, +{ + fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let zelf = PyRef::::try_from_object(vm, obj)?; + (self)(&zelf, vm).into_noresult() + } +} + +impl IntoPyDeleterFunc<(OwnedParam, R)> for F +where + F: Fn(T) -> R + 'static + Send + Sync, + T: TryFromObject, + R: IntoPyNoResult, +{ + fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let obj = T::try_from_object(vm, obj)?; + (self)(obj).into_noresult() + } +} + +impl IntoPyDeleterFunc<(RefParam, R)> for F +where + F: Fn(&S) -> R + 'static + Send + Sync, + S: PyValue, + R: IntoPyNoResult, +{ + fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let zelf = PyRef::::try_from_object(vm, obj)?; + (self)(&zelf).into_noresult() } } @@ -149,6 +211,7 @@ pub struct PyGetSet { name: String, getter: Option, setter: Option, + deleter: Option, // doc: Option, } @@ -202,27 +265,37 @@ impl SlotDescriptor for PyGetSet { } impl PyGetSet { - pub fn with_get(name: String, getter: G) -> Self - where - G: IntoPyGetterFunc, - { + pub fn new(name: String) -> Self { Self { name, - getter: Some(getter.into_getter()), + getter: None, setter: None, + deleter: None, } } - pub fn with_get_set(name: String, getter: G, setter: S) -> Self + pub fn with_get(mut self, getter: G) -> Self where G: IntoPyGetterFunc, - S: IntoPySetterFunc, { - Self { - name, - getter: Some(getter.into_getter()), - setter: Some(setter.into_setter()), - } + self.getter = Some(getter.into_getter()); + self + } + + pub fn with_set(mut self, setter: S) -> Self + where + S: IntoPySetterFunc, + { + self.setter = Some(setter.into_setter()); + self + } + + pub fn with_delete(mut self, setter: S) -> Self + where + S: IntoPyDeleterFunc, + { + self.deleter = Some(setter.into_deleter()); + self } } @@ -243,6 +316,19 @@ impl PyGetSet { } } + #[pymethod(magic)] + fn delete(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + if let Some(ref f) = self.deleter { + f(vm, obj) + } else { + Err(vm.new_attribute_error(format!( + "attribute '{}' of '{}' objects is not writable", + self.name, + obj.class().name + ))) + } + } + #[pyproperty(magic)] fn name(&self) -> String { self.name.clone() diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index ca5dc940ad..083b47972f 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -323,7 +323,7 @@ impl PyContext { F: IntoPyGetterFunc, { PyObject::new( - PyGetSet::with_get(name.into(), f), + PyGetSet::new(name.into()).with_get(f), self.types.getset_type.clone(), None, ) @@ -335,7 +335,7 @@ impl PyContext { S: IntoPySetterFunc, { PyObject::new( - PyGetSet::with_get_set(name.into(), g, s), + PyGetSet::new(name.into()).with_get(g).with_set(s), self.types.getset_type.clone(), None, ) From 6c20c3e50cbba3aa515e194022f85054340a1583 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Fri, 27 Nov 2020 17:54:16 -0600 Subject: [PATCH 02/14] Add the cell class --- vm/src/builtins/function.rs | 55 +++++++++++++++++++++++++++++++++---- vm/src/types.rs | 2 ++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index 3c699ea1be..a62759b6c3 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -10,8 +10,9 @@ use crate::builtins::asyncgenerator::PyAsyncGen; use crate::builtins::coroutine::PyCoroutine; use crate::builtins::generator::PyGenerator; use crate::bytecode; +use crate::common::lock::PyMutex; use crate::frame::Frame; -use crate::function::FuncArgs; +use crate::function::{FuncArgs, OptionalArg}; #[cfg(feature = "jit")] use crate::pyobject::IntoPyObject; use crate::pyobject::{ @@ -391,10 +392,52 @@ impl PyValue for PyBoundMethod { } } -pub fn init(context: &PyContext) { - let function_type = &context.types.function_type; - PyFunction::extend_class(context, function_type); +#[pyclass(module = false, name = "cell")] +#[derive(Debug)] +pub(crate) struct PyCell { + contents: PyMutex>, +} + +impl PyValue for PyCell { + fn class(vm: &VirtualMachine) -> &PyTypeRef { + &vm.ctx.types.cell_type + } +} + +#[pyimpl] +impl PyCell { + #[pyslot] + fn tp_new(cls: PyTypeRef, value: OptionalArg, vm: &VirtualMachine) -> PyResult> { + Self { + contents: PyMutex::new(value.into_option()), + } + .into_ref_with_type(vm, cls) + } - let method_type = &context.types.bound_method_type; - PyBoundMethod::extend_class(context, method_type); + pub fn get(&self) -> Option { + self.contents.lock().clone() + } + pub fn set(&self, x: Option) { + *self.contents.lock() = x; + } + + #[pyproperty] + fn cell_contents(&self, vm: &VirtualMachine) -> PyResult { + self.get() + .ok_or_else(|| vm.new_value_error("Cell is empty".to_owned())) + } + #[pyproperty(setter)] + fn set_cell_contents(&self, x: PyObjectRef) { + self.set(Some(x)) + } + #[pyproperty(deleter)] + fn del_cell_contents(&self) { + self.set(None) + } +} + +pub fn init(context: &PyContext) { + PyFunction::extend_class(context, &context.types.function_type); + PyBoundMethod::extend_class(context, &context.types.bound_method_type); + PyCell::extend_class(context, &context.types.cell_type); } diff --git a/vm/src/types.rs b/vm/src/types.rs index 834a9e01d8..a810107819 100644 --- a/vm/src/types.rs +++ b/vm/src/types.rs @@ -54,6 +54,7 @@ pub struct TypeZoo { pub bytearray_iterator_type: PyTypeRef, pub bool_type: PyTypeRef, pub callable_iterator: PyTypeRef, + pub cell_type: PyTypeRef, pub classmethod_type: PyTypeRef, pub code_type: PyTypeRef, pub coroutine_type: PyTypeRef, @@ -159,6 +160,7 @@ impl TypeZoo { bytearray_iterator_type: bytearray::PyByteArrayIterator::init_bare_type().clone(), bytes_iterator_type: bytes::PyBytesIterator::init_bare_type().clone(), callable_iterator: iter::PyCallableIterator::init_bare_type().clone(), + cell_type: function::PyCell::init_bare_type().clone(), code_type: code::PyCode::init_bare_type().clone(), coroutine_type: coroutine::PyCoroutine::init_bare_type().clone(), coroutine_wrapper_type: coroutine::PyCoroutineWrapper::init_bare_type().clone(), From 3d4474413bbb6de6270e2b9c69202051ea25ca14 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Fri, 27 Nov 2020 20:48:23 -0600 Subject: [PATCH 03/14] Add PyTupleTyped --- vm/src/builtins/mod.rs | 2 +- vm/src/builtins/tuple.rs | 40 ++++++++++++++++++++++++++++++++++------ vm/src/pyobject.rs | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/vm/src/builtins/mod.rs b/vm/src/builtins/mod.rs index b199b89f88..d7478b180f 100644 --- a/vm/src/builtins/mod.rs +++ b/vm/src/builtins/mod.rs @@ -70,7 +70,7 @@ pub(crate) mod staticmethod; pub use staticmethod::PyStaticMethod; pub(crate) mod traceback; pub use traceback::PyTraceback; -pub(crate) mod tuple; +pub mod tuple; pub use tuple::PyTuple; pub(crate) mod weakproxy; pub use weakproxy::PyWeakProxy; diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index 91ce4050d4..a47d8c69a1 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -1,12 +1,14 @@ use crossbeam_utils::atomic::AtomicCell; use std::fmt; +use std::marker::PhantomData; use super::pytype::PyTypeRef; use crate::common::hash::PyHash; use crate::function::OptionalArg; use crate::pyobject::{ self, BorrowValue, Either, IdProtocol, IntoPyObject, PyArithmaticValue, PyClassImpl, - PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol, + PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TransmuteFromObject, + TryFromObject, TypeProtocol, }; use crate::sequence::{self, SimpleSeq}; use crate::sliceable::PySliceableSequence; @@ -262,7 +264,7 @@ impl Iterable for PyTuple { #[pyclass(module = false, name = "tuple_iterator")] #[derive(Debug)] -pub struct PyTupleIterator { +pub(crate) struct PyTupleIterator { position: AtomicCell, tuple: PyTupleRef, } @@ -287,9 +289,35 @@ impl PyIter for PyTupleIterator { } } -pub fn init(context: &PyContext) { - let tuple_type = &context.types.tuple_type; - PyTuple::extend_class(context, tuple_type); - +pub(crate) fn init(context: &PyContext) { + PyTuple::extend_class(context, &context.types.tuple_type); PyTupleIterator::extend_class(context, &context.types.tuple_iterator_type); } + +pub struct PyTupleTyped { + // SAFETY INVARIANT: T must be repr(transparent) over PyObjectRef, and the + // elements must be logically valid when transmuted to T + tuple: PyTupleRef, + _marker: PhantomData>, +} + +impl TryFromObject for PyTupleTyped { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + let tuple = PyTupleRef::try_from_object(vm, obj)?; + for elem in tuple.borrow_value() { + T::check(vm, elem)? + } + // SAFETY: the contract of TransmuteFromObject upholds the variant on `tuple` + Ok(Self { + tuple, + _marker: PhantomData, + }) + } +} + +impl<'a, T: 'a> BorrowValue<'a> for PyTupleTyped { + type Borrowed = &'a [T]; + fn borrow_value(&'a self) -> Self::Borrowed { + unsafe { &*(self.tuple.borrow_value() as *const [PyObjectRef] as *const [T]) } + } +} diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 083b47972f..03dc97d718 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -785,6 +785,40 @@ pub trait TryFromObject: Sized { fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult; } +/// Marks a type that has the exact same layout as PyObjectRef, e.g. a type that is +/// `repr(transparent)` over PyObjectRef. +/// +/// # Safety +/// Can only be implemented for types that are `repr(transparent)` over a PyObjectRef `obj`, +/// and logically valid so long as `check(vm, obj)` returns `Ok(())` +pub unsafe trait TransmuteFromObject: Sized { + fn check(vm: &VirtualMachine, obj: &PyObjectRef) -> PyResult<()>; +} + +unsafe impl TransmuteFromObject for PyRef { + fn check(vm: &VirtualMachine, obj: &PyObjectRef) -> PyResult<()> { + let class = T::class(vm); + if obj.isinstance(class) { + if obj.payload_is::() { + Ok(()) + } else { + Err(vm.new_runtime_error(format!( + "Unexpected payload '{}' for type '{}'", + class.name, + obj.class().name, + ))) + } + } else { + let expected_type = &class.name; + let actual_type = &obj.class().name; + Err(vm.new_type_error(format!( + "Expected type '{}', not '{}'", + expected_type, actual_type, + ))) + } + } +} + pub trait IntoPyRef { fn into_pyref(self, vm: &VirtualMachine) -> PyRef; } From 479610f3cbe7374a3093b7b575a9f6440c0de354 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 30 Nov 2020 22:16:23 -0600 Subject: [PATCH 04/14] Move compiler snapshots --- ...nap => rustpython_compiler_core__compile__tests__if_ands.snap} | 0 ...ap => rustpython_compiler_core__compile__tests__if_mixed.snap} | 0 ...snap => rustpython_compiler_core__compile__tests__if_ors.snap} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename compiler/src/snapshots/{rustpython_compiler__compile__tests__if_ands.snap => rustpython_compiler_core__compile__tests__if_ands.snap} (100%) rename compiler/src/snapshots/{rustpython_compiler__compile__tests__if_mixed.snap => rustpython_compiler_core__compile__tests__if_mixed.snap} (100%) rename compiler/src/snapshots/{rustpython_compiler__compile__tests__if_ors.snap => rustpython_compiler_core__compile__tests__if_ors.snap} (100%) diff --git a/compiler/src/snapshots/rustpython_compiler__compile__tests__if_ands.snap b/compiler/src/snapshots/rustpython_compiler_core__compile__tests__if_ands.snap similarity index 100% rename from compiler/src/snapshots/rustpython_compiler__compile__tests__if_ands.snap rename to compiler/src/snapshots/rustpython_compiler_core__compile__tests__if_ands.snap diff --git a/compiler/src/snapshots/rustpython_compiler__compile__tests__if_mixed.snap b/compiler/src/snapshots/rustpython_compiler_core__compile__tests__if_mixed.snap similarity index 100% rename from compiler/src/snapshots/rustpython_compiler__compile__tests__if_mixed.snap rename to compiler/src/snapshots/rustpython_compiler_core__compile__tests__if_mixed.snap diff --git a/compiler/src/snapshots/rustpython_compiler__compile__tests__if_ors.snap b/compiler/src/snapshots/rustpython_compiler_core__compile__tests__if_ors.snap similarity index 100% rename from compiler/src/snapshots/rustpython_compiler__compile__tests__if_ors.snap rename to compiler/src/snapshots/rustpython_compiler_core__compile__tests__if_ors.snap From 97029af713d487c31e86b9791bfe69d2a4622645 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sat, 28 Nov 2020 14:51:28 -0600 Subject: [PATCH 05/14] Fast locals part 1 --- bytecode/src/bytecode.rs | 127 +++++++----- compiler/src/compile.rs | 347 +++++++++++++++++++-------------- compiler/src/symboltable.rs | 85 ++++---- vm/src/builtins/dict.rs | 7 +- vm/src/builtins/frame.rs | 6 +- vm/src/builtins/function.rs | 86 ++++---- vm/src/builtins/make_module.rs | 24 +-- vm/src/builtins/pysuper.rs | 38 ++-- vm/src/builtins/tuple.rs | 10 +- vm/src/frame.rs | 242 +++++++++++++++-------- vm/src/pyobject.rs | 26 +-- vm/src/scope.rs | 222 ++++++++++----------- vm/src/stdlib/symtable.rs | 4 +- vm/src/vm.rs | 26 +-- 14 files changed, 715 insertions(+), 535 deletions(-) diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 74a74af506..42caf79121 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -108,6 +108,9 @@ pub struct CodeObject { serialize = "C::Name: serde::Serialize" ))] pub names: Vec, + pub varnames: Vec, + pub cellvars: Vec, + pub freevars: Vec, } bitflags! { @@ -121,6 +124,7 @@ bitflags! { const IS_COROUTINE = 0x20; const HAS_VARARGS = 0x40; const HAS_VARKEYWORDS = 0x80; + const IS_OPTIMIZED = 0x0100; } } @@ -154,22 +158,6 @@ impl fmt::Display for Label { } } -#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] -/// An indication where the name must be accessed. -pub enum NameScope { - /// The name will be in the local scope. - Local, - - /// The name will be located in scope surrounding the current scope. - NonLocal, - - /// The name will be in global scope. - Global, - - /// The name will be located in any scope between the current scope and the top scope. - Free, -} - /// Transforms a value prior to formatting it. #[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum ConversionFlag { @@ -195,17 +183,20 @@ pub enum Instruction { ImportFrom { idx: NameIdx, }, - LoadName { - idx: NameIdx, - scope: NameScope, - }, - StoreName { - idx: NameIdx, - scope: NameScope, - }, - DeleteName { - idx: NameIdx, - }, + LoadFast(NameIdx), + LoadLocal(NameIdx), + LoadGlobal(NameIdx), + LoadDeref(NameIdx), + LoadClassDeref(NameIdx), + StoreFast(NameIdx), + StoreLocal(NameIdx), + StoreGlobal(NameIdx), + StoreDeref(NameIdx), + DeleteFast(NameIdx), + DeleteLocal(NameIdx), + DeleteGlobal(NameIdx), + DeleteDeref(NameIdx), + LoadClosure(NameIdx), Subscript, StoreSubscript, DeleteSubscript, @@ -561,6 +552,9 @@ impl CodeObject { obj_name, constants: Vec::new(), names: Vec::new(), + varnames: Vec::new(), + cellvars: Vec::new(), + freevars: Vec::new(), } } @@ -569,19 +563,19 @@ impl CodeObject { let nargs = self.arg_count; let nkwargs = self.kwonlyarg_count; let mut varargspos = nargs + nkwargs; - let posonlyargs = &self.names[..self.posonlyarg_count]; - let args = &self.names[..nargs]; - let kwonlyargs = &self.names[nargs..varargspos]; + let posonlyargs = &self.varnames[..self.posonlyarg_count]; + let args = &self.varnames[..nargs]; + let kwonlyargs = &self.varnames[nargs..varargspos]; let vararg = if self.flags.contains(CodeFlags::HAS_VARARGS) { - let vararg = &self.names[varargspos]; + let vararg = &self.varnames[varargspos]; varargspos += 1; Some(vararg) } else { None }; let varkwarg = if self.flags.contains(CodeFlags::HAS_VARKEYWORDS) { - Some(&self.names[varargspos]) + Some(&self.varnames[varargspos]) } else { None }; @@ -617,8 +611,10 @@ impl CodeObject { } #[rustfmt::skip] - Import { .. } | ImportStar | ImportFrom { .. } | LoadName { .. } | StoreName { .. } - | DeleteName { .. } | Subscript | StoreSubscript | DeleteSubscript + Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadLocal(_) + | LoadGlobal(_) | LoadDeref(_) | LoadClassDeref(_) | StoreFast(_) | StoreLocal(_) + | StoreGlobal(_) | StoreDeref(_) | DeleteFast(_) | DeleteLocal(_) | DeleteGlobal(_) + | DeleteDeref(_) | LoadClosure(_) | Subscript | StoreSubscript | DeleteSubscript | StoreAttr { .. } | DeleteAttr { .. } | LoadConst { .. } | UnaryOperation { .. } | BinaryOperation { .. } | LoadAttr { .. } | CompareOperation { .. } | Pop | Rotate { .. } | Duplicate | GetIter | Continue | Break | MakeFunction @@ -652,7 +648,16 @@ impl CodeObject { write!(f, " ")?; } write!(f, "{} {:5} ", arrow, offset)?; - instruction.fmt_dis(f, &self.constants, &self.names, expand_codeobjects, level)?; + instruction.fmt_dis( + f, + &self.constants, + &self.names, + &self.varnames, + &self.cellvars, + &self.freevars, + expand_codeobjects, + level, + )?; } Ok(()) } @@ -668,17 +673,22 @@ impl CodeObject { } pub fn map_bag(self, bag: &Bag) -> CodeObject { + let map_names = |names: Vec| { + names + .into_iter() + .map(|x| bag.make_name_ref(x.as_ref())) + .collect::>() + }; CodeObject { constants: self .constants .into_iter() .map(|x| x.map_constant(bag)) .collect(), - names: self - .names - .into_iter() - .map(|x| bag.make_name_ref(x.as_ref())) - .collect(), + names: map_names(self.names), + varnames: map_names(self.varnames), + cellvars: map_names(self.cellvars), + freevars: map_names(self.freevars), instructions: self.instructions, locations: self.locations, @@ -693,17 +703,22 @@ impl CodeObject { } pub fn map_clone_bag(&self, bag: &Bag) -> CodeObject { + let map_names = |names: &[C::Name]| { + names + .iter() + .map(|x| bag.make_name_ref(x.as_ref())) + .collect() + }; CodeObject { constants: self .constants .iter() .map(|x| bag.make_constant_borrowed(x.borrow_constant())) .collect(), - names: self - .names - .iter() - .map(|x| bag.make_name_ref(x.as_ref())) - .collect(), + names: map_names(&self.names), + varnames: map_names(&self.varnames), + cellvars: map_names(&self.cellvars), + freevars: map_names(&self.freevars), instructions: self.instructions.clone(), locations: self.locations.clone(), @@ -755,6 +770,9 @@ impl Instruction { f: &mut fmt::Formatter, constants: &[C], names: &[C::Name], + varnames: &[C::Name], + cellvars: &[C::Name], + freevars: &[C::Name], expand_codeobjects: bool, level: usize, ) -> fmt::Result { @@ -780,6 +798,8 @@ impl Instruction { }; } + let cellname = |i: usize| cellvars.get(i).unwrap_or_else(|| &freevars[i]).as_ref(); + match self { Import { name_idx, @@ -799,9 +819,20 @@ impl Instruction { ), ImportStar => w!(ImportStar), ImportFrom { idx } => w!(ImportFrom, names[*idx].as_ref()), - LoadName { idx, scope } => w!(LoadName, names[*idx].as_ref(), format!("{:?}", scope)), - StoreName { idx, scope } => w!(StoreName, names[*idx].as_ref(), format!("{:?}", scope)), - DeleteName { idx } => w!(DeleteName, names[*idx].as_ref()), + LoadFast(idx) => w!(LoadFast, varnames[*idx].as_ref()), + LoadLocal(idx) => w!(LoadLocal, names[*idx].as_ref()), + LoadGlobal(idx) => w!(LoadGlobal, names[*idx].as_ref()), + LoadDeref(idx) => w!(LoadDeref, cellname(*idx)), + LoadClassDeref(idx) => w!(LoadClassDeref, cellname(*idx)), + StoreFast(idx) => w!(StoreFast, varnames[*idx].as_ref()), + StoreLocal(idx) => w!(StoreLocal, names[*idx].as_ref()), + StoreGlobal(idx) => w!(StoreGlobal, names[*idx].as_ref()), + StoreDeref(idx) => w!(StoreDeref, cellname(*idx)), + DeleteFast(idx) => w!(DeleteFast, varnames[*idx].as_ref()), + DeleteLocal(idx) => w!(DeleteLocal, names[*idx].as_ref()), + DeleteGlobal(idx) => w!(DeleteGlobal, names[*idx].as_ref()), + DeleteDeref(idx) => w!(DeleteDeref, cellname(*idx)), + LoadClosure(i) => w!(LoadClosure, cellname(*i)), Subscript => w!(Subscript), StoreSubscript => w!(StoreSubscript), DeleteSubscript => w!(DeleteSubscript), diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index d06188307e..3137b47b4c 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -7,9 +7,7 @@ use crate::error::{CompileError, CompileErrorType}; pub use crate::mode::Mode; -use crate::symboltable::{ - make_symbol_table, statements_to_symbol_table, Symbol, SymbolScope, SymbolTable, -}; +use crate::symboltable::{make_symbol_table, statements_to_symbol_table, SymbolScope, SymbolTable}; use indexmap::IndexSet; use itertools::Itertools; use num_complex::Complex64; @@ -21,6 +19,9 @@ type CompileResult = Result; struct CodeInfo { code: CodeObject, name_cache: IndexSet, + varname_cache: IndexSet, + cellvar_cache: IndexSet, + freevar_cache: IndexSet, label_map: Vec>, } impl CodeInfo { @@ -28,9 +29,17 @@ impl CodeInfo { let CodeInfo { mut code, name_cache, + varname_cache, + cellvar_cache, + freevar_cache, label_map, } = self; + code.names.extend(name_cache); + code.varnames.extend(varname_cache); + code.cellvars.extend(cellvar_cache); + code.freevars.extend(freevar_cache); + for instruction in &mut code.instructions { use Instruction::*; // this is a little bit hacky, as until now the data stored inside Labels in @@ -55,8 +64,10 @@ impl CodeInfo { } #[rustfmt::skip] - Import { .. } | ImportStar | ImportFrom { .. } | LoadName { .. } | StoreName { .. } - | DeleteName { .. } | Subscript | StoreSubscript | DeleteSubscript + Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadLocal(_) + | LoadGlobal(_) | LoadDeref(_) | LoadClassDeref(_) | StoreFast(_) | StoreLocal(_) + | StoreGlobal(_) | StoreDeref(_) | DeleteFast(_) | DeleteLocal(_) | DeleteGlobal(_) + | DeleteDeref(_) | LoadClosure(_) | Subscript | StoreSubscript | DeleteSubscript | StoreAttr { .. } | DeleteAttr { .. } | LoadConst { .. } | UnaryOperation { .. } | BinaryOperation { .. } | LoadAttr { .. } | CompareOperation { .. } | Pop | Rotate { .. } | Duplicate | GetIter | Continue | Break | MakeFunction @@ -73,6 +84,12 @@ impl CodeInfo { } } +enum NameUsage { + Load, + Store, + Delete, +} + /// Main structure holding the state of compilation. struct Compiler { code_stack: Vec, @@ -100,6 +117,7 @@ impl Default for CompileOpts { #[derive(Clone, Copy)] struct CompileContext { in_loop: bool, + in_class: bool, func: FunctionContext, } @@ -122,8 +140,7 @@ fn with_compiler( opts: CompileOpts, f: impl FnOnce(&mut Compiler) -> CompileResult<()>, ) -> CompileResult { - let mut compiler = Compiler::new(opts, source_path); - compiler.push_new_code_object("".to_owned()); + let mut compiler = Compiler::new(opts, source_path, "".to_owned()); f(&mut compiler)?; let code = compiler.pop_code_object(); trace!("Compilation completed: {:?}", code); @@ -176,9 +193,25 @@ pub fn compile_program_single( } impl Compiler { - fn new(opts: CompileOpts, source_path: String) -> Self { + fn new(opts: CompileOpts, source_path: String, code_name: String) -> Self { + let module_code = CodeInfo { + code: CodeObject::new( + Default::default(), + 0, + 0, + 0, + source_path.clone(), + 0, + code_name, + ), + name_cache: IndexSet::new(), + varname_cache: IndexSet::new(), + cellvar_cache: IndexSet::new(), + freevar_cache: IndexSet::new(), + label_map: Vec::new(), + }; Compiler { - code_stack: Vec::new(), + code_stack: vec![module_code], symbol_table_stack: Vec::new(), source_path, current_source_location: ast::Location::default(), @@ -186,6 +219,7 @@ impl Compiler { done_with_future_stmts: false, ctx: CompileContext { in_loop: false, + in_class: false, func: FunctionContext::NoFunction, }, opts, @@ -204,43 +238,63 @@ impl Compiler { } fn push_output(&mut self, code: CodeObject) { - self.code_stack.push(CodeInfo { + let table = self + .symbol_table_stack + .last_mut() + .unwrap() + .sub_tables + .remove(0); + + let cellvar_cache = table + .symbols + .iter() + .filter(|(_, s)| matches!(s.scope, SymbolScope::Cell)) + .map(|(var, _)| var.clone()) + .collect(); + let freevar_cache = table + .symbols + .iter() + // TODO: check if Free or FREE_CLASS symbol + .filter(|(_, s)| matches!(s.scope, SymbolScope::Free)) + .map(|(var, _)| var.clone()) + .collect(); + + self.symbol_table_stack.push(table); + + let info = CodeInfo { code, name_cache: IndexSet::new(), + varname_cache: IndexSet::new(), + cellvar_cache, + freevar_cache, label_map: Vec::new(), - }); - } - - fn push_new_code_object(&mut self, obj_name: String) { - let line_number = self.get_source_line_number(); - self.push_output(CodeObject::new( - Default::default(), - 0, - 0, - 0, - self.source_path.clone(), - line_number, - obj_name, - )); + }; + self.code_stack.push(info); } fn pop_code_object(&mut self) -> CodeObject { + let table = self.symbol_table_stack.pop().unwrap(); + assert!(table.sub_tables.is_empty()); self.code_stack.pop().unwrap().finalize_code() } // could take impl Into>, but everything is borrowed from ast structs; we never // actually have a `String` to pass fn name(&mut self, name: &str) -> bytecode::NameIdx { - let cache = &mut self - .code_stack - .last_mut() - .expect("nothing on stack") - .name_cache; - if let Some(x) = cache.get_index_of(name) { - x - } else { - cache.insert_full(name.to_owned()).0 - } + self._name_inner(name, |i| &mut i.name_cache) + } + fn varname(&mut self, name: &str) -> bytecode::NameIdx { + self._name_inner(name, |i| &mut i.varname_cache) + } + fn _name_inner( + &mut self, + name: &str, + cache: impl FnOnce(&mut CodeInfo) -> &mut IndexSet, + ) -> bytecode::NameIdx { + let cache = cache(self.code_stack.last_mut().expect("nothing on stack")); + cache + .get_index_of(name) + .unwrap_or_else(|| cache.insert_full(name.to_owned()).0) } fn compile_program( @@ -255,10 +309,7 @@ impl Compiler { if let Some(value) = doc { self.emit_constant(bytecode::ConstantData::Str { value }); let doc = self.name("__doc__"); - self.emit(Instruction::StoreName { - idx: doc, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::StoreGlobal(doc)) } self.compile_statements(statements)?; @@ -331,34 +382,73 @@ impl Compiler { Ok(()) } - fn scope_for_name(&self, name: &str) -> bytecode::NameScope { - let symbol = self.lookup_name(name); - match symbol.scope { - SymbolScope::Global => bytecode::NameScope::Global, - SymbolScope::Nonlocal => bytecode::NameScope::NonLocal, - SymbolScope::Unknown => bytecode::NameScope::Free, - SymbolScope::Local => { - // Only in function block, we use load local - // https://github.com/python/cpython/blob/master/Python/compile.c#L3582 - if self.ctx.in_func() { - bytecode::NameScope::Local - } else { - bytecode::NameScope::Free - } - } - } - } - fn load_name(&mut self, name: &str) { - let scope = self.scope_for_name(name); - let idx = self.name(name); - self.emit(Instruction::LoadName { idx, scope }); + self.compile_name(name, NameUsage::Load) } fn store_name(&mut self, name: &str) { - let scope = self.scope_for_name(name); - let idx = self.name(name); - self.emit(Instruction::StoreName { idx, scope }); + self.compile_name(name, NameUsage::Store) + } + + fn compile_name(&mut self, name: &str, usage: NameUsage) { + let symbol_table = self.symbol_table_stack.last().unwrap(); + let symbol = symbol_table.lookup(name).expect( + "The symbol must be present in the symbol table, even when it is undefined in python.", + ); + let info = self.code_stack.last_mut().unwrap(); + let mut cache = &mut info.name_cache; + enum NameOpType { + Fast, + Global, + Deref, + Local, + } + let op_typ = match symbol.scope { + SymbolScope::Local if self.ctx.in_func() => { + cache = &mut info.varname_cache; + NameOpType::Fast + } + SymbolScope::Local => NameOpType::Local, + SymbolScope::GlobalImplicit if self.ctx.in_func() => NameOpType::Global, + SymbolScope::GlobalImplicit => NameOpType::Local, + SymbolScope::GlobalExplicit => NameOpType::Global, + SymbolScope::Free => { + cache = &mut info.freevar_cache; + NameOpType::Deref + } + SymbolScope::Cell => { + cache = &mut info.cellvar_cache; + NameOpType::Deref + } + // TODO: is this right? + SymbolScope::Unknown => NameOpType::Global, + }; + let idx = cache + .get_index_of(name) + .unwrap_or_else(|| cache.insert_full(name.to_owned()).0); + let op = match op_typ { + NameOpType::Fast => match usage { + NameUsage::Load => Instruction::LoadFast, + NameUsage::Store => Instruction::StoreFast, + NameUsage::Delete => Instruction::DeleteFast, + }, + NameOpType::Global => match usage { + NameUsage::Load => Instruction::LoadGlobal, + NameUsage::Store => Instruction::StoreGlobal, + NameUsage::Delete => Instruction::DeleteGlobal, + }, + NameOpType::Deref => match usage { + NameUsage::Load => Instruction::LoadDeref, + NameUsage::Store => Instruction::StoreDeref, + NameUsage::Delete => Instruction::DeleteDeref, + }, + NameOpType::Local => match usage { + NameUsage::Load => Instruction::LoadLocal, + NameUsage::Store => Instruction::StoreLocal, + NameUsage::Delete => Instruction::DeleteLocal, + }, + }; + self.emit(op(idx)); } fn compile_statement(&mut self, statement: &ast::Statement) -> CompileResult<()> { @@ -584,10 +674,7 @@ impl Compiler { let end_label = self.new_label(); self.compile_jump_if(test, true, end_label)?; let assertion_error = self.name("AssertionError"); - self.emit(Instruction::LoadName { - idx: assertion_error, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::LoadGlobal(assertion_error)); match msg { Some(e) => { self.compile_expression(e)?; @@ -683,8 +770,7 @@ impl Compiler { fn compile_delete(&mut self, expression: &ast::Expression) -> CompileResult<()> { match &expression.node { ast::ExpressionType::Identifier { name } => { - let idx = self.name(name); - self.emit(Instruction::DeleteName { idx }); + self.compile_name(name, NameUsage::Delete); } ast::ExpressionType::Attribute { value, name } => { self.compile_expression(value)?; @@ -738,7 +824,7 @@ impl Compiler { }); } - let mut flags = bytecode::CodeFlags::default(); + let mut flags = bytecode::CodeFlags::NEW_LOCALS | bytecode::CodeFlags::IS_OPTIMIZED; if have_defaults { flags |= bytecode::CodeFlags::HAS_DEFAULTS; } @@ -775,8 +861,6 @@ impl Compiler { compile_varargs(&args.vararg, bytecode::CodeFlags::HAS_VARARGS); compile_varargs(&args.kwarg, bytecode::CodeFlags::HAS_VARKEYWORDS); - self.enter_scope(); - Ok(()) } @@ -923,6 +1007,7 @@ impl Compiler { self.ctx = CompileContext { in_loop: false, + in_class: prev_ctx.in_class, func: if is_async { FunctionContext::AsyncFunction } else { @@ -954,7 +1039,6 @@ impl Compiler { } let mut code = self.pop_code_object(); - self.leave_scope(); // Prepare type annotations: let mut num_annotations = 0; @@ -1006,6 +1090,27 @@ impl Compiler { code.flags |= bytecode::CodeFlags::IS_COROUTINE; } + if !code.freevars.is_empty() { + for var in &code.freevars { + let symbol = self.symbol_table_stack.last().unwrap().lookup(var).unwrap(); + let parent_code = self.code_stack.last().unwrap(); + let vars = match symbol.scope { + SymbolScope::Free => &parent_code.freevar_cache, + SymbolScope::Cell => &parent_code.cellvar_cache, + _ => unreachable!(), + }; + let mut idx = vars.get_index_of(var).unwrap(); + if let SymbolScope::Free = symbol.scope { + idx += parent_code.cellvar_cache.len(); + } + self.emit(Instruction::LoadClosure(idx)) + } + self.emit(Instruction::BuildTuple { + size: code.freevars.len(), + unpack: false, + }) + } + self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), }); @@ -1098,6 +1203,7 @@ impl Compiler { let prev_ctx = self.ctx; self.ctx = CompileContext { func: FunctionContext::NoFunction, + in_class: true, in_loop: false, }; @@ -1109,7 +1215,7 @@ impl Compiler { self.emit(Instruction::LoadBuildClass); let line_number = self.get_source_line_number(); self.push_output(CodeObject::new( - Default::default(), + bytecode::CodeFlags::empty(), 0, 0, 0, @@ -1117,34 +1223,21 @@ impl Compiler { line_number, name.to_owned(), )); - self.enter_scope(); let (new_body, doc_str) = get_doc(body); let dunder_name = self.name("__name__"); - self.emit(Instruction::LoadName { - idx: dunder_name, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::LoadGlobal(dunder_name)); let dunder_module = self.name("__module__"); - self.emit(Instruction::StoreName { - idx: dunder_module, - scope: bytecode::NameScope::Free, - }); + self.emit(Instruction::StoreLocal(dunder_module)); self.emit_constant(bytecode::ConstantData::Str { value: qualified_name.clone(), }); let qualname = self.name("__qualname__"); - self.emit(Instruction::StoreName { - idx: qualname, - scope: bytecode::NameScope::Free, - }); + self.emit(Instruction::StoreLocal(qualname)); self.load_docstring(doc_str); let doc = self.name("__doc__"); - self.emit(Instruction::StoreName { - idx: doc, - scope: bytecode::NameScope::Free, - }); + self.emit(Instruction::StoreLocal(doc)); // setup annotations if self.find_ann(body) { self.emit(Instruction::SetupAnnotation); @@ -1153,9 +1246,7 @@ impl Compiler { self.emit_constant(bytecode::ConstantData::None); self.emit(Instruction::ReturnValue); - let mut code = self.pop_code_object(); - code.flags.remove(bytecode::CodeFlags::NEW_LOCALS); - self.leave_scope(); + let code = self.pop_code_object(); self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), @@ -1295,10 +1386,7 @@ impl Compiler { self.set_label(check_asynciter_label); self.emit(Instruction::Duplicate); let stopasynciter = self.name("StopAsyncIteration"); - self.emit(Instruction::LoadName { - idx: stopasynciter, - scope: bytecode::NameScope::Global, - }); + self.emit(Instruction::LoadGlobal(stopasynciter)); self.emit(Instruction::CompareOperation { op: bytecode::ComparisonOperator::ExceptionMatch, }); @@ -1436,15 +1524,14 @@ impl Compiler { if let ast::ExpressionType::Identifier { name } = &target.node { // Store as dict entry in __annotations__ dict: - let annotations = self.name("__annotations__"); - self.emit(Instruction::LoadName { - idx: annotations, - scope: bytecode::NameScope::Local, - }); - self.emit_constant(bytecode::ConstantData::Str { - value: name.to_owned(), - }); - self.emit(Instruction::StoreSubscript); + if !self.ctx.in_func() { + let annotations = self.name("__annotations__"); + self.emit(Instruction::LoadLocal(annotations)); + self.emit_constant(bytecode::ConstantData::Str { + value: name.to_owned(), + }); + self.emit(Instruction::StoreSubscript); + } } else { // Drop annotation if not assigned to simple identifier. self.emit(Instruction::Pop); @@ -1846,6 +1933,7 @@ impl Compiler { let prev_ctx = self.ctx; self.ctx = CompileContext { in_loop: false, + in_class: prev_ctx.in_class, func: FunctionContext::Function, }; @@ -1854,7 +1942,6 @@ impl Compiler { self.compile_expression(body)?; self.emit(Instruction::ReturnValue); let code = self.pop_code_object(); - self.leave_scope(); self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), }); @@ -2044,8 +2131,7 @@ impl Compiler { line_number, name.clone(), )); - let arg0 = self.name(".0"); - self.enter_scope(); + let arg0 = self.varname(".0"); // Create empty object of proper type: match kind { @@ -2079,10 +2165,7 @@ impl Compiler { if loop_labels.is_empty() { // Load iterator onto stack (passed as first argument): - self.emit(Instruction::LoadName { - idx: arg0, - scope: bytecode::NameScope::Local, - }); + self.emit(Instruction::LoadFast(arg0)); } else { // Evaluate iterated item: self.compile_expression(&generator.iter)?; @@ -2169,9 +2252,6 @@ impl Compiler { // Fetch code for listcomp function: let code = self.pop_code_object(); - // Pop scope - self.leave_scope(); - // List comprehension code: self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), @@ -2255,33 +2335,6 @@ impl Compiler { Ok(()) } - // Scope helpers: - fn enter_scope(&mut self) { - // println!("Enter scope {:?}", self.symbol_table_stack); - // Enter first subscope! - let table = self - .symbol_table_stack - .last_mut() - .unwrap() - .sub_tables - .remove(0); - self.symbol_table_stack.push(table); - } - - fn leave_scope(&mut self) { - // println!("Leave scope {:?}", self.symbol_table_stack); - let table = self.symbol_table_stack.pop().unwrap(); - assert!(table.sub_tables.is_empty()); - } - - fn lookup_name(&self, name: &str) -> &Symbol { - // println!("Looking up {:?}", name); - let symbol_table = self.symbol_table_stack.last().unwrap(); - symbol_table.lookup(name).expect( - "The symbol must be present in the symbol table, even when it is undefined in python.", - ) - } - // Low level helper functions: fn emit(&mut self, instruction: Instruction) { let location = compile_location(&self.current_source_location); @@ -2402,9 +2455,11 @@ mod tests { use rustpython_parser::parser; fn compile_exec(source: &str) -> CodeObject { - let mut compiler: Compiler = - Compiler::new(CompileOpts::default(), "source_path".to_owned()); - compiler.push_new_code_object("".to_owned()); + let mut compiler: Compiler = Compiler::new( + CompileOpts::default(), + "source_path".to_owned(), + "".to_owned(), + ); let ast = parser::parse_program(source).unwrap(); let symbol_scope = make_symbol_table(&ast).unwrap(); compiler.compile_program(&ast, symbol_scope).unwrap(); diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 276e55c49c..02f15ae796 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -64,7 +64,7 @@ impl SymbolTable { } } -#[derive(Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum SymbolTableType { Module, Class, @@ -85,12 +85,14 @@ impl fmt::Display for SymbolTableType { /// Indicator for a single symbol what the scope of this symbol is. /// The scope can be unknown, which is unfortunate, but not impossible. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub enum SymbolScope { - Global, - Nonlocal, - Local, Unknown, + Local, + GlobalExplicit, + GlobalImplicit, + Free, + Cell, } /// A single symbol in a table. Has various properties such as the scope @@ -104,7 +106,6 @@ pub struct Symbol { pub is_referenced: bool, pub is_assigned: bool, pub is_parameter: bool, - pub is_free: bool, pub is_annotated: bool, pub is_imported: bool, @@ -126,7 +127,6 @@ impl Symbol { is_referenced: false, is_assigned: false, is_parameter: false, - is_free: false, is_annotated: false, is_imported: false, is_assign_namedexpr_in_comprehension: false, @@ -135,7 +135,10 @@ impl Symbol { } pub fn is_global(&self) -> bool { - matches!(self.scope, SymbolScope::Global) + matches!( + self.scope, + SymbolScope::GlobalExplicit | SymbolScope::GlobalImplicit + ) } pub fn is_local(&self) -> bool { @@ -228,7 +231,7 @@ impl<'a> SymbolTableAnalyzer<'a> { self.analyze_symbol_comprehension(symbol, 0)? } else { match symbol.scope { - SymbolScope::Nonlocal => { + SymbolScope::Free => { let scope_depth = self.tables.len(); if scope_depth > 0 { // check if the name is already defined in any outer scope @@ -251,10 +254,10 @@ impl<'a> SymbolTableAnalyzer<'a> { }); } } - SymbolScope::Global => { + SymbolScope::GlobalExplicit | SymbolScope::GlobalImplicit => { // TODO: add more checks for globals? } - SymbolScope::Local => { + SymbolScope::Local | SymbolScope::Cell => { // all is well } SymbolScope::Unknown => { @@ -270,24 +273,28 @@ impl<'a> SymbolTableAnalyzer<'a> { // Interesting stuff about the __class__ variable: // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object symbol.name == "__class__" - || self.tables.iter().skip(1).any(|(symbols, typ)| { - *typ != SymbolTableType::Class && symbols.contains_key(&symbol.name) + || self.tables.iter().skip(1).rev().any(|(symbols, typ)| { + *typ != SymbolTableType::Class + && symbols + .get(&symbol.name) + .map_or(false, |sym| sym.is_local() && sym.is_assigned) }) } fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { - if symbol.is_assigned || symbol.is_parameter { - symbol.scope = SymbolScope::Local; + let scope = if symbol.is_assigned || symbol.is_parameter { + SymbolScope::Local } else if self.found_in_outer_scope(symbol) { // Symbol is in some outer scope. - symbol.is_free = true; + SymbolScope::Free } else if self.tables.is_empty() { // Don't make assumptions when we don't know. - symbol.scope = SymbolScope::Unknown; + SymbolScope::Unknown } else { - // If there are scopes above we can assume global. - symbol.scope = SymbolScope::Global; - } + // If there are scopes above we assume global. + SymbolScope::GlobalImplicit + }; + symbol.scope = scope; } // Implements the symbol analysis and scope extension for names @@ -302,7 +309,7 @@ impl<'a> SymbolTableAnalyzer<'a> { // when this is called, we expect to be in the direct parent scope of the scope that contains 'symbol' let offs = self.tables.len() - 1 - parent_offset; let last = self.tables.get_mut(offs).unwrap(); - let symbols = &mut last.0; + let symbols = &mut *last.0; let table_type = last.1; // it is not allowed to use an iterator variable as assignee in a named expression @@ -319,7 +326,7 @@ impl<'a> SymbolTableAnalyzer<'a> { match table_type { SymbolTableType::Module => { - symbol.scope = SymbolScope::Global; + symbol.scope = SymbolScope::GlobalImplicit; } SymbolTableType::Class => { // named expressions are forbidden in comprehensions on class scope @@ -327,22 +334,18 @@ impl<'a> SymbolTableAnalyzer<'a> { error: "assignment expression within a comprehension cannot be used in a class body".to_string(), // TODO: accurate location info, somehow location: Location::default(), - } ); + }); } SymbolTableType::Function => { if let Some(parent_symbol) = symbols.get_mut(&symbol.name) { if let SymbolScope::Unknown = parent_symbol.scope { - parent_symbol.is_assigned = true; // this information is new, as the asignment is done in inner scope - //self.analyze_unknown_symbol(symbol); // not needed, symbol is analyzed anyhow when its scope is analyzed + // this information is new, as the asignment is done in inner scope + parent_symbol.is_assigned = true; + //self.analyze_unknown_symbol(symbol); // not needed, symbol is analyzed anyhow when its scope is analyzed } - match symbol.scope { - SymbolScope::Global => { - symbol.scope = SymbolScope::Global; - } - _ => { - symbol.scope = SymbolScope::Nonlocal; - } + if !symbol.is_global() { + symbol.scope = SymbolScope::Free; } } else { let mut cloned_sym = symbol.clone(); @@ -372,7 +375,7 @@ impl<'a> SymbolTableAnalyzer<'a> { // ouside, too, and set it therefore to non-local scope. I.e., we expect to // find a definition on a higher level let mut cloned_sym = symbol.clone(); - cloned_sym.scope = SymbolScope::Nonlocal; + cloned_sym.scope = SymbolScope::Free; last.0.insert(cloned_sym.name.to_owned(), cloned_sym); } } @@ -408,7 +411,7 @@ struct SymbolTableBuilder { /// was used. /// In cpython this is stored in the AST, but I think this /// is not logical, since it is not context free. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq)] enum ExpressionContext { Load, Store, @@ -825,6 +828,12 @@ impl SymbolTableBuilder { self.register_name(name, SymbolUsage::Iter, location)?; } } + if context == ExpressionContext::Load + && self.tables.last().unwrap().typ == SymbolTableType::Function + && name == "super" + { + self.register_name("__class__", SymbolUsage::Used, location)?; + } } Lambda { args, body } => { self.enter_function("lambda", args, expression.location.row())?; @@ -949,7 +958,7 @@ impl SymbolTableBuilder { // Role already set.. match role { SymbolUsage::Global => { - if let SymbolScope::Global = symbol.scope { + if symbol.is_global() { // Ok } else { return Err(SymbolTableError { @@ -1014,7 +1023,7 @@ impl SymbolTableBuilder { let symbol = table.symbols.get_mut(name).unwrap(); match role { SymbolUsage::Nonlocal => { - symbol.scope = SymbolScope::Nonlocal; + symbol.scope = SymbolScope::Free; } SymbolUsage::Imported => { symbol.is_assigned = true; @@ -1040,8 +1049,8 @@ impl SymbolTableBuilder { } SymbolUsage::Global => { if let SymbolScope::Unknown = symbol.scope { - symbol.scope = SymbolScope::Global; - } else if let SymbolScope::Global = symbol.scope { + symbol.scope = SymbolScope::GlobalImplicit; + } else if symbol.is_global() { // Global scope can be set to global } else { return Err(SymbolTableError { diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index 04b6241ff7..a599102bc4 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -500,19 +500,18 @@ where self.as_object().get_item(key, vm) } - fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { + fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { if self.class().is(&vm.ctx.types.dict_type) { self.inner_setitem_fast(key, value, vm) - .map(|_| vm.ctx.none()) } else { // Fall back to slow path if we are in a dict subclass: self.as_object().set_item(key, value, vm) } } - fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult { + fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult<()> { if self.class().is(&vm.ctx.types.dict_type) { - self.entries.delete(vm, key).map(|_| vm.ctx.none()) + self.entries.delete(vm, key) } else { // Fall back to slow path if we are in a dict subclass: self.as_object().del_item(key, vm) diff --git a/vm/src/builtins/frame.rs b/vm/src/builtins/frame.rs index 947dd673a6..3203c725f6 100644 --- a/vm/src/builtins/frame.rs +++ b/vm/src/builtins/frame.rs @@ -44,12 +44,12 @@ impl FrameRef { #[pyproperty] fn f_globals(self) -> PyDictRef { - self.scope.globals.clone() + self.globals.clone() } #[pyproperty] - fn f_locals(self) -> PyDictRef { - self.scope.get_locals().clone() + fn f_locals(self, vm: &VirtualMachine) -> PyDictRef { + self.locals(vm).clone() } #[pyproperty] diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index a62759b6c3..2455fa396a 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -5,7 +5,7 @@ use super::code::PyCodeRef; use super::dict::PyDictRef; use super::pystr::PyStrRef; use super::pytype::PyTypeRef; -use super::tuple::PyTupleRef; +use super::tuple::{PyTupleRef, PyTupleTyped}; use crate::builtins::asyncgenerator::PyAsyncGen; use crate::builtins::coroutine::PyCoroutine; use crate::builtins::generator::PyGenerator; @@ -36,7 +36,8 @@ pub struct PyFunction { code: PyCodeRef, #[cfg(feature = "jit")] jitted_code: OnceCell, - scope: Scope, + globals: PyDictRef, + closure: Option>, defaults: Option, kw_only_defaults: Option, } @@ -44,7 +45,8 @@ pub struct PyFunction { impl PyFunction { pub fn new( code: PyCodeRef, - scope: Scope, + globals: PyDictRef, + closure: Option>, defaults: Option, kw_only_defaults: Option, ) -> Self { @@ -52,25 +54,23 @@ impl PyFunction { code, #[cfg(feature = "jit")] jitted_code: OnceCell::new(), - scope, + globals, + closure, defaults, kw_only_defaults, } } - pub fn scope(&self) -> &Scope { - &self.scope - } - fn fill_locals_from_args( &self, - locals: &PyDictRef, + frame: &Frame, func_args: FuncArgs, vm: &VirtualMachine, ) -> PyResult<()> { + let code = &*self.code; let nargs = func_args.args.len(); - let nexpected_args = self.code.arg_count; - let arg_names = self.code.arg_names(); + let nexpected_args = code.arg_count; + // let arg_names = self.code.arg_names(); // This parses the arguments from args and kwargs into // the proper variables keeping into account default values @@ -78,23 +78,24 @@ impl PyFunction { // See also: PyEval_EvalCodeWithName in cpython: // https://github.com/python/cpython/blob/master/Python/ceval.c#L3681 - let n = if nargs > nexpected_args { - nexpected_args - } else { - nargs - }; + let n = std::cmp::min(nargs, nexpected_args); let mut args_iter = func_args.args.into_iter(); + let mut fastlocals = frame.fastlocals.lock(); + // let mut fill_locals = fastlocals.iter_mut().enumerate(); + // Copy positional arguments into local variables - for (arg, arg_name) in args_iter.by_ref().take(n).zip(arg_names.args) { - locals.set_item(arg_name.clone(), arg, vm)?; + for (i, arg) in args_iter.by_ref().take(n).enumerate() { + fastlocals[i] = Some(arg); } + let mut vararg_offset = code.arg_count; // Pack other positional arguments in to *args: - if let Some(vararg_name) = arg_names.vararg { + if code.flags.contains(bytecode::CodeFlags::HAS_VARARGS) { let vararg_value = vm.ctx.new_tuple(args_iter.collect()); - locals.set_item(vararg_name.clone(), vararg_value, vm)?; + fastlocals[vararg_offset] = Some(vararg_value); + vararg_offset += 1; } else { // Check the number of positional arguments if nargs > nexpected_args { @@ -106,25 +107,25 @@ impl PyFunction { } // Do we support `**kwargs` ? - let kwargs = if let Some(varkwarg) = arg_names.varkwarg { + let kwargs = if code.flags.contains(bytecode::CodeFlags::HAS_VARKEYWORDS) { let d = vm.ctx.new_dict(); - locals.set_item(varkwarg.clone(), d.as_object().clone(), vm)?; + fastlocals[vararg_offset] = Some(d.clone()); Some(d) } else { None }; - let contains_arg = - |names: &[PyStrRef], name: &str| names.iter().any(|s| s.borrow_value() == name); + // let argpos = + // |names: &[PyStrRef], name: &str| names.iter().position(|s| s.borrow_value() == name); let mut posonly_passed_as_kwarg = Vec::new(); // Handle keyword arguments for (name, value) in func_args.kwargs { // Check if we have a parameter with this name: - let dict = if contains_arg(arg_names.args, &name) - || contains_arg(arg_names.kwonlyargs, &name) + let dict = if let Some(pos) = + argpos(arg_names.args, &name).or_else(|| argpos(arg_names.kwonlyargs, &name)) { - if contains_arg(arg_names.posonlyargs, &name) { + if argpos(arg_names.posonlyargs, &name) { posonly_passed_as_kwarg.push(name); continue; } else if locals.contains_key(&name, vm) { @@ -138,7 +139,7 @@ impl PyFunction { vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name)) })? }; - dict.set_item(vm.intern_string(name).into_object(), value, vm)?; + // dict.set_item(vm.intern_string(name).into_object(), value, vm)?; } if !posonly_passed_as_kwarg.is_empty() { return Err(vm.new_type_error(format!( @@ -204,10 +205,10 @@ impl PyFunction { Ok(()) } - pub fn invoke_with_scope( + pub fn invoke_with_locals( &self, func_args: FuncArgs, - scope: &Scope, + locals: Option, vm: &VirtualMachine, ) -> PyResult { #[cfg(feature = "jit")] @@ -226,16 +227,22 @@ impl PyFunction { let code = &self.code; - let scope = if self.code.flags.contains(bytecode::CodeFlags::NEW_LOCALS) { - scope.new_child_scope(&vm.ctx) + let locals = if self.code.flags.contains(bytecode::CodeFlags::NEW_LOCALS) { + vm.ctx.new_dict() } else { - scope.clone() + locals.unwrap_or_else(|| self.globals.clone()) }; - self.fill_locals_from_args(&scope.get_locals(), func_args, vm)?; - // Construct frame: - let frame = Frame::new(code.clone(), scope, vm).into_ref(vm); + let frame = Frame::new( + code.clone(), + Scope::new(Some(locals), self.globals.clone()), + self.closure.as_ref().map_or(&[], |c| c.borrow_value()), + vm, + ) + .into_ref(vm); + + self.fill_locals_from_args(&frame, func_args, vm)?; // If we have a generator, create a new generator let is_gen = code.flags.contains(bytecode::CodeFlags::IS_GENERATOR); @@ -249,7 +256,7 @@ impl PyFunction { } pub fn invoke(&self, func_args: FuncArgs, vm: &VirtualMachine) -> PyResult { - self.invoke_with_scope(func_args, &self.scope, vm) + self.invoke_with_locals(func_args, None, vm) } } @@ -278,7 +285,7 @@ impl PyFunction { #[pyproperty(magic)] fn globals(&self) -> PyDictRef { - self.scope.globals.clone() + self.globals.clone() } #[cfg(feature = "jit")] @@ -393,10 +400,11 @@ impl PyValue for PyBoundMethod { } #[pyclass(module = false, name = "cell")] -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct PyCell { contents: PyMutex>, } +pub(crate) type PyCellRef = PyRef; impl PyValue for PyCell { fn class(vm: &VirtualMachine) -> &PyTypeRef { diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index 1b2396bb12..dcaab34bcb 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -172,7 +172,7 @@ mod decl { fn dir(obj: OptionalArg, vm: &VirtualMachine) -> PyResult { let seq = match obj { OptionalArg::Present(obj) => vm.call_method(&obj, "__dir__", ())?, - OptionalArg::Missing => vm.call_method(&vm.get_locals().into_object(), "keys", ())?, + OptionalArg::Missing => vm.call_method(vm.current_locals().as_object(), "keys", ())?, }; let sorted = sorted(seq, Default::default(), vm)?; Ok(sorted) @@ -243,14 +243,13 @@ mod decl { #[cfg(feature = "rustpython-compiler")] fn make_scope(vm: &VirtualMachine, scope: ScopeArgs) -> PyResult { let globals = scope.globals; - let current_scope = vm.current_scope(); let locals = match scope.locals { Some(dict) => Some(dict), None => { if globals.is_some() { None } else { - current_scope.get_only_locals() + Some(vm.current_locals().clone()) } } }; @@ -262,7 +261,7 @@ mod decl { } dict } - None => current_scope.globals.clone(), + None => vm.current_globals().clone(), }; let scope = Scope::with_builtins(locals, globals, vm); @@ -318,7 +317,7 @@ mod decl { #[pyfunction] fn globals(vm: &VirtualMachine) -> PyResult { - Ok(vm.current_scope().globals.clone()) + Ok(vm.current_globals().clone()) } #[pyfunction] @@ -449,7 +448,7 @@ mod decl { #[pyfunction] fn locals(vm: &VirtualMachine) -> PyDictRef { - let locals = vm.get_locals(); + let locals = vm.current_locals(); locals.copy().into_ref(vm) } @@ -797,7 +796,7 @@ mod decl { if let OptionalArg::Present(obj) = obj { vm.get_attribute(obj, "__dict__") } else { - Ok(vm.get_locals().into_object()) + Ok(vm.current_locals().clone().into_object()) } } @@ -843,20 +842,13 @@ mod decl { let namespace: PyDictRef = TryFromObject::try_from_object(vm, namespace)?; - let cells = vm.ctx.new_dict(); - - let scope = function - .scope() - .new_child_scope_with_locals(cells.clone()) - .new_child_scope_with_locals(namespace.clone()); - - function.invoke_with_scope(().into(), &scope, vm)?; + function.invoke_with_locals(().into(), Some(namespace), vm)?; let class = vm.invoke( metaclass.as_object(), FuncArgs::new(vec![name_obj, bases, namespace.into_object()], kwargs), )?; - cells.set_item("__class__", class.clone(), vm)?; + // cells.set_item("__class__", class.clone(), vm)?; Ok(class) } } diff --git a/vm/src/builtins/pysuper.rs b/vm/src/builtins/pysuper.rs index b48c25c9da..f1950ee0a5 100644 --- a/vm/src/builtins/pysuper.rs +++ b/vm/src/builtins/pysuper.rs @@ -61,16 +61,30 @@ impl PySuper { let typ = if let OptionalArg::Present(ty) = py_type { ty } else { - let obj = vm - .current_scope() - .load_cell(vm, "__class__") - .ok_or_else(|| { - vm.new_type_error( - "super must be called with 1 argument or from inside class method" - .to_owned(), - ) - })?; - PyTypeRef::try_from_object(vm, obj)? + let frame = vm + .current_frame() + .expect("super() called from outside of frame?"); + let mut typ = None; + for (i, var) in frame.code.freevars.iter().enumerate() { + if var.borrow_value() == "__class__" { + let i = frame.code.cellvars.len() + i; + let class = frame.cells_frees[i].get().ok_or_else(|| { + vm.new_runtime_error("super(): empty __class__ cell".to_owned()) + })?; + typ = Some(class.downcast().map_err(|o| { + vm.new_type_error(format!( + "super(): __class__ is not a type ({})", + o.class().name + )) + })?); + break; + } + } + typ.ok_or_else(|| { + vm.new_type_error( + "super must be called with 1 argument or from inside class method".to_owned(), + ) + })? }; // Check type argument: @@ -87,8 +101,8 @@ impl PySuper { } else { let frame = vm.current_frame().expect("no current frame for super()"); if let Some(first_arg) = frame.code.arg_names().args.get(0) { - let locals = frame.scope.get_locals(); - locals + frame + .locals .get_item_option(first_arg.clone(), vm)? .ok_or_else(|| { vm.new_type_error(format!("super argument {} was not supplied", first_arg)) diff --git a/vm/src/builtins/tuple.rs b/vm/src/builtins/tuple.rs index a47d8c69a1..74991c69db 100644 --- a/vm/src/builtins/tuple.rs +++ b/vm/src/builtins/tuple.rs @@ -294,7 +294,7 @@ pub(crate) fn init(context: &PyContext) { PyTupleIterator::extend_class(context, &context.types.tuple_iterator_type); } -pub struct PyTupleTyped { +pub struct PyTupleTyped { // SAFETY INVARIANT: T must be repr(transparent) over PyObjectRef, and the // elements must be logically valid when transmuted to T tuple: PyTupleRef, @@ -315,9 +315,15 @@ impl TryFromObject for PyTupleTyped { } } -impl<'a, T: 'a> BorrowValue<'a> for PyTupleTyped { +impl<'a, T: TransmuteFromObject + 'a> BorrowValue<'a> for PyTupleTyped { type Borrowed = &'a [T]; fn borrow_value(&'a self) -> Self::Borrowed { unsafe { &*(self.tuple.borrow_value() as *const [PyObjectRef] as *const [T]) } } } + +impl fmt::Debug for PyTupleTyped { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.borrow_value().fmt(f) + } +} diff --git a/vm/src/frame.rs b/vm/src/frame.rs index fee9e595ce..8c504d3ae2 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -9,12 +9,13 @@ use crate::builtins::asyncgenerator::PyAsyncGenWrappedValue; use crate::builtins::code::PyCodeRef; use crate::builtins::coroutine::PyCoroutine; use crate::builtins::dict::{PyDict, PyDictRef}; +use crate::builtins::function::{PyCell, PyCellRef, PyFunction}; use crate::builtins::generator::PyGenerator; use crate::builtins::pystr::{self, PyStr, PyStrRef}; use crate::builtins::pytype::PyTypeRef; use crate::builtins::slice::PySlice; use crate::builtins::traceback::PyTraceback; -use crate::builtins::tuple::PyTuple; +use crate::builtins::tuple::{PyTuple, PyTupleTyped}; use crate::builtins::{list, pybool, set}; use crate::bytecode; use crate::common::lock::PyMutex; @@ -91,7 +92,12 @@ struct FrameState { #[pyclass(module = false, name = "frame")] pub struct Frame { pub code: PyCodeRef, - pub scope: Scope, + + pub fastlocals: PyMutex]>>, + pub cells_frees: Box<[PyCellRef]>, + pub locals: PyDictRef, + pub globals: PyDictRef, + /// index of last instruction ran pub lasti: AtomicUsize, /// tracer function for this frame (usually is None) @@ -151,7 +157,7 @@ impl ExecutionResult { pub type FrameResult = PyResult>; impl Frame { - pub fn new(code: PyCodeRef, scope: Scope, vm: &VirtualMachine) -> Frame { + pub fn new(code: PyCodeRef, scope: Scope, closure: &[PyCellRef], vm: &VirtualMachine) -> Frame { //populate the globals and locals //TODO: This is wrong, check https://github.com/nedbat/byterun/blob/31e6c4a8212c35b5157919abff43a7daa0f377c6/byterun/pyvm2.py#L95 /* @@ -162,10 +168,17 @@ impl Frame { */ // let locals = globals; // locals.extend(callargs); + let cells_frees = std::iter::repeat_with(|| PyCell::default().into_ref(vm)) + .take(code.cellvars.len()) + .chain(closure.iter().cloned()) + .collect(); Frame { + fastlocals: PyMutex::new(vec![None; code.varnames.len()].into_boxed_slice()), + cells_frees, + locals: scope.locals, + globals: scope.globals, code, - scope, lasti: AtomicUsize::new(0), state: PyMutex::new(FrameState { stack: Vec::new(), @@ -182,7 +195,10 @@ impl FrameRef { let mut state = self.state.lock(); let exec = ExecutingFrame { code: &self.code, - scope: &self.scope, + fastlocals: &self.fastlocals, + cells_frees: &self.cells_frees, + locals: &self.locals, + globals: &self.globals, lasti: &self.lasti, object: &self, state: &mut state, @@ -190,6 +206,10 @@ impl FrameRef { f(exec) } + pub fn locals(&self, vm: &VirtualMachine) -> &PyDictRef { + todo!() + } + // #[cfg_attr(feature = "flame-it", flame("Frame"))] pub fn run(&self, vm: &VirtualMachine) -> PyResult { self.with_exec(|mut exec| exec.run(vm)) @@ -233,7 +253,10 @@ impl FrameRef { /// with the mutable data inside struct ExecutingFrame<'a> { code: &'a PyCodeRef, - scope: &'a Scope, + fastlocals: &'a PyMutex]>>, + cells_frees: &'a [PyCellRef], + locals: &'a PyDictRef, + globals: &'a PyDictRef, object: &'a FrameRef, lasti: &'a AtomicUsize, state: &'a mut FrameState, @@ -243,7 +266,7 @@ impl fmt::Debug for ExecutingFrame<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("ExecutingFrame") .field("code", self.code) - .field("scope", self.scope) + // .field("scope", self.scope) .field("lasti", self.lasti) .field("state", self.state) .finish() @@ -330,6 +353,24 @@ impl ExecutingFrame<'_> { } } + fn unbound_cell_exception(&self, i: usize, vm: &VirtualMachine) -> PyBaseExceptionRef { + if let Some(name) = self.code.cellvars.get(i) { + vm.new_exception_msg( + vm.ctx.exceptions.unbound_local_error.clone(), + format!( + "local variable '{}' referenced before assignment", + self.code.cellvars[i] + ), + ) + } else { + let name = &self.code.freevars[i - self.code.cellvars.len()]; + vm.new_name_error(format!( + "free variable '{}' referenced before assignment in enclosing scope", + name + )) + } + } + /// Execute a single instruction. #[inline(always)] fn execute_instruction( @@ -366,9 +407,100 @@ impl ExecutingFrame<'_> { } => self.import(vm, *name_idx, symbols_idx, *level), bytecode::Instruction::ImportStar => self.import_star(vm), bytecode::Instruction::ImportFrom { idx } => self.import_from(vm, *idx), - bytecode::Instruction::LoadName { idx, scope } => self.load_name(vm, *idx, *scope), - bytecode::Instruction::StoreName { idx, scope } => self.store_name(vm, *idx, *scope), - bytecode::Instruction::DeleteName { idx } => self.delete_name(vm, *idx), + bytecode::Instruction::LoadFast(idx) => { + let x = self.fastlocals.lock()[*idx].clone().ok_or_else(|| { + vm.new_exception_msg( + vm.ctx.exceptions.unbound_local_error.clone(), + format!( + "local variable '{}' referenced before assignment", + self.code.varnames[*idx] + ), + ) + })?; + self.push_value(x); + Ok(None) + } + bytecode::Instruction::LoadLocal(idx) => { + let name = &self.code.names[*idx]; + let x = self + .locals + .get_item_option(name.clone(), vm)? + .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; + self.push_value(x); + Ok(None) + } + bytecode::Instruction::LoadGlobal(idx) => { + let name = &self.code.names[*idx]; + let x = self + .globals + .get_item_option(name.clone(), vm)? + .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; + self.push_value(x); + Ok(None) + } + bytecode::Instruction::LoadDeref(i) => { + let i = *i; + self.cells_frees[i] + .get() + .ok_or_else(|| self.unbound_cell_exception(i, vm))?; + Ok(None) + } + bytecode::Instruction::LoadClassDeref(i) => { + let i = *i; + let name = self.code.freevars[i - self.code.cellvars.len()].clone(); + let value = if let Some(value) = self.locals.get_item_option(name, vm)? { + value + } else { + self.cells_frees[i] + .get() + .ok_or_else(|| self.unbound_cell_exception(i, vm))? + }; + self.push_value(value); + Ok(None) + } + bytecode::Instruction::StoreFast(idx) => { + let value = self.pop_value(); + self.fastlocals.lock()[*idx] = Some(value); + Ok(None) + } + bytecode::Instruction::StoreLocal(idx) => { + let value = self.pop_value(); + self.locals + .set_item(self.code.names[*idx].clone(), value, vm)?; + Ok(None) + } + bytecode::Instruction::StoreGlobal(idx) => { + let value = self.pop_value(); + self.globals + .set_item(self.code.names[*idx].clone(), value, vm)?; + Ok(None) + } + bytecode::Instruction::StoreDeref(i) => { + let value = self.pop_value(); + self.cells_frees[*i].set(Some(value)); + Ok(None) + } + bytecode::Instruction::DeleteFast(idx) => { + self.fastlocals.lock()[*idx] = None; + Ok(None) + } + bytecode::Instruction::DeleteLocal(idx) => { + self.locals.del_item(self.code.names[*idx].clone(), vm)?; + Ok(None) + } + bytecode::Instruction::DeleteGlobal(idx) => { + self.globals.del_item(self.code.names[*idx].clone(), vm)?; + Ok(None) + } + bytecode::Instruction::DeleteDeref(i) => { + self.cells_frees[*i].set(None); + Ok(None) + } + bytecode::Instruction::LoadClosure(i) => { + let value = self.cells_frees[*i].clone(); + self.push_value(value.into_object()); + Ok(None) + } bytecode::Instruction::Subscript => self.execute_subscript(vm), bytecode::Instruction::StoreSubscript => self.execute_store_subscript(vm), bytecode::Instruction::DeleteSubscript => self.execute_delete_subscript(vm), @@ -472,9 +604,9 @@ impl ExecutingFrame<'_> { } bytecode::Instruction::YieldFrom => self.execute_yield_from(vm), bytecode::Instruction::SetupAnnotation => { - let locals = self.scope.get_locals(); - if !locals.contains_key("__annotations__", vm) { - locals.set_item("__annotations__", vm.ctx.new_dict().into_object(), vm)?; + if !self.locals.contains_key("__annotations__", vm) { + self.locals + .set_item("__annotations__", vm.ctx.new_dict().into_object(), vm)?; } Ok(None) } @@ -826,9 +958,8 @@ impl ExecutingFrame<'_> { }; for (k, v) in &dict { let k = PyStrRef::try_from_object(vm, k)?; - let k = k.as_ref(); - if filter_pred(k) { - self.scope.store_name(&vm, k, v); + if filter_pred(k.borrow_value()) { + self.locals.set_item(k, v, vm); } } } @@ -904,61 +1035,6 @@ impl ExecutingFrame<'_> { } } - fn store_name( - &mut self, - vm: &VirtualMachine, - idx: bytecode::NameIdx, - name_scope: bytecode::NameScope, - ) -> FrameResult { - let name = self.code.names[idx].clone(); - let obj = self.pop_value(); - match name_scope { - bytecode::NameScope::Global => { - self.scope.store_global(vm, name, obj); - } - bytecode::NameScope::NonLocal => { - self.scope.store_cell(vm, name, obj); - } - bytecode::NameScope::Local => { - self.scope.store_name(vm, name, obj); - } - bytecode::NameScope::Free => { - self.scope.store_name(vm, name, obj); - } - } - Ok(None) - } - - fn delete_name(&self, vm: &VirtualMachine, idx: bytecode::NameIdx) -> FrameResult { - let name = &self.code.names[idx]; - match self.scope.delete_name(vm, name.clone()) { - Ok(_) => Ok(None), - Err(_) => Err(vm.new_name_error(format!("name '{}' is not defined", name))), - } - } - - #[cfg_attr(feature = "flame-it", flame("Frame"))] - #[inline] - fn load_name( - &mut self, - vm: &VirtualMachine, - idx: bytecode::NameIdx, - name_scope: bytecode::NameScope, - ) -> FrameResult { - let name = &self.code.names[idx]; - let optional_value = match name_scope { - bytecode::NameScope::Global => self.scope.load_global(vm, name.clone()), - bytecode::NameScope::NonLocal => self.scope.load_cell(vm, name.clone()), - bytecode::NameScope::Local => self.scope.load_local(vm, name.clone()), - bytecode::NameScope::Free => self.scope.load_name(vm, name.clone()), - }; - - let value = optional_value - .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; - self.push_value(value); - Ok(None) - } - fn execute_rotate(&mut self, amount: usize) -> FrameResult { // Shuffles top of stack amount down if amount < 2 { @@ -1278,6 +1354,12 @@ impl ExecutingFrame<'_> { let flags = code_obj.flags; + let closure = if code_obj.freevars.is_empty() { + None + } else { + Some(PyTupleTyped::try_from_object(vm, self.pop_value()).unwrap()) + }; + let annotations = if flags.contains(bytecode::CodeFlags::HAS_ANNOTATIONS) { self.pop_value() } else { @@ -1306,10 +1388,15 @@ impl ExecutingFrame<'_> { // pop argc arguments // argument: name, args, globals - let scope = self.scope.clone(); - let func_obj = vm - .ctx - .new_pyfunction(code_obj, scope, defaults, kw_only_defaults); + // let scope = self.scope.clone(); + let func_obj = PyFunction::new( + code_obj, + self.globals.clone(), + closure, + defaults, + kw_only_defaults, + ) + .into_object(vm); vm.set_attr(&func_obj, "__doc__", vm.ctx.none())?; @@ -1320,7 +1407,7 @@ impl ExecutingFrame<'_> { .unwrap(); vm.set_attr(&func_obj, "__name__", vm.ctx.new_str(name))?; vm.set_attr(&func_obj, "__qualname__", qualified_name)?; - let module = vm.unwrap_or_none(self.scope.globals.get_item_option("__name__", vm)?); + let module = vm.unwrap_or_none(self.globals.get_item_option("__name__", vm)?); vm.set_attr(&func_obj, "__module__", module)?; vm.set_attr(&func_obj, "__annotations__", annotations)?; @@ -1546,7 +1633,8 @@ impl fmt::Debug for Frame { .iter() .map(|elem| format!("\n > {:?}", elem)) .collect::(); - let dict = self.scope.get_locals(); + // TODO: fix this up + let dict = self.locals.clone(); let local_str = dict .into_iter() .map(|elem| format!("\n {:?} = {:?}", elem.0, elem.1)) diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 03dc97d718..90f4ca583a 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -348,20 +348,6 @@ impl PyContext { PyRef::new_ref(code::PyCode::new(code), self.types.code_type.clone(), None) } - pub fn new_pyfunction( - &self, - code_obj: PyCodeRef, - scope: Scope, - defaults: Option, - kw_only_defaults: Option, - ) -> PyObjectRef { - PyObject::new( - PyFunction::new(code_obj, scope, defaults, kw_only_defaults), - self.types.function_type.clone(), - Some(self.new_dict()), - ) - } - pub fn new_bound_method(&self, function: PyObjectRef, object: PyObjectRef) -> PyObjectRef { PyObject::new( PyBoundMethod::new(object, function), @@ -637,8 +623,8 @@ where T: IntoPyObject + ?Sized, { fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult; - fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult; - fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult; + fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()>; + fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()>; } impl ItemProtocol for PyObjectRef @@ -649,12 +635,12 @@ where vm.call_method(self, "__getitem__", (key,)) } - fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult { - vm.call_method(self, "__setitem__", (key, value)) + fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + vm.call_method(self, "__setitem__", (key, value)).map(drop) } - fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult { - vm.call_method(self, "__delitem__", (key,)) + fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult<()> { + vm.call_method(self, "__delitem__", (key,)).map(drop) } } diff --git a/vm/src/scope.rs b/vm/src/scope.rs index e770279263..ed6da13509 100644 --- a/vm/src/scope.rs +++ b/vm/src/scope.rs @@ -1,17 +1,13 @@ use std::fmt; use crate::builtins::{PyDictRef, PyStr, PyStrRef}; +use crate::common::lock::PyMutex; use crate::pyobject::{IntoPyObject, ItemProtocol, PyContext, PyObjectRef, PyResult, TryIntoRef}; use crate::VirtualMachine; -/* - * So a scope is a linked list of scopes. - * When a name is looked up, it is check in its scope. - */ -#[derive(Clone)] pub struct Scope { - locals: Vec, - pub globals: PyDictRef, + pub(crate) locals: PyDictRef, + pub(crate) globals: PyDictRef, } impl fmt::Debug for Scope { @@ -22,14 +18,10 @@ impl fmt::Debug for Scope { } impl Scope { - pub fn new(locals: Option, globals: PyDictRef, vm: &VirtualMachine) -> Scope { - let locals = match locals { - Some(dict) => vec![dict], - None => vec![], - }; - let scope = Scope { locals, globals }; - scope.store_name(vm, "__annotations__", vm.ctx.new_dict().into_object()); - scope + #[inline] + pub fn new(locals: Option, globals: PyDictRef) -> Scope { + let locals = locals.unwrap_or_else(|| globals.clone()); + Scope { locals, globals } } pub fn with_builtins( @@ -42,108 +34,108 @@ impl Scope { .set_item("__builtins__", vm.builtins.clone(), vm) .unwrap(); } - Scope::new(locals, globals, vm) + Scope::new(locals, globals) } - pub fn get_locals(&self) -> &PyDictRef { - match self.locals.first() { - Some(dict) => dict, - None => &self.globals, - } - } - - pub fn get_only_locals(&self) -> Option { - self.locals.first().cloned() - } - - pub fn new_child_scope_with_locals(&self, locals: PyDictRef) -> Scope { - let mut new_locals = Vec::with_capacity(self.locals.len() + 1); - new_locals.push(locals); - new_locals.extend_from_slice(&self.locals); - Scope { - locals: new_locals, - globals: self.globals.clone(), - } - } - - pub fn new_child_scope(&self, ctx: &PyContext) -> Scope { - self.new_child_scope_with_locals(ctx.new_dict()) - } - - #[cfg_attr(feature = "flame-it", flame("Scope"))] - pub fn load_name(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - for dict in self.locals.iter() { - if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { - return Some(value); - } - } - - // Fall back to loading a global after all scopes have been searched! - self.load_global(vm, name) - } - - #[cfg_attr(feature = "flame-it", flame("Scope"))] - /// Load a local name. Only check the local dictionary for the given name. - pub fn load_local(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - self.get_locals().get_item_option(name, vm).unwrap() - } - - #[cfg_attr(feature = "flame-it", flame("Scope"))] - pub fn load_cell(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - for dict in self.locals.iter().skip(1) { - if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { - return Some(value); - } - } - None - } - - pub fn store_cell(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { - // find the innermost outer scope that contains the symbol name - if let Some(locals) = self - .locals - .iter() - .rev() - .find(|l| l.contains_key(name.clone(), vm)) - { - // add to the symbol - locals.set_item(name, value, vm).unwrap(); - } else { - // somewhat limited solution -> fallback to the old rustpython strategy - // and store the next outer scope - // This case is usually considered as a failure case, but kept for the moment - // to support the scope propagation for named expression assignments to so far - // unknown names in comprehensions. We need to consider here more context - // information for correct handling. - self.locals - .get(1) - .expect("no outer scope for non-local") - .set_item(name, value, vm) - .unwrap(); - } - } - - pub fn store_name(&self, vm: &VirtualMachine, key: impl PyName, value: PyObjectRef) { - self.get_locals().set_item(key, value, vm).unwrap(); - } - - pub fn delete_name(&self, vm: &VirtualMachine, key: impl PyName) -> PyResult { - self.get_locals().del_item(key, vm) - } - - #[cfg_attr(feature = "flame-it", flame("Scope"))] - /// Load a global name. - pub fn load_global(&self, vm: &VirtualMachine, name: impl PyName) -> Option { - if let Some(value) = self.globals.get_item_option(name.clone(), vm).unwrap() { - Some(value) - } else { - vm.get_attribute(vm.builtins.clone(), name).ok() - } - } - - pub fn store_global(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { - self.globals.set_item(name, value, vm).unwrap(); - } + // pub fn get_locals(&self) -> &PyDictRef { + // match self.locals.first() { + // Some(dict) => dict, + // None => &self.globals, + // } + // } + + // pub fn get_only_locals(&self) -> Option { + // self.locals.first().cloned() + // } + + // pub fn new_child_scope_with_locals(&self, locals: PyDictRef) -> Scope { + // let mut new_locals = Vec::with_capacity(self.locals.len() + 1); + // new_locals.push(locals); + // new_locals.extend_from_slice(&self.locals); + // Scope { + // locals: new_locals, + // globals: self.globals.clone(), + // } + // } + + // pub fn new_child_scope(&self, ctx: &PyContext) -> Scope { + // self.new_child_scope_with_locals(ctx.new_dict()) + // } + + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // pub fn load_name(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // for dict in self.locals.iter() { + // if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { + // return Some(value); + // } + // } + + // // Fall back to loading a global after all scopes have been searched! + // self.load_global(vm, name) + // } + + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // /// Load a local name. Only check the local dictionary for the given name. + // pub fn load_local(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // self.get_locals().get_item_option(name, vm).unwrap() + // } + + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // pub fn load_cell(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // for dict in self.locals.iter().skip(1) { + // if let Some(value) = dict.get_item_option(name.clone(), vm).unwrap() { + // return Some(value); + // } + // } + // None + // } + + // pub fn store_cell(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { + // // find the innermost outer scope that contains the symbol name + // if let Some(locals) = self + // .locals + // .iter() + // .rev() + // .find(|l| l.contains_key(name.clone(), vm)) + // { + // // add to the symbol + // locals.set_item(name, value, vm).unwrap(); + // } else { + // // somewhat limited solution -> fallback to the old rustpython strategy + // // and store the next outer scope + // // This case is usually considered as a failure case, but kept for the moment + // // to support the scope propagation for named expression assignments to so far + // // unknown names in comprehensions. We need to consider here more context + // // information for correct handling. + // self.locals + // .get(1) + // .expect("no outer scope for non-local") + // .set_item(name, value, vm) + // .unwrap(); + // } + // } + + // pub fn store_name(&self, vm: &VirtualMachine, key: impl PyName, value: PyObjectRef) { + // self.get_locals().set_item(key, value, vm).unwrap(); + // } + + // pub fn delete_name(&self, vm: &VirtualMachine, key: impl PyName) -> PyResult { + // self.get_locals().del_item(key, vm) + // } + + // #[cfg_attr(feature = "flame-it", flame("Scope"))] + // /// Load a global name. + // pub fn load_global(&self, vm: &VirtualMachine, name: impl PyName) -> Option { + // if let Some(value) = self.globals.get_item_option(name.clone(), vm).unwrap() { + // Some(value) + // } else { + // vm.get_attribute(vm.builtins.clone(), name).ok() + // } + // } + + // pub fn store_global(&self, vm: &VirtualMachine, name: impl PyName, value: PyObjectRef) { + // self.globals.set_item(name, value, vm).unwrap(); + // } } mod sealed { diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index e645dc1dee..b6825a6f31 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -205,7 +205,7 @@ mod decl { #[pymethod(name = "is_nonlocal")] fn is_nonlocal(&self) -> bool { - matches!(self.symbol.scope, SymbolScope::Nonlocal) + matches!(self.symbol.scope, SymbolScope::Free) } #[pymethod(name = "is_referenced")] @@ -225,7 +225,7 @@ mod decl { #[pymethod(name = "is_free")] fn is_free(&self) -> bool { - self.symbol.is_free + matches!(self.symbol.scope, SymbolScope::Free) } #[pymethod(name = "is_namespace")] diff --git a/vm/src/vm.rs b/vm/src/vm.rs index ce1baf5a87..8a114e9765 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -448,7 +448,7 @@ impl VirtualMachine { } pub fn run_code_obj(&self, code: PyCodeRef, scope: Scope) -> PyResult { - let frame = Frame::new(code, scope, self).into_ref(self); + let frame = Frame::new(code, scope, &[], self).into_ref(self); self.run_frame_full(frame) } @@ -495,11 +495,18 @@ impl VirtualMachine { } } - pub fn current_scope(&self) -> Ref { + pub fn current_locals(&self) -> Ref { let frame = self .current_frame() - .expect("called current_scope but no frames on the stack"); - Ref::map(frame, |f| &f.scope) + .expect("called current_locals but no frames on the stack"); + Ref::map(frame, |f| f.locals(self)) + } + + pub fn current_globals(&self) -> Ref { + let frame = self + .current_frame() + .expect("called current_globals but no frames on the stack"); + Ref::map(frame, |f| &f.globals) } pub fn try_class(&self, module: &str, class: &str) -> PyResult { @@ -794,10 +801,6 @@ impl VirtualMachine { obj.unwrap_or_else(|| self.ctx.none()) } - pub fn get_locals(&self) -> PyDictRef { - self.current_scope().get_locals().clone() - } - // Container of the virtual machine state: pub fn to_str(&self, obj: &PyObjectRef) -> PyResult { if obj.class().is(&self.ctx.types.str_type) { @@ -872,12 +875,9 @@ impl VirtualMachine { })?; let (locals, globals) = if let Some(frame) = self.current_frame() { - ( - frame.scope.get_locals().clone().into_object(), - frame.scope.globals.clone().into_object(), - ) + (Some(frame.locals.clone()), Some(frame.globals.clone())) } else { - (self.ctx.none(), self.ctx.none()) + (None, None) }; let from_list = self .ctx From 92b8e59158ed3c7c5d5a090faaa468059a412311 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Sun, 29 Nov 2020 20:16:01 -0600 Subject: [PATCH 06/14] Fast locals part 2 --- bytecode/src/bytecode.rs | 43 +++++---- compiler/src/compile.rs | 146 ++++++++++++++++++++-------- compiler/src/symboltable.rs | 168 +++++++++++++++++++++++++-------- src/shell.rs | 2 +- src/shell/helper.rs | 16 ++-- vm/src/builtins/dict.rs | 78 ++++++++++----- vm/src/builtins/function.rs | 134 ++++++++++++++++---------- vm/src/builtins/make_module.rs | 13 ++- vm/src/builtins/pysuper.rs | 34 ++++--- vm/src/dictdatatype.rs | 23 +++++ vm/src/frame.rs | 47 ++++++--- vm/src/pyobject.rs | 3 +- vm/src/scope.rs | 8 +- vm/src/vm.rs | 3 +- 14 files changed, 505 insertions(+), 213 deletions(-) diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 42caf79121..85ec43a99e 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -102,6 +102,7 @@ pub struct CodeObject { pub source_path: String, pub first_line_number: usize, pub obj_name: String, // Name of the object that created this code object + pub cell2arg: Option>, pub constants: Vec, #[serde(bound( deserialize = "C::Name: serde::Deserialize<'de>", @@ -184,7 +185,7 @@ pub enum Instruction { idx: NameIdx, }, LoadFast(NameIdx), - LoadLocal(NameIdx), + LoadNameAny(NameIdx), LoadGlobal(NameIdx), LoadDeref(NameIdx), LoadClassDeref(NameIdx), @@ -550,6 +551,7 @@ impl CodeObject { source_path, first_line_number, obj_name, + cell2arg: None, constants: Vec::new(), names: Vec::new(), varnames: Vec::new(), @@ -611,7 +613,7 @@ impl CodeObject { } #[rustfmt::skip] - Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadLocal(_) + Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadNameAny(_) | LoadGlobal(_) | LoadDeref(_) | LoadClassDeref(_) | StoreFast(_) | StoreLocal(_) | StoreGlobal(_) | StoreDeref(_) | DeleteFast(_) | DeleteLocal(_) | DeleteGlobal(_) | DeleteDeref(_) | LoadClosure(_) | Subscript | StoreSubscript | DeleteSubscript @@ -699,6 +701,7 @@ impl CodeObject { source_path: self.source_path, first_line_number: self.first_line_number, obj_name: self.obj_name, + cell2arg: self.cell2arg, } } @@ -729,6 +732,7 @@ impl CodeObject { source_path: self.source_path.clone(), first_line_number: self.first_line_number, obj_name: self.obj_name.clone(), + cell2arg: self.cell2arg.clone(), } } } @@ -798,7 +802,12 @@ impl Instruction { }; } - let cellname = |i: usize| cellvars.get(i).unwrap_or_else(|| &freevars[i]).as_ref(); + let cellname = |i: usize| { + cellvars + .get(i) + .unwrap_or_else(|| &freevars[i - cellvars.len()]) + .as_ref() + }; match self { Import { @@ -819,20 +828,20 @@ impl Instruction { ), ImportStar => w!(ImportStar), ImportFrom { idx } => w!(ImportFrom, names[*idx].as_ref()), - LoadFast(idx) => w!(LoadFast, varnames[*idx].as_ref()), - LoadLocal(idx) => w!(LoadLocal, names[*idx].as_ref()), - LoadGlobal(idx) => w!(LoadGlobal, names[*idx].as_ref()), - LoadDeref(idx) => w!(LoadDeref, cellname(*idx)), - LoadClassDeref(idx) => w!(LoadClassDeref, cellname(*idx)), - StoreFast(idx) => w!(StoreFast, varnames[*idx].as_ref()), - StoreLocal(idx) => w!(StoreLocal, names[*idx].as_ref()), - StoreGlobal(idx) => w!(StoreGlobal, names[*idx].as_ref()), - StoreDeref(idx) => w!(StoreDeref, cellname(*idx)), - DeleteFast(idx) => w!(DeleteFast, varnames[*idx].as_ref()), - DeleteLocal(idx) => w!(DeleteLocal, names[*idx].as_ref()), - DeleteGlobal(idx) => w!(DeleteGlobal, names[*idx].as_ref()), - DeleteDeref(idx) => w!(DeleteDeref, cellname(*idx)), - LoadClosure(i) => w!(LoadClosure, cellname(*i)), + LoadFast(idx) => w!(LoadFast, *idx, varnames[*idx].as_ref()), + LoadNameAny(idx) => w!(LoadNameAny, *idx, names[*idx].as_ref()), + LoadGlobal(idx) => w!(LoadGlobal, *idx, names[*idx].as_ref()), + LoadDeref(idx) => w!(LoadDeref, *idx, cellname(*idx)), + LoadClassDeref(idx) => w!(LoadClassDeref, *idx, cellname(*idx)), + StoreFast(idx) => w!(StoreFast, *idx, varnames[*idx].as_ref()), + StoreLocal(idx) => w!(StoreLocal, *idx, names[*idx].as_ref()), + StoreGlobal(idx) => w!(StoreGlobal, *idx, names[*idx].as_ref()), + StoreDeref(idx) => w!(StoreDeref, *idx, cellname(*idx)), + DeleteFast(idx) => w!(DeleteFast, *idx, varnames[*idx].as_ref()), + DeleteLocal(idx) => w!(DeleteLocal, *idx, names[*idx].as_ref()), + DeleteGlobal(idx) => w!(DeleteGlobal, *idx, names[*idx].as_ref()), + DeleteDeref(idx) => w!(DeleteDeref, *idx, cellname(*idx)), + LoadClosure(i) => w!(LoadClosure, *i, cellname(*i)), Subscript => w!(Subscript), StoreSubscript => w!(StoreSubscript), DeleteSubscript => w!(DeleteSubscript), diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 3137b47b4c..2917ccd465 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -40,6 +40,31 @@ impl CodeInfo { code.cellvars.extend(cellvar_cache); code.freevars.extend(freevar_cache); + if !code.cellvars.is_empty() { + let total_args = code.arg_count + + code.kwonlyarg_count + + code.flags.contains(bytecode::CodeFlags::HAS_VARARGS) as usize + + code.flags.contains(bytecode::CodeFlags::HAS_VARKEYWORDS) as usize; + let all_args = &code.varnames[..total_args]; + let mut found_cellarg = false; + let cell2arg = code + .cellvars + .iter() + .map(|var| { + for (i, arg) in all_args.iter().enumerate() { + if var == arg { + found_cellarg = true; + return i as isize; + } + } + -1 + }) + .collect::>(); + if found_cellarg { + code.cell2arg = Some(cell2arg); + } + } + for instruction in &mut code.instructions { use Instruction::*; // this is a little bit hacky, as until now the data stored inside Labels in @@ -64,7 +89,7 @@ impl CodeInfo { } #[rustfmt::skip] - Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadLocal(_) + Import { .. } | ImportStar | ImportFrom { .. } | LoadFast(_) | LoadNameAny(_) | LoadGlobal(_) | LoadDeref(_) | LoadClassDeref(_) | StoreFast(_) | StoreLocal(_) | StoreGlobal(_) | StoreDeref(_) | DeleteFast(_) | DeleteLocal(_) | DeleteGlobal(_) | DeleteDeref(_) | LoadClosure(_) | Subscript | StoreSubscript | DeleteSubscript @@ -114,14 +139,14 @@ impl Default for CompileOpts { } } -#[derive(Clone, Copy)] +#[derive(Debug, Clone, Copy)] struct CompileContext { in_loop: bool, in_class: bool, func: FunctionContext, } -#[derive(Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq)] enum FunctionContext { NoFunction, Function, @@ -130,7 +155,7 @@ enum FunctionContext { impl CompileContext { fn in_func(self) -> bool { - !matches!(self.func, FunctionContext::NoFunction) + self.func != FunctionContext::NoFunction } } @@ -408,9 +433,8 @@ impl Compiler { cache = &mut info.varname_cache; NameOpType::Fast } - SymbolScope::Local => NameOpType::Local, SymbolScope::GlobalImplicit if self.ctx.in_func() => NameOpType::Global, - SymbolScope::GlobalImplicit => NameOpType::Local, + SymbolScope::Local | SymbolScope::GlobalImplicit => NameOpType::Local, SymbolScope::GlobalExplicit => NameOpType::Global, SymbolScope::Free => { cache = &mut info.freevar_cache; @@ -423,9 +447,12 @@ impl Compiler { // TODO: is this right? SymbolScope::Unknown => NameOpType::Global, }; - let idx = cache + let mut idx = cache .get_index_of(name) .unwrap_or_else(|| cache.insert_full(name.to_owned()).0); + if let SymbolScope::Free = symbol.scope { + idx += info.cellvar_cache.len(); + } let op = match op_typ { NameOpType::Fast => match usage { NameUsage::Load => Instruction::LoadFast, @@ -438,12 +465,15 @@ impl Compiler { NameUsage::Delete => Instruction::DeleteGlobal, }, NameOpType::Deref => match usage { + NameUsage::Load if !self.ctx.in_func() && self.ctx.in_class => { + Instruction::LoadClassDeref + } NameUsage::Load => Instruction::LoadDeref, NameUsage::Store => Instruction::StoreDeref, NameUsage::Delete => Instruction::DeleteDeref, }, NameOpType::Local => match usage { - NameUsage::Load => Instruction::LoadLocal, + NameUsage::Load => Instruction::LoadNameAny, NameUsage::Store => Instruction::StoreLocal, NameUsage::Delete => Instruction::DeleteLocal, }, @@ -844,17 +874,17 @@ impl Compiler { )); for name in &args.args { - self.name(&name.arg); + self.varname(&name.arg); } for name in &args.kwonlyargs { - self.name(&name.arg); + self.varname(&name.arg); } let mut compile_varargs = |va: &ast::Varargs, flag| match va { ast::Varargs::None | ast::Varargs::Unnamed => {} ast::Varargs::Named(name) => { self.current_code().flags |= flag; - self.name(&name.arg); + self.varname(&name.arg); } }; @@ -1002,6 +1032,10 @@ impl Compiler { is_async: bool, ) -> CompileResult<()> { // Create bytecode for this function: + + self.prepare_decorators(decorator_list)?; + self.enter_function(name, args)?; + // remember to restore self.ctx.in_loop to the original after the function is compiled let prev_ctx = self.ctx; @@ -1019,10 +1053,6 @@ impl Compiler { let old_qualified_path = self.current_qualified_path.take(); self.current_qualified_path = Some(self.create_qualified_name(name, ".")); - self.prepare_decorators(decorator_list)?; - - self.enter_function(name, args)?; - let (body, doc_str) = get_doc(body); self.compile_statements(body)?; @@ -1090,26 +1120,7 @@ impl Compiler { code.flags |= bytecode::CodeFlags::IS_COROUTINE; } - if !code.freevars.is_empty() { - for var in &code.freevars { - let symbol = self.symbol_table_stack.last().unwrap().lookup(var).unwrap(); - let parent_code = self.code_stack.last().unwrap(); - let vars = match symbol.scope { - SymbolScope::Free => &parent_code.freevar_cache, - SymbolScope::Cell => &parent_code.cellvar_cache, - _ => unreachable!(), - }; - let mut idx = vars.get_index_of(var).unwrap(); - if let SymbolScope::Free = symbol.scope { - idx += parent_code.cellvar_cache.len(); - } - self.emit(Instruction::LoadClosure(idx)) - } - self.emit(Instruction::BuildTuple { - size: code.freevars.len(), - unpack: false, - }) - } + self.build_closure(&code); self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), @@ -1126,15 +1137,40 @@ impl Compiler { self.emit(Instruction::Rotate { amount: 2 }); let doc = self.name("__doc__"); self.emit(Instruction::StoreAttr { idx: doc }); + + self.current_qualified_path = old_qualified_path; + self.ctx = prev_ctx; + self.apply_decorators(decorator_list); self.store_name(name); - self.current_qualified_path = old_qualified_path; - self.ctx = prev_ctx; Ok(()) } + fn build_closure(&mut self, code: &CodeObject) { + if !code.freevars.is_empty() { + for var in &code.freevars { + let symbol = self.symbol_table_stack.last().unwrap().lookup(var).unwrap(); + let parent_code = self.code_stack.last().unwrap(); + let vars = match symbol.scope { + SymbolScope::Free => &parent_code.freevar_cache, + SymbolScope::Cell => &parent_code.cellvar_cache, + _ => unreachable!(), + }; + let mut idx = vars.get_index_of(var).unwrap(); + if let SymbolScope::Free = symbol.scope { + idx += parent_code.cellvar_cache.len(); + } + self.emit(Instruction::LoadClosure(idx)) + } + self.emit(Instruction::BuildTuple { + size: code.freevars.len(), + unpack: false, + }) + } + } + fn find_ann(&self, body: &[ast::Statement]) -> bool { use ast::StatementType::*; let option_stmt_to_bool = |suit: &Option| -> bool { @@ -1243,11 +1279,30 @@ impl Compiler { self.emit(Instruction::SetupAnnotation); } self.compile_statements(new_body)?; - self.emit_constant(bytecode::ConstantData::None); + + let classcell_idx = self + .code_stack + .last_mut() + .unwrap() + .cellvar_cache + .iter() + .position(|var| *var == "__class__"); + + if let Some(classcell_idx) = classcell_idx { + self.emit(Instruction::LoadClosure(classcell_idx)); + self.emit(Instruction::Duplicate); + let classcell = self.name("__classcell__"); + self.emit(Instruction::StoreLocal(classcell)); + } else { + self.emit_constant(bytecode::ConstantData::None); + } + self.emit(Instruction::ReturnValue); let code = self.pop_code_object(); + self.build_closure(&code); + self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), }); @@ -1526,7 +1581,7 @@ impl Compiler { // Store as dict entry in __annotations__ dict: if !self.ctx.in_func() { let annotations = self.name("__annotations__"); - self.emit(Instruction::LoadLocal(annotations)); + self.emit(Instruction::LoadNameAny(annotations)); self.emit_constant(bytecode::ConstantData::Str { value: name.to_owned(), }); @@ -1942,6 +1997,7 @@ impl Compiler { self.compile_expression(body)?; self.emit(Instruction::ReturnValue); let code = self.pop_code_object(); + self.build_closure(&code); self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), }); @@ -2109,6 +2165,14 @@ impl Compiler { kind: &ast::ComprehensionKind, generators: &[ast::Comprehension], ) -> CompileResult<()> { + let prev_ctx = self.ctx; + + self.ctx = CompileContext { + in_loop: false, + in_class: prev_ctx.in_class, + func: FunctionContext::Function, + }; + // We must have at least one generator: assert!(!generators.is_empty()); @@ -2252,6 +2316,10 @@ impl Compiler { // Fetch code for listcomp function: let code = self.pop_code_object(); + self.ctx = prev_ctx; + + self.build_closure(&code); + // List comprehension code: self.emit_constant(bytecode::ConstantData::Code { code: Box::new(code), diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 02f15ae796..ce70620898 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -144,6 +144,10 @@ impl Symbol { pub fn is_local(&self) -> bool { matches!(self.scope, SymbolScope::Local) } + + pub fn is_bound(&self) -> bool { + self.is_assigned || self.is_parameter || self.is_imported || self.is_iter + } } #[derive(Debug)] @@ -189,29 +193,89 @@ fn analyze_symbol_table(symbol_table: &mut SymbolTable) -> SymbolTableResult { analyzer.analyze_symbol_table(symbol_table) } +type SymbolMap = IndexMap; + +mod stack { + use std::panic; + use std::ptr::NonNull; + pub struct StackStack { + v: Vec>, + } + impl Default for StackStack { + fn default() -> Self { + Self { v: Vec::new() } + } + } + impl StackStack { + pub fn append(&mut self, x: &mut T, f: F) -> R + where + F: FnOnce(&mut Self) -> R, + { + self.v.push(x.into()); + let res = panic::catch_unwind(panic::AssertUnwindSafe(|| f(self))); + self.v.pop(); + res.unwrap_or_else(|x| panic::resume_unwind(x)) + } + + pub fn iter(&self) -> impl Iterator + DoubleEndedIterator + '_ { + self.as_ref().iter().copied() + } + pub fn iter_mut(&mut self) -> impl Iterator + DoubleEndedIterator + '_ { + self.as_mut().iter_mut().map(|x| &mut **x) + } + // pub fn top(&self) -> Option<&T> { + // self.as_ref().last().copied() + // } + // pub fn top_mut(&mut self) -> Option<&mut T> { + // self.as_mut().last_mut().map(|x| &mut **x) + // } + pub fn len(&self) -> usize { + self.v.len() + } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + pub fn as_ref(&self) -> &[&T] { + unsafe { &*(self.v.as_slice() as *const [NonNull] as *const [&T]) } + } + + pub fn as_mut(&mut self) -> &mut [&mut T] { + unsafe { &mut *(self.v.as_mut_slice() as *mut [NonNull] as *mut [&mut T]) } + } + } +} +use stack::StackStack; + /// Symbol table analysis. Can be used to analyze a fully /// build symbol table structure. It will mark variables /// as local variables for example. #[derive(Default)] -struct SymbolTableAnalyzer<'a> { - tables: Vec<(&'a mut IndexMap, SymbolTableType)>, +#[repr(transparent)] +struct SymbolTableAnalyzer { + tables: StackStack<(SymbolMap, SymbolTableType)>, } -impl<'a> SymbolTableAnalyzer<'a> { - fn analyze_symbol_table(&mut self, symbol_table: &'a mut SymbolTable) -> SymbolTableResult { - let symbols = &mut symbol_table.symbols; - let sub_tables = &mut symbol_table.sub_tables; +impl SymbolTableAnalyzer { + fn analyze_symbol_table(&mut self, symbol_table: &mut SymbolTable) -> SymbolTableResult { + let symbols = std::mem::take(&mut symbol_table.symbols); + let sub_tables = &mut *symbol_table.sub_tables; + + let mut info = (symbols, symbol_table.typ); + self.tables.append(&mut info, |list| { + let inner_scope = unsafe { &mut *(list as *mut _ as *mut SymbolTableAnalyzer) }; + // Analyze sub scopes: + for sub_table in sub_tables.iter_mut() { + inner_scope.analyze_symbol_table(sub_table)?; + } + Ok(()) + })?; - self.tables.push((symbols, symbol_table.typ)); - // Analyze sub scopes: - for sub_table in sub_tables { - self.analyze_symbol_table(sub_table)?; - } - let (symbols, st_typ) = self.tables.pop().unwrap(); + symbol_table.symbols = info.0; // Analyze symbols: - for symbol in symbols.values_mut() { - self.analyze_symbol(symbol, st_typ)?; + for symbol in symbol_table.symbols.values_mut() { + self.analyze_symbol(symbol, symbol_table.typ, sub_tables)?; } Ok(()) } @@ -220,6 +284,7 @@ impl<'a> SymbolTableAnalyzer<'a> { &mut self, symbol: &mut Symbol, curr_st_typ: SymbolTableType, + sub_tables: &mut [SymbolTable], ) -> SymbolTableResult { if symbol.is_assign_namedexpr_in_comprehension && curr_st_typ == SymbolTableType::Comprehension @@ -232,11 +297,11 @@ impl<'a> SymbolTableAnalyzer<'a> { } else { match symbol.scope { SymbolScope::Free => { - let scope_depth = self.tables.len(); - if scope_depth > 0 { + if !self.tables.as_ref().is_empty() { + let scope_depth = self.tables.as_ref().iter().count(); // check if the name is already defined in any outer scope // therefore - if scope_depth < 2 || !self.found_in_outer_scope(symbol) { + if scope_depth < 2 || !self.found_in_outer_scope(&symbol.name) { return Err(SymbolTableError { error: format!("no binding for nonlocal '{}' found", symbol.name), // TODO: accurate location info, somehow @@ -262,29 +327,56 @@ impl<'a> SymbolTableAnalyzer<'a> { } SymbolScope::Unknown => { // Try hard to figure out what the scope of this symbol is. - self.analyze_unknown_symbol(symbol); + self.analyze_unknown_symbol(sub_tables, symbol); } } } Ok(()) } - fn found_in_outer_scope(&self, symbol: &Symbol) -> bool { + fn found_in_outer_scope(&mut self, name: &str) -> bool { // Interesting stuff about the __class__ variable: // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object - symbol.name == "__class__" - || self.tables.iter().skip(1).rev().any(|(symbols, typ)| { - *typ != SymbolTableType::Class - && symbols - .get(&symbol.name) - .map_or(false, |sym| sym.is_local() && sym.is_assigned) - }) + if name == "__class__" { + return true; + } + let decl_depth = self.tables.iter().rev().position(|(symbols, typ)| { + !matches!(typ, SymbolTableType::Class | SymbolTableType::Module) + && symbols.get(name).map_or(false, |sym| sym.is_bound()) + }); + + if let Some(decl_depth) = decl_depth { + // decl_depth is the number of tables between the current one and + // the one that declared the cell var + for (table, _) in self.tables.iter_mut().rev().take(decl_depth) { + if !table.contains_key(name) { + let mut symbol = Symbol::new(name); + symbol.scope = SymbolScope::Free; + symbol.is_referenced = true; + table.insert(name.to_owned(), symbol); + } + } + } + + decl_depth.is_some() + } + + fn found_in_inner_scope(sub_tables: &mut [SymbolTable], name: &str) -> bool { + sub_tables.iter().any(|x| { + x.symbols + .get(name) + .map_or(false, |sym| matches!(sym.scope, SymbolScope::Free)) + }) } - fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { - let scope = if symbol.is_assigned || symbol.is_parameter { - SymbolScope::Local - } else if self.found_in_outer_scope(symbol) { + fn analyze_unknown_symbol(&mut self, sub_tables: &mut [SymbolTable], symbol: &mut Symbol) { + let scope = if symbol.is_bound() { + if Self::found_in_inner_scope(sub_tables, &symbol.name) { + SymbolScope::Cell + } else { + SymbolScope::Local + } + } else if self.found_in_outer_scope(&symbol.name) { // Symbol is in some outer scope. SymbolScope::Free } else if self.tables.is_empty() { @@ -305,11 +397,9 @@ impl<'a> SymbolTableAnalyzer<'a> { symbol: &mut Symbol, parent_offset: usize, ) -> SymbolTableResult { - // TODO: quite C-ish way to implement the iteration // when this is called, we expect to be in the direct parent scope of the scope that contains 'symbol' - let offs = self.tables.len() - 1 - parent_offset; - let last = self.tables.get_mut(offs).unwrap(); - let symbols = &mut *last.0; + let last = self.tables.iter_mut().rev().nth(parent_offset).unwrap(); + let symbols = &mut last.0; let table_type = last.1; // it is not allowed to use an iterator variable as assignee in a named expression @@ -532,6 +622,7 @@ impl SymbolTableBuilder { self.register_name("__module__", SymbolUsage::Assigned, location)?; self.register_name("__qualname__", SymbolUsage::Assigned, location)?; self.register_name("__doc__", SymbolUsage::Assigned, location)?; + self.register_name("__class__", SymbolUsage::Assigned, location)?; self.scan_statements(body)?; self.leave_scope(); self.scan_expressions(bases, ExpressionContext::Load)?; @@ -954,7 +1045,7 @@ impl SymbolTableBuilder { let table = self.tables.last_mut().unwrap(); // Some checks for the symbol that present on this scope level: - if let Some(symbol) = table.symbols.get(name) { + let symbol = if let Some(symbol) = table.symbols.get_mut(name) { // Role already set.. match role { SymbolUsage::Global => { @@ -1000,6 +1091,7 @@ impl SymbolTableBuilder { // Ok? } } + symbol } else { // The symbol does not present on this scope level. // Some checks to insert new symbol into symbol table: @@ -1016,11 +1108,10 @@ impl SymbolTableBuilder { } // Insert symbol when required: let symbol = Symbol::new(name); - table.symbols.insert(name.to_owned(), symbol); - } + table.symbols.entry(name.to_owned()).or_insert(symbol) + }; // Set proper flags on symbol: - let symbol = table.symbols.get_mut(name).unwrap(); match role { SymbolUsage::Nonlocal => { symbol.scope = SymbolScope::Free; @@ -1064,7 +1155,6 @@ impl SymbolTableBuilder { } SymbolUsage::Iter => { symbol.is_iter = true; - symbol.scope = SymbolScope::Local; } } diff --git a/src/shell.rs b/src/shell.rs index e42fd39d94..1987e08344 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -35,7 +35,7 @@ fn shell_exec(vm: &VirtualMachine, source: &str, scope: Scope) -> ShellExecResul } pub fn run_shell(vm: &VirtualMachine, scope: Scope) -> PyResult<()> { - let mut repl = Readline::new(helper::ShellHelper::new(vm, scope.clone())); + let mut repl = Readline::new(helper::ShellHelper::new(vm, scope.globals.clone())); let mut full_input = String::new(); // Retrieve a `history_path_str` dependent on the OS diff --git a/src/shell/helper.rs b/src/shell/helper.rs index c639a004de..e8d07967d1 100644 --- a/src/shell/helper.rs +++ b/src/shell/helper.rs @@ -1,11 +1,10 @@ -use rustpython_vm::builtins::PyStrRef; +use rustpython_vm::builtins::{PyDictRef, PyStrRef}; use rustpython_vm::pyobject::{BorrowValue, PyIterable, PyResult, TryFromObject}; -use rustpython_vm::scope::Scope; use rustpython_vm::VirtualMachine; pub struct ShellHelper<'vm> { vm: &'vm VirtualMachine, - scope: Scope, + globals: PyDictRef, } fn reverse_string(s: &mut String) { @@ -51,8 +50,8 @@ fn split_idents_on_dot(line: &str) -> Option<(usize, Vec)> { } impl<'vm> ShellHelper<'vm> { - pub fn new(vm: &'vm VirtualMachine, scope: Scope) -> Self { - ShellHelper { vm, scope } + pub fn new(vm: &'vm VirtualMachine, globals: PyDictRef) -> Self { + ShellHelper { vm, globals } } #[allow(clippy::type_complexity)] @@ -74,7 +73,10 @@ impl<'vm> ShellHelper<'vm> { // last: the last word, could be empty if it ends with a dot // parents: the words before the dot - let mut current = self.scope.load_global(self.vm, first.as_str())?; + let mut current = self + .globals + .get_item_option(first.as_str(), self.vm) + .ok()??; for attr in parents { current = self.vm.get_attribute(current.clone(), attr.as_str()).ok()?; @@ -86,7 +88,7 @@ impl<'vm> ShellHelper<'vm> { } else { // we need to get a variable based off of globals/builtins - let globals = str_iter_method(self.scope.globals.as_object(), "keys").ok()?; + let globals = str_iter_method(self.globals.as_object(), "keys").ok()?; let builtins = str_iter_method(&self.vm.builtins, "__dir__").ok()?; Some((&first, Box::new(Iterator::chain(globals, builtins)) as _)) } diff --git a/vm/src/builtins/dict.rs b/vm/src/builtins/dict.rs index a599102bc4..1dff22379e 100644 --- a/vm/src/builtins/dict.rs +++ b/vm/src/builtins/dict.rs @@ -268,7 +268,7 @@ impl PyDict { #[pymethod(magic)] #[cfg_attr(feature = "flame-it", flame("PyDictRef"))] fn getitem(zelf: PyRef, key: PyObjectRef, vm: &VirtualMachine) -> PyResult { - if let Some(value) = zelf.inner_getitem_option(key.clone(), vm)? { + if let Some(value) = zelf.inner_getitem_option(key.clone(), zelf.exact_dict(vm), vm)? { Ok(value) } else { Err(vm.new_key_error(key)) @@ -422,23 +422,30 @@ impl Iterable for PyDict { } impl PyDictRef { + #[inline] + fn exact_dict(&self, vm: &VirtualMachine) -> bool { + self.class().is(&vm.ctx.types.dict_type) + } + /// Return an optional inner item, or an error (can be key error as well) #[inline] fn inner_getitem_option( &self, key: K, + exact: bool, vm: &VirtualMachine, ) -> PyResult> { if let Some(value) = self.entries.get(vm, &key)? { return Ok(Some(value)); } - if let Some(method_or_err) = vm.get_method(self.clone().into_object(), "__missing__") { - let method = method_or_err?; - Ok(Some(vm.invoke(&method, (key,))?)) - } else { - Ok(None) + if !exact { + if let Some(method_or_err) = vm.get_method(self.clone().into_object(), "__missing__") { + let method = method_or_err?; + return vm.invoke(&method, (key,)).map(Some); + } } + Ok(None) } /// Take a python dictionary and convert it to attributes. @@ -466,28 +473,51 @@ impl PyDictRef { // and prevent the creation of the KeyError exception. // Also note, that we prevent the creation of a full PyStr object // if we lookup local names (which happens all of the time). - if self.class().is(&vm.ctx.types.dict_type) { + self._get_item_option_inner(key, self.exact_dict(vm), vm) + } + + #[inline] + fn _get_item_option_inner( + &self, + key: K, + exact: bool, + vm: &VirtualMachine, + ) -> PyResult> { + if exact { // We can take the short path here! - self.inner_getitem_option(key, vm).or_else(|exc| { - if exc.isinstance(&vm.ctx.exceptions.key_error) { - Ok(None) - } else { - Err(exc) - } - }) + self.entries.get(vm, &key) } else { // Fall back to full get_item with KeyError checking + self._subcls_getitem_option(key.into_pyobject(vm), vm) + } + } - match self.get_item(key, vm) { - Ok(value) => Ok(Some(value)), - Err(exc) => { - if exc.isinstance(&vm.ctx.exceptions.key_error) { - Ok(None) - } else { - Err(exc) - } - } - } + fn _subcls_getitem_option( + &self, + key: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult> { + match self.get_item(key, vm) { + Ok(value) => Ok(Some(value)), + Err(exc) if exc.isinstance(&vm.ctx.exceptions.key_error) => Ok(None), + Err(exc) => Err(exc), + } + } + + pub fn get2( + &self, + other: &Self, + key: K, + vm: &VirtualMachine, + ) -> PyResult> { + let self_exact = self.class().is(&vm.ctx.types.dict_type); + let other_exact = self.class().is(&vm.ctx.types.dict_type); + if self_exact && other_exact { + self.entries.get2(&other.entries, vm, &key) + } else if let Some(value) = self._get_item_option_inner(key.clone(), self_exact, vm)? { + Ok(Some(value)) + } else { + other._get_item_option_inner(key, other_exact, vm) } } } diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs index 2455fa396a..125d54317f 100644 --- a/vm/src/builtins/function.rs +++ b/vm/src/builtins/function.rs @@ -43,7 +43,7 @@ pub struct PyFunction { } impl PyFunction { - pub fn new( + pub(crate) fn new( code: PyCodeRef, globals: PyDictRef, closure: Option>, @@ -70,6 +70,7 @@ impl PyFunction { let code = &*self.code; let nargs = func_args.args.len(); let nexpected_args = code.arg_count; + let total_args = code.arg_count + code.kwonlyarg_count; // let arg_names = self.code.arg_names(); // This parses the arguments from args and kwargs into @@ -78,19 +79,21 @@ impl PyFunction { // See also: PyEval_EvalCodeWithName in cpython: // https://github.com/python/cpython/blob/master/Python/ceval.c#L3681 - let n = std::cmp::min(nargs, nexpected_args); + let mut fastlocals = frame.fastlocals.lock(); let mut args_iter = func_args.args.into_iter(); - let mut fastlocals = frame.fastlocals.lock(); - // let mut fill_locals = fastlocals.iter_mut().enumerate(); - // Copy positional arguments into local variables - for (i, arg) in args_iter.by_ref().take(n).enumerate() { - fastlocals[i] = Some(arg); + // zip short-circuits if either iterator returns None, which is the behavior we want -- + // only fill as much as there is to fill with as much as we have + for (local, arg) in Iterator::zip( + fastlocals.iter_mut().take(nexpected_args), + args_iter.by_ref().take(nargs), + ) { + *local = Some(arg); } - let mut vararg_offset = code.arg_count; + let mut vararg_offset = total_args; // Pack other positional arguments in to *args: if code.flags.contains(bytecode::CodeFlags::HAS_VARARGS) { let vararg_value = vm.ctx.new_tuple(args_iter.collect()); @@ -109,37 +112,43 @@ impl PyFunction { // Do we support `**kwargs` ? let kwargs = if code.flags.contains(bytecode::CodeFlags::HAS_VARKEYWORDS) { let d = vm.ctx.new_dict(); - fastlocals[vararg_offset] = Some(d.clone()); + fastlocals[vararg_offset] = Some(d.clone().into_object()); Some(d) } else { None }; - // let argpos = - // |names: &[PyStrRef], name: &str| names.iter().position(|s| s.borrow_value() == name); + let argpos = |range: std::ops::Range<_>, name: &str| { + code.varnames + .iter() + .enumerate() + .skip(range.start) + .take(range.end - range.start) + .find(|(_, s)| s.borrow_value() == name) + .map(|(p, _)| p) + }; let mut posonly_passed_as_kwarg = Vec::new(); // Handle keyword arguments for (name, value) in func_args.kwargs { // Check if we have a parameter with this name: - let dict = if let Some(pos) = - argpos(arg_names.args, &name).or_else(|| argpos(arg_names.kwonlyargs, &name)) - { - if argpos(arg_names.posonlyargs, &name) { - posonly_passed_as_kwarg.push(name); - continue; - } else if locals.contains_key(&name, vm) { + if let Some(pos) = argpos(code.posonlyarg_count..total_args, &name) { + let slot = &mut fastlocals[pos]; + if slot.is_some() { return Err( vm.new_type_error(format!("Got multiple values for argument '{}'", name)) ); } - locals + *slot = Some(value); + } else if argpos(0..code.posonlyarg_count, &name).is_some() { + posonly_passed_as_kwarg.push(name); + } else if let Some(kwargs) = kwargs.as_ref() { + kwargs.set_item(name, value, vm)?; } else { - kwargs.as_ref().ok_or_else(|| { + return Err( vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name)) - })? - }; - // dict.set_item(vm.intern_string(name).into_object(), value, vm)?; + ); + } } if !posonly_passed_as_kwarg.is_empty() { return Err(vm.new_type_error(format!( @@ -152,53 +161,76 @@ impl PyFunction { // Add missing positional arguments, if we have fewer positional arguments than the // function definition calls for if nargs < nexpected_args { - let num_defaults_available = - self.defaults.as_ref().map_or(0, |d| d.borrow_value().len()); + let ndefs = self.defaults.as_ref().map_or(0, |d| d.borrow_value().len()); + + let nrequired = code.arg_count - ndefs; // Given the number of defaults available, check all the arguments for which we // _don't_ have defaults; if any are missing, raise an exception - let required_args = nexpected_args - num_defaults_available; let mut missing = vec![]; - for i in 0..required_args { - let variable_name = &arg_names.args[i]; - if !locals.contains_key(variable_name.clone(), vm) { - missing.push(variable_name) + for i in nargs..nrequired { + if fastlocals[i].is_none() { + missing.push(&code.varnames[i]); } } if !missing.is_empty() { return Err(vm.new_type_error(format!( - "Missing {} required positional arguments: {:?}", + "Missing {} required positional arguments: {}", missing.len(), - missing + missing.iter().format(", ") ))); } + if let Some(defaults) = &self.defaults { let defaults = defaults.borrow_value(); + + let n = std::cmp::min(nargs, nexpected_args); + let i = n.saturating_sub(nrequired); + // We have sufficient defaults, so iterate over the corresponding names and use // the default if we don't already have a value - for (default_index, i) in (required_args..nexpected_args).enumerate() { - let arg_name = &arg_names.args[i]; - if !locals.contains_key(arg_name.clone(), vm) { - locals.set_item(arg_name.clone(), defaults[default_index].clone(), vm)?; + for i in i..defaults.len() { + let slot = &mut fastlocals[nrequired + i]; + if slot.is_none() { + *slot = Some(defaults[i].clone()); } } } }; - // Check if kw only arguments are all present: - for arg_name in arg_names.kwonlyargs { - if !locals.contains_key(arg_name.clone(), vm) { - if let Some(kw_only_defaults) = &self.kw_only_defaults { - if let Some(default) = kw_only_defaults.get_item_option(arg_name.clone(), vm)? { - locals.set_item(arg_name.clone(), default, vm)?; - continue; + if code.kwonlyarg_count > 0 { + // TODO: compile a list of missing arguments + // let mut missing = vec![]; + // Check if kw only arguments are all present: + for (slot, kwarg) in fastlocals + .iter_mut() + .zip(&code.varnames) + .skip(code.arg_count) + .take(code.kwonlyarg_count) + { + if slot.is_none() { + if let Some(kw_only_defaults) = &self.kw_only_defaults { + if let Some(default) = + kw_only_defaults.get_item_option(kwarg.clone(), vm)? + { + *slot = Some(default); + continue; + } } + + // No default value and not specified. + return Err(vm.new_type_error(format!( + "Missing required kw only argument: '{}'", + kwarg + ))); } + } + } - // No default value and not specified. - return Err( - vm.new_type_error(format!("Missing required kw only argument: '{}'", arg_name)) - ); + if let Some(cell2arg) = code.cell2arg.as_deref() { + for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) { + let x = fastlocals[*arg_idx as usize].take(); + frame.cells_frees[cell_idx].set(x); } } @@ -237,6 +269,7 @@ impl PyFunction { let frame = Frame::new( code.clone(), Scope::new(Some(locals), self.globals.clone()), + vm.builtins.dict().unwrap(), self.closure.as_ref().map_or(&[], |c| c.borrow_value()), vm, ) @@ -416,10 +449,13 @@ impl PyValue for PyCell { impl PyCell { #[pyslot] fn tp_new(cls: PyTypeRef, value: OptionalArg, vm: &VirtualMachine) -> PyResult> { + Self::new(value.into_option()).into_ref_with_type(vm, cls) + } + + pub fn new(contents: Option) -> Self { Self { - contents: PyMutex::new(value.into_option()), + contents: PyMutex::new(contents), } - .into_ref_with_type(vm, cls) } pub fn get(&self) -> Option { diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index dcaab34bcb..0226d6b833 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -12,7 +12,7 @@ mod decl { use crate::builtins::bytes::PyBytesRef; use crate::builtins::code::PyCodeRef; use crate::builtins::dict::PyDictRef; - use crate::builtins::function::PyFunctionRef; + use crate::builtins::function::{PyCellRef, PyFunctionRef}; use crate::builtins::int::{self, PyIntRef}; use crate::builtins::iter::{PyCallableIterator, PySequenceIterator}; use crate::builtins::list::{PyList, SortOptions}; @@ -840,15 +840,20 @@ mod decl { let prepare = vm.get_attribute(metaclass.clone().into_object(), "__prepare__")?; let namespace = vm.invoke(&prepare, vec![name_obj.clone(), bases.clone()])?; - let namespace: PyDictRef = TryFromObject::try_from_object(vm, namespace)?; + let namespace = PyDictRef::try_from_object(vm, namespace)?; - function.invoke_with_locals(().into(), Some(namespace), vm)?; + let classcell = function.invoke_with_locals(().into(), Some(namespace.clone()), vm)?; + let classcell = >::try_from_object(vm, classcell)?; let class = vm.invoke( metaclass.as_object(), FuncArgs::new(vec![name_obj, bases, namespace.into_object()], kwargs), )?; - // cells.set_item("__class__", class.clone(), vm)?; + + if let Some(ref classcell) = classcell { + classcell.set(Some(class.clone())); + } + Ok(class) } } diff --git a/vm/src/builtins/pysuper.rs b/vm/src/builtins/pysuper.rs index f1950ee0a5..58c4dc6d0d 100644 --- a/vm/src/builtins/pysuper.rs +++ b/vm/src/builtins/pysuper.rs @@ -11,7 +11,7 @@ use super::pytype::{PyType, PyTypeRef}; use crate::function::OptionalArg; use crate::pyobject::{ BorrowValue, IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, - TryFromObject, TypeProtocol, + TypeProtocol, }; use crate::slots::{SlotDescriptor, SlotGetattro}; use crate::vm::VirtualMachine; @@ -63,7 +63,7 @@ impl PySuper { } else { let frame = vm .current_frame() - .expect("super() called from outside of frame?"); + .ok_or_else(|| vm.new_runtime_error("super(): no current frame".to_owned()))?; let mut typ = None; for (i, var) in frame.code.freevars.iter().enumerate() { if var.borrow_value() == "__class__" { @@ -99,17 +99,27 @@ impl PySuper { let obj = if let OptionalArg::Present(obj) = py_obj { obj } else { - let frame = vm.current_frame().expect("no current frame for super()"); - if let Some(first_arg) = frame.code.arg_names().args.get(0) { - frame - .locals - .get_item_option(first_arg.clone(), vm)? - .ok_or_else(|| { - vm.new_type_error(format!("super argument {} was not supplied", first_arg)) - })? - } else { - vm.ctx.none() + let frame = vm + .current_frame() + .ok_or_else(|| vm.new_runtime_error("super(): no current frame".to_owned()))?; + if frame.code.arg_count == 0 { + return Err(vm.new_runtime_error("super(): no arguments".to_owned())); } + let fastlocals = frame.fastlocals.lock(); + fastlocals[0] + .clone() + .or_else(|| { + if let Some(cell2arg) = frame.code.cell2arg.as_deref() { + cell2arg[..frame.code.cellvars.len()] + .iter() + .enumerate() + .find(|(_, arg_idx)| **arg_idx == 0) + .and_then(|(cell_idx, _)| frame.cells_frees[cell_idx].get()) + } else { + None + } + }) + .ok_or_else(|| vm.new_runtime_error("super(): arg[0] deleted".to_owned()))? }; PySuper::new(typ, obj, vm)?.into_ref_with_type(vm, cls) diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index 43df5a5a00..b66bf0d416 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -254,6 +254,15 @@ impl Dict { #[cfg_attr(feature = "flame-it", flame("Dict"))] pub fn get(&self, vm: &VirtualMachine, key: &K) -> PyResult> { let hash = key.key_hash(vm)?; + self._get_inner(vm, key, hash) + } + + fn _get_inner( + &self, + vm: &VirtualMachine, + key: &K, + hash: HashValue, + ) -> PyResult> { let ret = loop { let (entry, index_index) = self.lookup(vm, key, hash, None)?; if let IndexEntry::Index(index) = entry { @@ -275,6 +284,20 @@ impl Dict { Ok(ret) } + pub fn get2( + &self, + other: &Self, + vm: &VirtualMachine, + key: &K, + ) -> PyResult> { + let hash = key.key_hash(vm)?; + if let Some(x) = self._get_inner(vm, key, hash)? { + Ok(Some(x)) + } else { + other._get_inner(vm, key, hash) + } + } + pub fn clear(&self) { let _removed = { let mut inner = self.borrow_value_mut(); diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 8c504d3ae2..83dee8cad3 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -94,9 +94,10 @@ pub struct Frame { pub code: PyCodeRef, pub fastlocals: PyMutex]>>, - pub cells_frees: Box<[PyCellRef]>, + pub(crate) cells_frees: Box<[PyCellRef]>, pub locals: PyDictRef, pub globals: PyDictRef, + pub builtins: PyDictRef, /// index of last instruction ran pub lasti: AtomicUsize, @@ -157,7 +158,13 @@ impl ExecutionResult { pub type FrameResult = PyResult>; impl Frame { - pub fn new(code: PyCodeRef, scope: Scope, closure: &[PyCellRef], vm: &VirtualMachine) -> Frame { + pub(crate) fn new( + code: PyCodeRef, + scope: Scope, + builtins: PyDictRef, + closure: &[PyCellRef], + vm: &VirtualMachine, + ) -> Frame { //populate the globals and locals //TODO: This is wrong, check https://github.com/nedbat/byterun/blob/31e6c4a8212c35b5157919abff43a7daa0f377c6/byterun/pyvm2.py#L95 /* @@ -178,6 +185,7 @@ impl Frame { cells_frees, locals: scope.locals, globals: scope.globals, + builtins, code, lasti: AtomicUsize::new(0), state: PyMutex::new(FrameState { @@ -199,6 +207,7 @@ impl FrameRef { cells_frees: &self.cells_frees, locals: &self.locals, globals: &self.globals, + builtins: &self.builtins, lasti: &self.lasti, object: &self, state: &mut state, @@ -257,6 +266,7 @@ struct ExecutingFrame<'a> { cells_frees: &'a [PyCellRef], locals: &'a PyDictRef, globals: &'a PyDictRef, + builtins: &'a PyDictRef, object: &'a FrameRef, lasti: &'a AtomicUsize, state: &'a mut FrameState, @@ -357,10 +367,7 @@ impl ExecutingFrame<'_> { if let Some(name) = self.code.cellvars.get(i) { vm.new_exception_msg( vm.ctx.exceptions.unbound_local_error.clone(), - format!( - "local variable '{}' referenced before assignment", - self.code.cellvars[i] - ), + format!("local variable '{}' referenced before assignment", name), ) } else { let name = &self.code.freevars[i - self.code.cellvars.len()]; @@ -420,12 +427,18 @@ impl ExecutingFrame<'_> { self.push_value(x); Ok(None) } - bytecode::Instruction::LoadLocal(idx) => { + bytecode::Instruction::LoadNameAny(idx) => { let name = &self.code.names[*idx]; - let x = self - .locals - .get_item_option(name.clone(), vm)? - .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; + let x = self.locals.get_item_option(name.clone(), vm)?; + let x = match x { + Some(x) => x, + None => self + .globals + .get2(self.builtins, name.clone(), vm)? + .ok_or_else(|| { + vm.new_name_error(format!("name '{}' is not defined", name)) + })?, + }; self.push_value(x); Ok(None) } @@ -433,16 +446,17 @@ impl ExecutingFrame<'_> { let name = &self.code.names[*idx]; let x = self .globals - .get_item_option(name.clone(), vm)? + .get2(self.builtins, name.clone(), vm)? .ok_or_else(|| vm.new_name_error(format!("name '{}' is not defined", name)))?; self.push_value(x); Ok(None) } bytecode::Instruction::LoadDeref(i) => { let i = *i; - self.cells_frees[i] + let x = self.cells_frees[i] .get() .ok_or_else(|| self.unbound_cell_exception(i, vm))?; + self.push_value(x); Ok(None) } bytecode::Instruction::LoadClassDeref(i) => { @@ -959,7 +973,7 @@ impl ExecutingFrame<'_> { for (k, v) in &dict { let k = PyStrRef::try_from_object(vm, k)?; if filter_pred(k.borrow_value()) { - self.locals.set_item(k, v, vm); + self.locals.set_item(k, v, vm)?; } } } @@ -1189,6 +1203,11 @@ impl ExecutingFrame<'_> { }; // Call function: + // eprintln!( + // "calling from {} {:?}", + // self.code.obj_name, + // self.code.locations[self.lasti.load(Ordering::Relaxed)] + // ); let func_ref = self.pop_value(); let value = vm.invoke(&func_ref, args)?; self.push_value(value); diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 90f4ca583a..77f5ff6430 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -15,7 +15,7 @@ use crate::builtins::code::PyCodeRef; use crate::builtins::complex::PyComplex; use crate::builtins::dict::{PyDict, PyDictRef}; use crate::builtins::float::PyFloat; -use crate::builtins::function::{PyBoundMethod, PyFunction}; +use crate::builtins::function::PyBoundMethod; use crate::builtins::getset::{IntoPyGetterFunc, IntoPySetterFunc, PyGetSet}; use crate::builtins::int::{PyInt, PyIntRef}; use crate::builtins::iter::PySequenceIterator; @@ -38,7 +38,6 @@ use crate::exceptions::{self, PyBaseExceptionRef}; use crate::function::{IntoFuncArgs, IntoPyNativeFunc}; use crate::iterator; pub use crate::pyobjectrc::{PyObject, PyObjectRef, PyObjectWeak, PyRef, PyWeakRef}; -use crate::scope::Scope; use crate::slots::{PyTpFlags, PyTypeSlots}; use crate::types::{create_type_with_slots, TypeZoo}; use crate::vm::VirtualMachine; diff --git a/vm/src/scope.rs b/vm/src/scope.rs index ed6da13509..8a25dc3a55 100644 --- a/vm/src/scope.rs +++ b/vm/src/scope.rs @@ -1,13 +1,13 @@ use std::fmt; use crate::builtins::{PyDictRef, PyStr, PyStrRef}; -use crate::common::lock::PyMutex; -use crate::pyobject::{IntoPyObject, ItemProtocol, PyContext, PyObjectRef, PyResult, TryIntoRef}; +use crate::pyobject::{IntoPyObject, ItemProtocol, TryIntoRef}; use crate::VirtualMachine; +#[derive(Clone)] pub struct Scope { - pub(crate) locals: PyDictRef, - pub(crate) globals: PyDictRef, + pub locals: PyDictRef, + pub globals: PyDictRef, } impl fmt::Debug for Scope { diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 8a114e9765..3b9e9cd5ce 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -448,7 +448,8 @@ impl VirtualMachine { } pub fn run_code_obj(&self, code: PyCodeRef, scope: Scope) -> PyResult { - let frame = Frame::new(code, scope, &[], self).into_ref(self); + let frame = + Frame::new(code, scope, self.builtins.dict().unwrap(), &[], self).into_ref(self); self.run_frame_full(frame) } From 7c58f4a0c846599645d6b28b7dd4ed7ba99acdf5 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 30 Nov 2020 19:40:12 -0600 Subject: [PATCH 07/14] Fast locals part 3 --- Lib/test/test_builtin.py | 2 - Lib/test/test_grammar.py | 2 - Lib/test/test_importlib/test_api.py | 4 - Lib/test/test_named_expression.py | 6 -- Lib/test/test_opcodes.py | 2 - Lib/test/test_os.py | 6 ++ Lib/test/test_scope.py | 17 +--- Lib/test/test_symtable.py | 2 + Lib/unittest/test/test_runner.py | 2 - compiler/src/compile.rs | 64 +++++++----- compiler/src/error.rs | 4 + compiler/src/symboltable.rs | 147 ++++++++++++++++++---------- src/lib.rs | 2 + vm/src/builtins/code.rs | 2 +- vm/src/builtins/frame.rs | 4 +- vm/src/builtins/make_module.rs | 78 ++++++++------- vm/src/frame.rs | 42 +++++++- vm/src/vm.rs | 9 +- 18 files changed, 244 insertions(+), 151 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 0ad09cb230..a8f0aa7f35 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -562,8 +562,6 @@ def keys(self): return 1 # used to be 'a' but that's no longer an error self.assertRaises(TypeError, eval, 'dir()', globals(), C()) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_exec(self): g = {} exec('z = 1', g) diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index 29137b1b52..258e501882 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -1067,8 +1067,6 @@ def g2(x): self.assertEqual(g2(False), 0) self.assertEqual(g2(True), ('end', 1)) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_yield(self): # Allowed as standalone statement def g(): yield 1 diff --git a/Lib/test/test_importlib/test_api.py b/Lib/test/test_importlib/test_api.py index 7b41376520..edb745c2cd 100644 --- a/Lib/test/test_importlib/test_api.py +++ b/Lib/test/test_importlib/test_api.py @@ -242,8 +242,6 @@ def test_reload_loader_replaced(self): self.assertIs(reloaded, types) self.assertIs(sys.modules['types'], types) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_reload_location_changed(self): name = 'spam' with support.temp_cwd(None) as cwd: @@ -295,8 +293,6 @@ def test_reload_location_changed(self): self.maxDiff = None self.assertEqual(ns, expected) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_reload_namespace_changed(self): name = 'spam' with support.temp_cwd(None) as cwd: diff --git a/Lib/test/test_named_expression.py b/Lib/test/test_named_expression.py index 3b946865c7..acdfe67fa3 100644 --- a/Lib/test/test_named_expression.py +++ b/Lib/test/test_named_expression.py @@ -336,9 +336,6 @@ def spam(a): self.assertEqual(res, [(1, 1, 1.0), (2, 2, 1.0), (3, 3, 1.0)]) self.assertEqual(y, 3) - # TODO RustPython, added test_named_expression_scope_06_rp_modified as - # minimaly modified variant of this test. - @unittest.expectedFailure def test_named_expression_scope_06(self): res = [[spam := i for i in range(3)] for j in range(2)] @@ -385,8 +382,6 @@ def eggs(b): self.assertEqual(res, [0, 2]) self.assertEqual(a, 2) - # TODO RustPython, added test_named_expression_scope_10_rp_modified - @unittest.expectedFailure def test_named_expression_scope_10(self): res = [b := [a := 1 for i in range(2)] for j in range(2)] @@ -522,7 +517,6 @@ def test_named_expression_variable_reuse_in_comprehensions(self): self.assertEqual(ns["x"], 2) self.assertEqual(ns["result"], [0, 1, 2]) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_named_expression_global_scope(self): sentinel = object() global GLOBAL_VAR diff --git a/Lib/test/test_opcodes.py b/Lib/test/test_opcodes.py index 4a56e858e2..1f821e629b 100644 --- a/Lib/test/test_opcodes.py +++ b/Lib/test/test_opcodes.py @@ -38,8 +38,6 @@ class C: pass with self.assertRaises(AttributeError): C.__annotations__ - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_use_existing_annotations(self): ns = {'__annotations__': {1: 2}} exec('x: int', ns) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 43d93f58b5..95db6f15ed 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -1888,6 +1888,12 @@ def get_single(f): def helper(self): if hasattr(os, f): self.check(getattr(os, f)) + + # TODO: RUSTPYTHON; io.FileIO(fd) should check if the fd passed is valid + if f == "fdopen": + # this is test_fdopen + helper = unittest.expectedFailure(helper) + return helper for f in singles: locals()["test_"+f] = get_single(f) diff --git a/Lib/test/test_scope.py b/Lib/test/test_scope.py index 0cb5ec572a..be868e50e2 100644 --- a/Lib/test/test_scope.py +++ b/Lib/test/test_scope.py @@ -136,7 +136,6 @@ def h(): h = g(2, 4, 6) self.assertEqual(h(), 39) - @unittest.expectedFailure # TODO: RUSTPYTHON def testFreeVarInMethod(self): def test(): @@ -202,7 +201,6 @@ def fact(n): self.assertEqual(f(6), 720) - @unittest.expectedFailure # TODO: RUSTPYTHON def testUnoptimizedNamespaces(self): check_syntax_error(self, """if 1: @@ -262,7 +260,6 @@ def testLambdas(self): h = g(2, 4, 6) self.assertEqual(h(), 18) - @unittest.expectedFailure # TODO: RUSTPYTHON def testUnboundLocal(self): def errorInOuter(): @@ -280,7 +277,6 @@ def inner(): self.assertRaises(UnboundLocalError, errorInOuter) self.assertRaises(NameError, errorInInner) - @unittest.expectedFailure # TODO: RUSTPYTHON def testUnboundLocal_AfterDel(self): # #4617: It is now legal to delete a cell variable. # The following functions must obviously compile, @@ -302,7 +298,6 @@ def inner(): self.assertRaises(UnboundLocalError, errorInOuter) self.assertRaises(NameError, errorInInner) - @unittest.expectedFailure # TODO: RUSTPYTHON def testUnboundLocal_AugAssign(self): # test for bug #1501934: incorrect LOAD/STORE_GLOBAL generation exec("""if 1: @@ -335,7 +330,6 @@ def returner(): self.assertEqual(makeReturner2(a=11)()['a'], 11) - @unittest.expectedFailure # TODO: RUSTPYTHON def testScopeOfGlobalStmt(self): # Examples posted by Samuele Pedroni to python-dev on 3/1/2001 @@ -414,7 +408,6 @@ def get(self): self.assertEqual(g.get(), 13) """) - @unittest.expectedFailure # TODO: RUSTPYTHON def testLeaks(self): class Foo: @@ -437,7 +430,6 @@ def f2(): self.assertEqual(Foo.count, 0) - @unittest.expectedFailure # TODO: RUSTPYTHON def testClassAndGlobal(self): exec("""if 1: @@ -460,7 +452,6 @@ class X: self.assertTrue(X.passed) """) - @unittest.expectedFailure # TODO: RUSTPYTHON def testLocalsFunction(self): def f(x): @@ -568,7 +559,6 @@ class TestClass: self.assertRaises(TypeError, sys.settrace) - @unittest.expectedFailure # TODO: RUSTPYTHON def testEvalExecFreeVars(self): def f(x): @@ -603,7 +593,6 @@ def x(): except NameError: pass - @unittest.expectedFailure # TODO: RUSTPYTHON def testEvalFreeVars(self): def f(x): @@ -661,7 +650,10 @@ def dec(self): self.assertEqual(c.dec(), 1) self.assertEqual(c.dec(), 0) - @unittest.expectedFailure # TODO: RUSTPYTHON + # TODO: RUSTPYTHON, figure out how to communicate that `y = 9` should be + # stored as a global rather than a STORE_NAME, even when + # the `global y` is in a nested subscope + @unittest.expectedFailure def testGlobalInParallelNestedFunctions(self): # A symbol table bug leaked the global statement from one # function to other nested functions in the same block. @@ -741,7 +733,6 @@ def top(a): def b(): global a - @unittest.expectedFailure # TODO: RUSTPYTHON def testClassNamespaceOverridesClosure(self): # See #17853. x = 42 diff --git a/Lib/test/test_symtable.py b/Lib/test/test_symtable.py index 65c081681e..d853e0d66a 100644 --- a/Lib/test/test_symtable.py +++ b/Lib/test/test_symtable.py @@ -121,6 +121,8 @@ def test_nonlocal(self): expected = ("some_var",) self.assertEqual(self.other_internal.get_nonlocals(), expected) + # TODO: RUSTPYTHON + @unittest.expectedFailure def test_local(self): self.assertTrue(self.spam.lookup("x").is_local()) self.assertFalse(self.spam.lookup("bar").is_local()) diff --git a/Lib/unittest/test/test_runner.py b/Lib/unittest/test/test_runner.py index c00d6e5327..3759696043 100644 --- a/Lib/unittest/test/test_runner.py +++ b/Lib/unittest/test/test_runner.py @@ -277,8 +277,6 @@ def MockResultClass(*args): self.assertEqual(runner._makeResult(), expectedresult) - # TODO: RUSTPYTHON; -W arg for warning control - @unittest.expectedFailure def test_warnings(self): """ Check that warnings argument of TextTestRunner correctly affects the diff --git a/compiler/src/compile.rs b/compiler/src/compile.rs index 2917ccd465..051cb2cd5e 100644 --- a/compiler/src/compile.rs +++ b/compiler/src/compile.rs @@ -273,14 +273,13 @@ impl Compiler { let cellvar_cache = table .symbols .iter() - .filter(|(_, s)| matches!(s.scope, SymbolScope::Cell)) + .filter(|(_, s)| s.scope == SymbolScope::Cell) .map(|(var, _)| var.clone()) .collect(); let freevar_cache = table .symbols .iter() - // TODO: check if Free or FREE_CLASS symbol - .filter(|(_, s)| matches!(s.scope, SymbolScope::Free)) + .filter(|(_, s)| s.scope == SymbolScope::Free || s.is_free_class) .map(|(var, _)| var.clone()) .collect(); @@ -336,6 +335,11 @@ impl Compiler { let doc = self.name("__doc__"); self.emit(Instruction::StoreGlobal(doc)) } + + if self.find_ann(statements) { + self.emit(Instruction::SetupAnnotation); + } + self.compile_statements(statements)?; assert_eq!(self.code_stack.len(), size_before); @@ -433,9 +437,12 @@ impl Compiler { cache = &mut info.varname_cache; NameOpType::Fast } - SymbolScope::GlobalImplicit if self.ctx.in_func() => NameOpType::Global, - SymbolScope::Local | SymbolScope::GlobalImplicit => NameOpType::Local, SymbolScope::GlobalExplicit => NameOpType::Global, + SymbolScope::GlobalImplicit | SymbolScope::Unknown if self.ctx.in_func() => { + NameOpType::Global + } + SymbolScope::GlobalImplicit | SymbolScope::Unknown => NameOpType::Local, + SymbolScope::Local => NameOpType::Local, SymbolScope::Free => { cache = &mut info.freevar_cache; NameOpType::Deref @@ -444,8 +451,8 @@ impl Compiler { cache = &mut info.cellvar_cache; NameOpType::Deref } - // TODO: is this right? - SymbolScope::Unknown => NameOpType::Global, + // // TODO: is this right? + // SymbolScope::Unknown => NameOpType::Global, }; let mut idx = cache .get_index_of(name) @@ -527,6 +534,10 @@ impl Compiler { let module_idx = module.as_ref().map(|s| self.name(s)); if import_star { + if self.ctx.in_func() { + return Err(self + .error_loc(CompileErrorType::FunctionImportStar, statement.location)); + } let star = self.name("*"); // from .... import * self.emit(Instruction::Import { @@ -1069,6 +1080,8 @@ impl Compiler { } let mut code = self.pop_code_object(); + self.current_qualified_path = old_qualified_path; + self.ctx = prev_ctx; // Prepare type annotations: let mut num_annotations = 0; @@ -1138,9 +1151,6 @@ impl Compiler { let doc = self.name("__doc__"); self.emit(Instruction::StoreAttr { idx: doc }); - self.current_qualified_path = old_qualified_path; - self.ctx = prev_ctx; - self.apply_decorators(decorator_list); self.store_name(name); @@ -1151,12 +1161,17 @@ impl Compiler { fn build_closure(&mut self, code: &CodeObject) { if !code.freevars.is_empty() { for var in &code.freevars { - let symbol = self.symbol_table_stack.last().unwrap().lookup(var).unwrap(); + let table = self.symbol_table_stack.last().unwrap(); + let symbol = table.lookup(var).unwrap(); let parent_code = self.code_stack.last().unwrap(); let vars = match symbol.scope { SymbolScope::Free => &parent_code.freevar_cache, SymbolScope::Cell => &parent_code.cellvar_cache, - _ => unreachable!(), + _ if symbol.is_free_class => &parent_code.freevar_cache, + x => unreachable!( + "var {} in a {:?} should be free or cell but it's {:?}", + var, table.typ, x + ), }; let mut idx = vars.get_index_of(var).unwrap(); if let SymbolScope::Free = symbol.scope { @@ -1236,6 +1251,8 @@ impl Compiler { keywords: &[ast::Keyword], decorator_list: &[ast::Expression], ) -> CompileResult<()> { + self.prepare_decorators(decorator_list)?; + let prev_ctx = self.ctx; self.ctx = CompileContext { func: FunctionContext::NoFunction, @@ -1247,7 +1264,6 @@ impl Compiler { let old_qualified_path = self.current_qualified_path.take(); self.current_qualified_path = Some(qualified_name.clone()); - self.prepare_decorators(decorator_list)?; self.emit(Instruction::LoadBuildClass); let line_number = self.get_source_line_number(); self.push_output(CodeObject::new( @@ -1301,6 +1317,9 @@ impl Compiler { let code = self.pop_code_object(); + self.current_qualified_path = old_qualified_path; + self.ctx = prev_ctx; + self.build_closure(&code); self.emit_constant(bytecode::ConstantData::Code { @@ -1350,8 +1369,6 @@ impl Compiler { self.apply_decorators(decorator_list); self.store_name(name); - self.current_qualified_path = old_qualified_path; - self.ctx = prev_ctx; Ok(()) } @@ -1579,18 +1596,17 @@ impl Compiler { if let ast::ExpressionType::Identifier { name } = &target.node { // Store as dict entry in __annotations__ dict: - if !self.ctx.in_func() { - let annotations = self.name("__annotations__"); - self.emit(Instruction::LoadNameAny(annotations)); - self.emit_constant(bytecode::ConstantData::Str { - value: name.to_owned(), - }); - self.emit(Instruction::StoreSubscript); - } + let annotations = self.name("__annotations__"); + self.emit(Instruction::LoadNameAny(annotations)); + self.emit_constant(bytecode::ConstantData::Str { + value: name.to_owned(), + }); + self.emit(Instruction::StoreSubscript); } else { // Drop annotation if not assigned to simple identifier. self.emit(Instruction::Pop); } + Ok(()) } @@ -2187,7 +2203,7 @@ impl Compiler { let line_number = self.get_source_line_number(); // Create magnificent function : self.push_output(CodeObject::new( - Default::default(), + bytecode::CodeFlags::NEW_LOCALS | bytecode::CodeFlags::IS_OPTIMIZED, 1, 1, 0, diff --git a/compiler/src/error.rs b/compiler/src/error.rs index 37881f5c2b..b5b1d866c9 100644 --- a/compiler/src/error.rs +++ b/compiler/src/error.rs @@ -36,6 +36,7 @@ pub enum CompileErrorType { AsyncReturnValue, InvalidFuturePlacement, InvalidFutureFeature(String), + FunctionImportStar, } impl fmt::Display for CompileErrorType { @@ -66,6 +67,9 @@ impl fmt::Display for CompileErrorType { CompileErrorType::InvalidFutureFeature(feat) => { write!(f, "future feature {} is not defined", feat) } + CompileErrorType::FunctionImportStar => { + write!(f, "import * only allowed at module level") + } } } } diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index ce70620898..ca1ed1af0c 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -85,7 +85,7 @@ impl fmt::Display for SymbolTableType { /// Indicator for a single symbol what the scope of this symbol is. /// The scope can be unknown, which is unfortunate, but not impossible. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub enum SymbolScope { Unknown, Local, @@ -116,6 +116,16 @@ pub struct Symbol { // inidicates that the symbol is used a bound iterator variable. We distinguish this case // from normal assignment to detect unallowed re-assignment to iterator variables. pub is_iter: bool, + + /// indicates that the symbol is a free variable in a class method from the scope that the + /// class is defined in, e.g.: + /// ```python + /// def foo(x): + /// class A: + /// def method(self): + /// return x // is_free_class + /// ``` + pub is_free_class: bool, } impl Symbol { @@ -131,6 +141,7 @@ impl Symbol { is_imported: false, is_assign_namedexpr_in_comprehension: false, is_iter: false, + is_free_class: false, } } @@ -142,7 +153,7 @@ impl Symbol { } pub fn is_local(&self) -> bool { - matches!(self.scope, SymbolScope::Local) + self.scope == SymbolScope::Local } pub fn is_bound(&self) -> bool { @@ -283,12 +294,10 @@ impl SymbolTableAnalyzer { fn analyze_symbol( &mut self, symbol: &mut Symbol, - curr_st_typ: SymbolTableType, + st_typ: SymbolTableType, sub_tables: &mut [SymbolTable], ) -> SymbolTableResult { - if symbol.is_assign_namedexpr_in_comprehension - && curr_st_typ == SymbolTableType::Comprehension - { + if symbol.is_assign_namedexpr_in_comprehension && st_typ == SymbolTableType::Comprehension { // propagate symbol to next higher level that can hold it, // i.e., function or module. Comprehension is skipped and // Class is not allowed and detected as error. @@ -298,10 +307,12 @@ impl SymbolTableAnalyzer { match symbol.scope { SymbolScope::Free => { if !self.tables.as_ref().is_empty() { - let scope_depth = self.tables.as_ref().iter().count(); + let scope_depth = self.tables.as_ref().len(); // check if the name is already defined in any outer scope // therefore - if scope_depth < 2 || !self.found_in_outer_scope(&symbol.name) { + if scope_depth < 2 + || self.found_in_outer_scope(&symbol.name) != Some(SymbolScope::Free) + { return Err(SymbolTableError { error: format!("no binding for nonlocal '{}' found", symbol.name), // TODO: accurate location info, somehow @@ -327,68 +338,101 @@ impl SymbolTableAnalyzer { } SymbolScope::Unknown => { // Try hard to figure out what the scope of this symbol is. - self.analyze_unknown_symbol(sub_tables, symbol); + let scope = if symbol.is_bound() { + self.found_in_inner_scope(sub_tables, &symbol.name, st_typ) + .unwrap_or(SymbolScope::Local) + } else if let Some(scope) = self.found_in_outer_scope(&symbol.name) { + scope + } else if self.tables.is_empty() { + // Don't make assumptions when we don't know. + SymbolScope::Unknown + } else { + // If there are scopes above we assume global. + SymbolScope::GlobalImplicit + }; + symbol.scope = scope; } } } Ok(()) } - fn found_in_outer_scope(&mut self, name: &str) -> bool { + fn found_in_outer_scope(&mut self, name: &str) -> Option { // Interesting stuff about the __class__ variable: // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object if name == "__class__" { - return true; + return Some(SymbolScope::Free); + } + + let mut decl_depth = None; + for (i, (symbols, typ)) in self.tables.iter().rev().enumerate() { + if let SymbolTableType::Class | SymbolTableType::Module = typ { + continue; + } + if let Some(sym) = symbols.get(name) { + match sym.scope { + SymbolScope::GlobalExplicit => return Some(SymbolScope::GlobalExplicit), + SymbolScope::GlobalImplicit => {} + _ => { + if sym.is_bound() { + decl_depth = Some(i); + break; + } + } + } + } } - let decl_depth = self.tables.iter().rev().position(|(symbols, typ)| { - !matches!(typ, SymbolTableType::Class | SymbolTableType::Module) - && symbols.get(name).map_or(false, |sym| sym.is_bound()) - }); if let Some(decl_depth) = decl_depth { // decl_depth is the number of tables between the current one and // the one that declared the cell var - for (table, _) in self.tables.iter_mut().rev().take(decl_depth) { - if !table.contains_key(name) { + for (table, typ) in self.tables.iter_mut().rev().take(decl_depth) { + if let SymbolTableType::Class = typ { + if let Some(free_class) = table.get_mut(name) { + free_class.is_free_class = true; + } else { + let mut symbol = Symbol::new(name); + symbol.is_free_class = true; + symbol.scope = SymbolScope::Free; + table.insert(name.to_owned(), symbol); + } + } else if !table.contains_key(name) { let mut symbol = Symbol::new(name); symbol.scope = SymbolScope::Free; - symbol.is_referenced = true; + // symbol.is_referenced = true; table.insert(name.to_owned(), symbol); } } } - decl_depth.is_some() + decl_depth.map(|_| SymbolScope::Free) } - fn found_in_inner_scope(sub_tables: &mut [SymbolTable], name: &str) -> bool { - sub_tables.iter().any(|x| { - x.symbols - .get(name) - .map_or(false, |sym| matches!(sym.scope, SymbolScope::Free)) + fn found_in_inner_scope( + &self, + sub_tables: &[SymbolTable], + name: &str, + st_typ: SymbolTableType, + ) -> Option { + sub_tables.iter().find_map(|st| { + st.symbols.get(name).and_then(|sym| { + if sym.scope == SymbolScope::Free || sym.is_free_class { + if st_typ == SymbolTableType::Class && name != "__class__" { + None + } else { + Some(SymbolScope::Cell) + } + } else if sym.scope == SymbolScope::GlobalExplicit && self.tables.is_empty() { + // the symbol is defined on the module level, and an inner scope declares + // a global that points to it + Some(SymbolScope::GlobalExplicit) + } else { + None + } + }) }) } - fn analyze_unknown_symbol(&mut self, sub_tables: &mut [SymbolTable], symbol: &mut Symbol) { - let scope = if symbol.is_bound() { - if Self::found_in_inner_scope(sub_tables, &symbol.name) { - SymbolScope::Cell - } else { - SymbolScope::Local - } - } else if self.found_in_outer_scope(&symbol.name) { - // Symbol is in some outer scope. - SymbolScope::Free - } else if self.tables.is_empty() { - // Don't make assumptions when we don't know. - SymbolScope::Unknown - } else { - // If there are scopes above we assume global. - SymbolScope::GlobalImplicit - }; - symbol.scope = scope; - } - // Implements the symbol analysis and scope extension for names // assigned by a named expression in a comprehension. See: // https://github.com/python/cpython/blob/7b78e7f9fd77bb3280ee39fb74b86772a7d46a70/Python/symtable.c#L1435 @@ -431,12 +475,13 @@ impl SymbolTableAnalyzer { if let SymbolScope::Unknown = parent_symbol.scope { // this information is new, as the asignment is done in inner scope parent_symbol.is_assigned = true; - //self.analyze_unknown_symbol(symbol); // not needed, symbol is analyzed anyhow when its scope is analyzed } - if !symbol.is_global() { - symbol.scope = SymbolScope::Free; - } + symbol.scope = if parent_symbol.is_global() { + parent_symbol.scope + } else { + SymbolScope::Free + }; } else { let mut cloned_sym = symbol.clone(); cloned_sym.scope = SymbolScope::Local; @@ -907,6 +952,7 @@ impl SymbolTableBuilder { // Determine the contextual usage of this symbol: match context { ExpressionContext::Delete => { + self.register_name(name, SymbolUsage::Assigned, location)?; self.register_name(name, SymbolUsage::Used, location)?; } ExpressionContext::Load | ExpressionContext::IterDefinitionExp => { @@ -1051,6 +1097,7 @@ impl SymbolTableBuilder { SymbolUsage::Global => { if symbol.is_global() { // Ok + symbol.scope = SymbolScope::GlobalExplicit; } else { return Err(SymbolTableError { error: format!("name '{}' is used prior to global declaration", name), @@ -1140,9 +1187,9 @@ impl SymbolTableBuilder { } SymbolUsage::Global => { if let SymbolScope::Unknown = symbol.scope { - symbol.scope = SymbolScope::GlobalImplicit; + symbol.scope = SymbolScope::GlobalExplicit; } else if symbol.is_global() { - // Global scope can be set to global + symbol.scope = SymbolScope::GlobalExplicit; } else { return Err(SymbolTableError { error: format!("Symbol {} scope cannot be set to global, since its scope was already determined otherwise.", name), diff --git a/src/lib.rs b/src/lib.rs index 2ef1def6ba..dbfad98b37 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -260,6 +260,7 @@ fn parse_arguments<'a>(app: App<'a, '_>) -> ArgMatches<'a> { .short("X") .takes_value(true) .multiple(true) + .number_of_values(1) .help("set implementation-specific option"), ) .arg( @@ -267,6 +268,7 @@ fn parse_arguments<'a>(app: App<'a, '_>) -> ArgMatches<'a> { .short("W") .takes_value(true) .multiple(true) + .number_of_values(1) .help("warning control; arg is action:message:category:module:lineno"), ) .arg( diff --git a/vm/src/builtins/code.rs b/vm/src/builtins/code.rs index c8af7f3892..7d4d10e48a 100644 --- a/vm/src/builtins/code.rs +++ b/vm/src/builtins/code.rs @@ -259,7 +259,7 @@ impl PyCodeRef { fn co_varnames(self, vm: &VirtualMachine) -> PyObjectRef { let varnames = self .code - .names + .varnames .iter() .map(|s| s.clone().into_object()) .collect(); diff --git a/vm/src/builtins/frame.rs b/vm/src/builtins/frame.rs index 3203c725f6..5248dac61d 100644 --- a/vm/src/builtins/frame.rs +++ b/vm/src/builtins/frame.rs @@ -48,8 +48,8 @@ impl FrameRef { } #[pyproperty] - fn f_locals(self, vm: &VirtualMachine) -> PyDictRef { - self.locals(vm).clone() + fn f_locals(self, vm: &VirtualMachine) -> PyResult { + self.locals(vm) } #[pyproperty] diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs index 0226d6b833..821f17fb4d 100644 --- a/vm/src/builtins/make_module.rs +++ b/vm/src/builtins/make_module.rs @@ -172,7 +172,7 @@ mod decl { fn dir(obj: OptionalArg, vm: &VirtualMachine) -> PyResult { let seq = match obj { OptionalArg::Present(obj) => vm.call_method(&obj, "__dir__", ())?, - OptionalArg::Missing => vm.call_method(vm.current_locals().as_object(), "keys", ())?, + OptionalArg::Missing => vm.call_method(vm.current_locals()?.as_object(), "keys", ())?, }; let sorted = sorted(seq, Default::default(), vm)?; Ok(sorted) @@ -195,6 +195,33 @@ mod decl { locals: Option, } + #[cfg(feature = "rustpython-compiler")] + impl ScopeArgs { + fn make_scope(self, vm: &VirtualMachine) -> PyResult { + let (globals, locals) = match self.globals { + Some(globals) => { + if !globals.contains_key("__builtins__", vm) { + let builtins_dict = vm.builtins.dict().unwrap().into_object(); + globals.set_item("__builtins__", builtins_dict, vm)?; + } + let locals = self.locals.unwrap_or_else(|| globals.clone()); + (globals, locals) + } + None => { + let globals = vm.current_globals().clone(); + let locals = match self.locals { + Some(l) => l, + None => vm.current_locals()?, + }; + (globals, locals) + } + }; + + let scope = Scope::with_builtins(Some(locals), globals, vm); + Ok(scope) + } + } + /// Implements `eval`. /// See also: https://docs.python.org/3/library/functions.html#eval #[cfg(feature = "rustpython-compiler")] @@ -204,7 +231,7 @@ mod decl { scope: ScopeArgs, vm: &VirtualMachine, ) -> PyResult { - run_code(vm, source, scope, compile::Mode::Eval) + run_code(vm, source, scope, compile::Mode::Eval, "eval") } /// Implements `exec` @@ -216,7 +243,7 @@ mod decl { scope: ScopeArgs, vm: &VirtualMachine, ) -> PyResult { - run_code(vm, source, scope, compile::Mode::Exec) + run_code(vm, source, scope, compile::Mode::Exec, "exec") } #[cfg(feature = "rustpython-compiler")] @@ -225,8 +252,9 @@ mod decl { source: Either, scope: ScopeArgs, mode: compile::Mode, + func: &str, ) -> PyResult { - let scope = make_scope(vm, scope)?; + let scope = scope.make_scope(vm)?; // Determine code object: let code_obj = match source { @@ -236,38 +264,17 @@ mod decl { Either::B(code_obj) => code_obj, }; + if !code_obj.freevars.is_empty() { + return Err(vm.new_type_error(format!( + "code object passed to {}() may not contain free variables", + func + ))); + } + // Run the code: vm.run_code_obj(code_obj, scope) } - #[cfg(feature = "rustpython-compiler")] - fn make_scope(vm: &VirtualMachine, scope: ScopeArgs) -> PyResult { - let globals = scope.globals; - let locals = match scope.locals { - Some(dict) => Some(dict), - None => { - if globals.is_some() { - None - } else { - Some(vm.current_locals().clone()) - } - } - }; - let globals = match globals { - Some(dict) => { - if !dict.contains_key("__builtins__", vm) { - let builtins_dict = vm.builtins.dict().unwrap().as_object().clone(); - dict.set_item("__builtins__", builtins_dict, vm).unwrap(); - } - dict - } - None => vm.current_globals().clone(), - }; - - let scope = Scope::with_builtins(locals, globals, vm); - Ok(scope) - } - #[pyfunction] fn format( value: PyObjectRef, @@ -447,9 +454,8 @@ mod decl { } #[pyfunction] - fn locals(vm: &VirtualMachine) -> PyDictRef { - let locals = vm.current_locals(); - locals.copy().into_ref(vm) + fn locals(vm: &VirtualMachine) -> PyResult { + vm.current_locals() } fn min_or_max( @@ -796,7 +802,7 @@ mod decl { if let OptionalArg::Present(obj) = obj { vm.get_attribute(obj, "__dict__") } else { - Ok(vm.current_locals().clone().into_object()) + Ok(vm.current_locals()?.into_object()) } } diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 83dee8cad3..814286694c 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -215,8 +215,46 @@ impl FrameRef { f(exec) } - pub fn locals(&self, vm: &VirtualMachine) -> &PyDictRef { - todo!() + pub fn locals(&self, vm: &VirtualMachine) -> PyResult { + let locals = &self.locals; + let code = &**self.code; + let map = &code.varnames; + let j = std::cmp::min(map.len(), code.varnames.len()); + if !code.varnames.is_empty() { + let fastlocals = self.fastlocals.lock(); + for (k, v) in itertools::zip(&map[..j], &**fastlocals) { + if let Some(v) = v { + locals.set_item(k.clone(), v.clone(), vm)?; + } else { + match locals.del_item(k.clone(), vm) { + Ok(()) => {} + Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => {} + Err(e) => return Err(e), + } + } + } + } + if !code.cellvars.is_empty() || !code.freevars.is_empty() { + let map_to_dict = |keys: &[PyStrRef], values: &[PyCellRef]| { + for (k, v) in itertools::zip(keys, values) { + if let Some(v) = v.get() { + locals.set_item(k.clone(), v, vm)?; + } else { + match locals.del_item(k.clone(), vm) { + Ok(()) => {} + Err(e) if e.isinstance(&vm.ctx.exceptions.key_error) => {} + Err(e) => return Err(e), + } + } + } + Ok(()) + }; + map_to_dict(&code.cellvars, &self.cells_frees)?; + if code.flags.contains(bytecode::CodeFlags::IS_OPTIMIZED) { + map_to_dict(&code.freevars, &self.cells_frees[code.cellvars.len()..])?; + } + } + Ok(locals.clone()) } // #[cfg_attr(feature = "flame-it", flame("Frame"))] diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 3b9e9cd5ce..5e9b32203e 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -496,11 +496,10 @@ impl VirtualMachine { } } - pub fn current_locals(&self) -> Ref { - let frame = self - .current_frame() - .expect("called current_locals but no frames on the stack"); - Ref::map(frame, |f| f.locals(self)) + pub fn current_locals(&self) -> PyResult { + self.current_frame() + .expect("called current_locals but no frames on the stack") + .locals(self) } pub fn current_globals(&self) -> Ref { From cd87f6da257ca97264eb3ef1fa271eda99cc96c3 Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 30 Nov 2020 22:15:19 -0600 Subject: [PATCH 08/14] Try some mini-optimizations --- vm/src/frame.rs | 4 ++-- vm/src/pyobject.rs | 54 +++++++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 814286694c..872eed95c6 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -331,7 +331,7 @@ impl ExecutingFrame<'_> { let instr = &self.code.instructions[idx]; let result = self.execute_instruction(instr, vm); match result { - Ok(None) => {} + Ok(None) => continue, Ok(Some(value)) => { break Ok(value); } @@ -349,7 +349,7 @@ impl ExecutingFrame<'_> { exception.set_traceback(Some(new_traceback.into_ref(vm))); match self.unwind_blocks(vm, UnwindReason::Raising { exception }) { - Ok(None) => {} + Ok(None) => continue, Ok(Some(result)) => { break Ok(result); } diff --git a/vm/src/pyobject.rs b/vm/src/pyobject.rs index 77f5ff6430..a2d29e9f71 100644 --- a/vm/src/pyobject.rs +++ b/vm/src/pyobject.rs @@ -383,26 +383,41 @@ impl TryFromObject for PyRef where T: PyValue, { + #[inline] fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { let class = T::class(vm); if obj.isinstance(class) { - obj.downcast().map_err(|obj| { - vm.new_runtime_error(format!( - "Unexpected payload '{}' for type '{}'", - class.name, - obj.class().name, - )) - }) + obj.downcast() + .map_err(|obj| pyref_payload_error(vm, class, obj)) } else { - let expected_type = &class.name; - let actual_type = &obj.class().name; - Err(vm.new_type_error(format!( - "Expected type '{}', not '{}'", - expected_type, actual_type, - ))) + Err(pyref_type_error(vm, class, obj)) } } } +// the impl Borrow allows to pass PyObjectRef or &PyObjectRef +fn pyref_payload_error( + vm: &VirtualMachine, + class: &PyTypeRef, + obj: impl std::borrow::Borrow, +) -> PyBaseExceptionRef { + vm.new_runtime_error(format!( + "Unexpected payload '{}' for type '{}'", + &*class.name, + &*obj.borrow().class().name, + )) +} +fn pyref_type_error( + vm: &VirtualMachine, + class: &PyTypeRef, + obj: impl std::borrow::Borrow, +) -> PyBaseExceptionRef { + let expected_type = &*class.name; + let actual_type = &*obj.borrow().class().name; + vm.new_type_error(format!( + "Expected type '{}', not '{}'", + expected_type, actual_type, + )) +} impl<'a, T: PyValue> From<&'a PyRef> for &'a PyObjectRef { fn from(obj: &'a PyRef) -> Self { @@ -787,19 +802,10 @@ unsafe impl TransmuteFromObject for PyRef { if obj.payload_is::() { Ok(()) } else { - Err(vm.new_runtime_error(format!( - "Unexpected payload '{}' for type '{}'", - class.name, - obj.class().name, - ))) + Err(pyref_payload_error(vm, class, obj)) } } else { - let expected_type = &class.name; - let actual_type = &obj.class().name; - Err(vm.new_type_error(format!( - "Expected type '{}', not '{}'", - expected_type, actual_type, - ))) + Err(pyref_type_error(vm, class, obj)) } } } From 22e76f1a679eca1f5e586cd132ea3800aa54b69b Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 30 Nov 2020 22:35:02 -0600 Subject: [PATCH 09/14] Use a boxed slice for codeobject fields --- bytecode/src/bytecode.rs | 45 +++++++++++++------------- compiler/src/compile.rs | 63 +++++++++++++++++++++++-------------- vm/src/builtins/function.rs | 2 +- 3 files changed, 62 insertions(+), 48 deletions(-) diff --git a/bytecode/src/bytecode.rs b/bytecode/src/bytecode.rs index 85ec43a99e..778455581f 100644 --- a/bytecode/src/bytecode.rs +++ b/bytecode/src/bytecode.rs @@ -93,8 +93,8 @@ impl ConstantBag for BasicBag { /// a codeobject. Also a module has a codeobject. #[derive(Clone, PartialEq, Serialize, Deserialize)] pub struct CodeObject { - pub instructions: Vec, - pub locations: Vec, + pub instructions: Box<[Instruction]>, + pub locations: Box<[Location]>, pub flags: CodeFlags, pub posonlyarg_count: usize, // Number of positional-only arguments pub arg_count: usize, @@ -103,15 +103,15 @@ pub struct CodeObject { pub first_line_number: usize, pub obj_name: String, // Name of the object that created this code object pub cell2arg: Option>, - pub constants: Vec, + pub constants: Box<[C]>, #[serde(bound( deserialize = "C::Name: serde::Deserialize<'de>", serialize = "C::Name: serde::Serialize" ))] - pub names: Vec, - pub varnames: Vec, - pub cellvars: Vec, - pub freevars: Vec, + pub names: Box<[C::Name]>, + pub varnames: Box<[C::Name]>, + pub cellvars: Box<[C::Name]>, + pub freevars: Box<[C::Name]>, } bitflags! { @@ -129,12 +129,6 @@ bitflags! { } } -impl Default for CodeFlags { - fn default() -> Self { - Self::NEW_LOCALS - } -} - impl CodeFlags { pub const NAME_MAPPING: &'static [(&'static str, CodeFlags)] = &[ ("GENERATOR", CodeFlags::IS_GENERATOR), @@ -542,8 +536,8 @@ impl CodeObject { obj_name: String, ) -> Self { CodeObject { - instructions: Vec::new(), - locations: Vec::new(), + instructions: Box::new([]), + locations: Box::new([]), flags, posonlyarg_count, arg_count, @@ -552,11 +546,11 @@ impl CodeObject { first_line_number, obj_name, cell2arg: None, - constants: Vec::new(), - names: Vec::new(), - varnames: Vec::new(), - cellvars: Vec::new(), - freevars: Vec::new(), + constants: Box::new([]), + names: Box::new([]), + varnames: Box::new([]), + cellvars: Box::new([]), + freevars: Box::new([]), } } @@ -593,7 +587,7 @@ impl CodeObject { pub fn label_targets(&self) -> BTreeSet