Merge pull request #1 from RustPython/master

Sync with master

.github/workflows/cron-ci.yaml

@@ -56,3 +56,33 @@ jobs:
         uses: codecov/codecov-action@v1
         with:
           file: ${{ steps.coverage.outputs.report }}
+
+  testdata:
+    name: Collect regression test data
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@master
+      - name: build rustpython
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --release --verbose --all
+      - name: collect tests data
+        run: cargo run --release tests/jsontests.py
+        env:
+          RUSTPYTHONPATH: ${{ github.workspace }}/Lib
+      - name: upload tests data to the website
+        env:
+          SSHKEY: ${{ secrets.ACTIONS_TESTS_DATA_DEPLOY_KEY }}
+          GITHUB_ACTOR: ${{ github.actor }}
+        run: |
+          echo "$SSHKEY" >~/github_key
+          chmod 600 ~/github_key
+          export GIT_SSH_COMMAND="ssh -i ~/github_key"
+
+          git clone git@github.com:RustPython/rustpython.github.io.git website
+          cd website
+          cp ../tests/cpython_tests_results.json ./_data/regrtests_results.json
+          git add ./_data/regrtests_results.json
+          git -c user.name="Github Actions" -c user.email="actions@github.com" commit -m "Update regression test results" --author="$GITHUB_ACTOR"
+          git push

Lib/test/string_tests.py

@@ -500,7 +500,6 @@ class BaseTest:
         self.checkraises(ValueError, 'hello', 'rsplit', '')
         self.checkraises(ValueError, 'hello', 'rsplit', '', 0)
 
-    @unittest.skip("TODO: RUSTPYTHON test_bytes")
     def test_replace(self):
         EQ = self.checkequal
 

Lib/test/test_bytes.py

@@ -1291,7 +1291,8 @@ class ByteArrayTest(BaseBytesTest, unittest.TestCase):
         del b[4]
         self.assertEqual(b, bytearray([1, 2, 3, 4, 6, 7, 8]))
 
-    @unittest.skip("TODO: RUSTPYTHON")
+    # TODO: RUSTPYTHON
+    @unittest.expectedFailure
     def test_setslice(self):
         b = bytearray(range(10))
         self.assertEqual(list(b), list(range(10)))

Lib/types.py

@@ -24,11 +24,10 @@ _c = _c()
 CoroutineType = type(_c)
 _c.close()  # Prevent ResourceWarning
 
-# XXX RUSTPYTHON TODO: async generators
-# async def _ag():
-#     yield
-# _ag = _ag()
-# AsyncGeneratorType = type(_ag)
+async def _ag():
+    yield
+_ag = _ag()
+AsyncGeneratorType = type(_ag)
 
 class _C:
     def _m(self): pass

compiler/src/peephole.rs

@@ -92,7 +92,7 @@ impl<O: OutputStream> PeepholeOptimizer<O> {
     }
 
     fn optimize(&mut self) {
-        apply_optimizations!(self, operator, unpack);
+        apply_optimizations!(self, operator /* , unpack */);
     }
 }
 

compiler/src/peephole/optimizations.rs

@@ -73,26 +73,27 @@ pub fn operator(buf: &mut impl OptimizationBuffer) {
     }
 }
 
-pub fn unpack(buf: &mut impl OptimizationBuffer) {
-    let (instruction, meta) = buf.pop();
-    if let Instruction::UnpackSequence { size } = instruction {
-        let (arg, arg_meta) = buf.pop();
-        match arg {
-            Instruction::BuildTuple {
-                size: tup_size,
-                unpack,
-            } if !unpack && tup_size == size => {
-                buf.emit(
-                    Instruction::Reverse { amount: size },
-                    vec![arg_meta, meta].into(),
-                );
-            }
-            arg => {
-                buf.emit(arg, arg_meta);
-                buf.emit(instruction, meta);
-            }
-        }
-    } else {
-        buf.emit(instruction, meta)
-    }
-}
+// TODO: make a version of this that doesn't miscompile `a, b = (1, 2) if True else (3, 4)`
+// pub fn unpack(buf: &mut impl OptimizationBuffer) {
+//     let (instruction, meta) = buf.pop();
+//     if let Instruction::UnpackSequence { size } = instruction {
+//         let (arg, arg_meta) = buf.pop();
+//         match arg {
+//             Instruction::BuildTuple {
+//                 size: tup_size,
+//                 unpack,
+//             } if !unpack && tup_size == size => {
+//                 buf.emit(
+//                     Instruction::Reverse { amount: size },
+//                     vec![arg_meta, meta].into(),
+//                 );
+//             }
+//             arg => {
+//                 buf.emit(arg, arg_meta);
+//                 buf.emit(instruction, meta);
+//             }
+//         }
+//     } else {
+//         buf.emit(instruction, meta)
+//     }
+// }

compiler/src/symboltable.rs

@@ -95,7 +95,6 @@ pub struct Symbol {
     pub name: String,
     // pub table: SymbolTableRef,
     pub scope: SymbolScope,
-    pub is_param: bool,
     pub is_referenced: bool,
     pub is_assigned: bool,
     pub is_parameter: bool,
@@ -108,7 +107,6 @@ impl Symbol {
             name: name.to_owned(),
             // table,
             scope: SymbolScope::Unknown,
-            is_param: false,
             is_referenced: false,
             is_assigned: false,
             is_parameter: false,
@@ -741,7 +739,6 @@ impl SymbolTableBuilder {
         Ok(())
     }
 
-    #[allow(clippy::single_match)]
     fn register_name(&mut self, name: &str, role: SymbolUsage) -> SymbolTableResult {
         let scope_depth = self.tables.len();
         let table = self.tables.last_mut().unwrap();
@@ -777,13 +774,11 @@ impl SymbolTableBuilder {
 
         // Some more checks:
         match role {
-            SymbolUsage::Nonlocal => {
-                if scope_depth < 2 {
-                    return Err(SymbolTableError {
-                        error: format!("cannot define nonlocal '{}' at top level.", name),
-                        location,
-                    });
-                }
+            SymbolUsage::Nonlocal if scope_depth < 2 => {
+                return Err(SymbolTableError {
+                    error: format!("cannot define nonlocal '{}' at top level.", name),
+                    location,
+                })
             }
             _ => {
                 // Ok!

derive/src/from_args.rs

@@ -16,14 +16,11 @@ enum ParameterKind {
 
 impl ParameterKind {
     fn from_ident(ident: &Ident) -> Option<ParameterKind> {
-        if ident == "positional_only" {
-            Some(ParameterKind::PositionalOnly)
-        } else if ident == "positional_or_keyword" {
-            Some(ParameterKind::PositionalOrKeyword)
-        } else if ident == "keyword_only" {
-            Some(ParameterKind::KeywordOnly)
-        } else {
-            None
+        match ident.to_string().as_str() {
+            "positional_only" => Some(ParameterKind::PositionalOnly),
+            "positional_or_keyword" => Some(ParameterKind::PositionalOrKeyword),
+            "keyword_only" => Some(ParameterKind::KeywordOnly),
+            _ => None,
         }
     }
 }
@@ -150,6 +147,13 @@ fn generate_field(field: &Field) -> Result<TokenStream2, Diagnostic> {
     };
 
     let name = &field.ident;
+    if let Some(name) = name {
+        if name.to_string().starts_with("_phantom") {
+            return Ok(quote! {
+                #name: std::marker::PhantomData,
+            });
+        }
+    }
     let middle = quote! {
         .map(|x| ::rustpython_vm::pyobject::TryFromObject::try_from_object(vm, x)).transpose()?
     };
@@ -210,8 +214,9 @@ pub fn impl_from_args(input: DeriveInput) -> Result<TokenStream2, Diagnostic> {
     };
 
     let name = input.ident;
+    let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
     let output = quote! {
-        impl ::rustpython_vm::function::FromArgs for #name {
+        impl #impl_generics ::rustpython_vm::function::FromArgs for #name #ty_generics #where_clause {
             fn from_args(
                 vm: &::rustpython_vm::VirtualMachine,
                 args: &mut ::rustpython_vm::function::PyFuncArgs

derive/src/lib.rs

@@ -12,6 +12,7 @@ mod error;
 mod compile_bytecode;
 mod from_args;
 mod pyclass;
+mod pymodule;
 mod util;
 
 use error::{extract_spans, Diagnostic};
@@ -44,6 +45,13 @@ pub fn pyimpl(attr: TokenStream, item: TokenStream) -> TokenStream {
     result_to_tokens(pyclass::impl_pyimpl(attr, item))
 }
 
+#[proc_macro_attribute]
+pub fn pymodule(attr: TokenStream, item: TokenStream) -> TokenStream {
+    let attr = parse_macro_input!(attr as AttributeArgs);
+    let item = parse_macro_input!(item as Item);
+    result_to_tokens(pymodule::impl_pymodule(attr, item))
+}
+
 #[proc_macro_attribute]
 pub fn pystruct_sequence(attr: TokenStream, item: TokenStream) -> TokenStream {
     let attr = parse_macro_input!(attr as AttributeArgs);

derive/src/pyclass.rs

@@ -1,11 +1,11 @@
 use super::Diagnostic;
-use crate::util::path_eq;
+use crate::util::{def_to_name, path_eq, strip_prefix, ItemIdent, ItemMeta};
 use proc_macro2::{Span, TokenStream as TokenStream2};
 use quote::{quote, quote_spanned, ToTokens};
 use std::collections::{HashMap, HashSet};
 use syn::{
     parse_quote, spanned::Spanned, Attribute, AttributeArgs, Ident, Index, Item, Lit, Meta,
-    NestedMeta, Signature,
+    NestedMeta,
 };
 
 fn meta_to_vec(meta: Meta) -> Result<Vec<NestedMeta>, Meta> {
@@ -42,181 +42,6 @@ enum ClassItem {
     },
 }
 
-struct ClassItemMeta<'a> {
-    sig: &'a Signature,
-    parent_type: &'static str,
-    meta: HashMap<String, Option<Lit>>,
-}
-
-impl<'a> ClassItemMeta<'a> {
-    const METHOD_NAMES: &'static [&'static str] = &["name", "magic"];
-    const PROPERTY_NAMES: &'static [&'static str] = &["name", "magic", "setter"];
-
-    fn from_nested_meta(
-        parent_type: &'static str,
-        sig: &'a Signature,
-        nested_meta: &[NestedMeta],
-        names: &[&'static str],
-    ) -> Result<Self, Diagnostic> {
-        let mut extracted = Self {
-            sig,
-            parent_type,
-            meta: HashMap::new(),
-        };
-
-        let validate_name = |name: &str, extracted: &Self| -> Result<(), Diagnostic> {
-            if names.contains(&name) {
-                if extracted.meta.contains_key(name) {
-                    bail_span!(
-                        &sig.ident,
-                        "#[{}] must have only one '{}'",
-                        parent_type,
-                        name
-                    );
-                } else {
-                    Ok(())
-                }
-            } else {
-                bail_span!(
-                    &sig.ident,
-                    "#[{}({})] is not one of allowed attributes {}",
-                    parent_type,
-                    name,
-                    names.join(", ")
-                );
-            }
-        };
-
-        for meta in nested_meta {
-            let meta = match meta {
-                NestedMeta::Meta(meta) => meta,
-                NestedMeta::Lit(_) => continue,
-            };
-
-            match meta {
-                Meta::NameValue(name_value) => {
-                    if let Some(ident) = name_value.path.get_ident() {
-                        let name = ident.to_string();
-                        validate_name(&name, &extracted)?;
-                        extracted.meta.insert(name, Some(name_value.lit.clone()));
-                    }
-                }
-                Meta::Path(path) => {
-                    if let Some(ident) = path.get_ident() {
-                        let name = ident.to_string();
-                        validate_name(&name, &extracted)?;
-                        extracted.meta.insert(name, None);
-                    } else {
-                        continue;
-                    }
-                }
-                _ => (),
-            }
-        }
-
-        Ok(extracted)
-    }
-
-    fn _str(&self, key: &str) -> Result<Option<String>, Diagnostic> {
-        Ok(match self.meta.get(key) {
-            Some(Some(lit)) => {
-                if let Lit::Str(s) = lit {
-                    Some(s.value())
-                } else {
-                    bail_span!(
-                        &self.sig.ident,
-                        "#[{}({} = ...)] must be a string",
-                        self.parent_type,
-                        key
-                    );
-                }
-            }
-            Some(None) => {
-                bail_span!(
-                    &self.sig.ident,
-                    "#[{}({} = ...)] is expected",
-                    self.parent_type,
-                    key,
-                );
-            }
-            None => None,
-        })
-    }
-
-    fn _bool(&self, key: &str) -> Result<bool, Diagnostic> {
-        Ok(match self.meta.get(key) {
-            Some(Some(_)) => {
-                bail_span!(
-                    &self.sig.ident,
-                    "#[{}({})] is expected",
-                    self.parent_type,
-                    key,
-                );
-            }
-            Some(None) => true,
-            None => false,
-        })
-    }
-
-    fn method_name(&self) -> Result<String, Diagnostic> {
-        let name = self._str("name")?;
-        let magic = self._bool("magic")?;
-        Ok(if let Some(name) = name {
-            name
-        } else {
-            let name = self.sig.ident.to_string();
-            if magic {
-                format!("__{}__", name)
-            } else {
-                name
-            }
-        })
-    }
-
-    fn setter(&self) -> Result<bool, Diagnostic> {
-        self._bool("setter")
-    }
-
-    fn property_name(&self) -> Result<String, Diagnostic> {
-        let magic = self._bool("magic")?;
-        let setter = self._bool("setter")?;
-        let name = self._str("name")?;
-
-        Ok(if let Some(name) = name {
-            name
-        } else {
-            let sig_name = self.sig.ident.to_string();
-            let name = if setter {
-                if let Some(name) = strip_prefix(&sig_name, "set_") {
-                    if name.is_empty() {
-                        bail_span!(
-                            &self.sig.ident,
-                            "A #[{}(setter)] fn with a set_* name must \
-                             have something after \"set_\"",
-                            self.parent_type
-                        )
-                    }
-                    name.to_string()
-                } else {
-                    bail_span!(
-                        &self.sig.ident,
-                        "A #[{}(setter)] fn must either have a `name` \
-                         parameter or a fn name along the lines of \"set_*\"",
-                        self.parent_type
-                    )
-                }
-            } else {
-                sig_name
-            };
-            if magic {
-                format!("__{}__", name)
-            } else {
-                name
-            }
-        })
-    }
-}
-
 impl Class {
     fn add_item(&mut self, item: ClassItem, span: Span) -> Result<(), Diagnostic> {
         if self.items.insert(item) {
@@ -229,7 +54,7 @@ impl Class {
         }
     }
 
-    fn extract_method(sig: &Signature, meta: Meta) -> Result<ClassItem, Diagnostic> {
+    fn extract_method(ident: &Ident, meta: Meta) -> Result<ClassItem, Diagnostic> {
         let nesteds = meta_to_vec(meta).map_err(|meta| {
             err_span!(
                 meta,
@@ -238,19 +63,15 @@ impl Class {
             )
         })?;
 
-        let item_meta = ClassItemMeta::from_nested_meta(
-            "pymethod",
-            sig,
-            &nesteds,
-            ClassItemMeta::METHOD_NAMES,
-        )?;
+        let item_meta =
+            ItemMeta::from_nested_meta("pymethod", &ident, &nesteds, ItemMeta::ATTRIBUTE_NAMES)?;
         Ok(ClassItem::Method {
-            item_ident: sig.ident.clone(),
+            item_ident: ident.clone(),
             py_name: item_meta.method_name()?,
         })
     }
 
-    fn extract_classmethod(sig: &Signature, meta: Meta) -> Result<ClassItem, Diagnostic> {
+    fn extract_classmethod(ident: &Ident, meta: Meta) -> Result<ClassItem, Diagnostic> {
         let nesteds = meta_to_vec(meta).map_err(|meta| {
             err_span!(
                 meta,
@@ -258,19 +79,19 @@ impl Class {
                  #[pyclassmethod(name = \"...\")]",
             )
         })?;
-        let item_meta = ClassItemMeta::from_nested_meta(
+        let item_meta = ItemMeta::from_nested_meta(
             "pyclassmethod",
-            sig,
+            &ident,
             &nesteds,
-            ClassItemMeta::METHOD_NAMES,
+            ItemMeta::ATTRIBUTE_NAMES,
         )?;
         Ok(ClassItem::ClassMethod {
-            item_ident: sig.ident.clone(),
+            item_ident: ident.clone(),
             py_name: item_meta.method_name()?,
         })
     }
 
-    fn extract_property(sig: &Signature, meta: Meta) -> Result<ClassItem, Diagnostic> {
+    fn extract_property(ident: &Ident, meta: Meta) -> Result<ClassItem, Diagnostic> {
         let nesteds = meta_to_vec(meta).map_err(|meta| {
             err_span!(
                 meta,
@@ -278,31 +99,27 @@ impl Class {
                  #[pyproperty(name = \"...\")]"
             )
         })?;
-        let item_meta = ClassItemMeta::from_nested_meta(
-            "pyproperty",
-            sig,
-            &nesteds,
-            ClassItemMeta::PROPERTY_NAMES,
-        )?;
+        let item_meta =
+            ItemMeta::from_nested_meta("pyproperty", &ident, &nesteds, ItemMeta::PROPERTY_NAMES)?;
         Ok(ClassItem::Property {
             py_name: item_meta.property_name()?,
-            item_ident: sig.ident.clone(),
+            item_ident: ident.clone(),
             setter: item_meta.setter()?,
         })
     }
 
-    fn extract_slot(sig: &Signature, meta: Meta) -> Result<ClassItem, Diagnostic> {
+    fn extract_slot(ident: &Ident, meta: Meta) -> Result<ClassItem, Diagnostic> {
         let pyslot_err = "#[pyslot] must be of the form #[pyslot] or #[pyslot(slotname)]";
         let nesteds = meta_to_vec(meta).map_err(|meta| err_span!(meta, "{}", pyslot_err))?;
         if nesteds.len() > 1 {
             return Err(Diagnostic::spanned_error(&quote!(#(#nesteds)*), pyslot_err));
         }
         let slot_ident = if nesteds.is_empty() {
-            let ident_str = sig.ident.to_string();
+            let ident_str = ident.to_string();
             if let Some(stripped) = strip_prefix(&ident_str, "tp_") {
-                proc_macro2::Ident::new(stripped, sig.ident.span())
+                proc_macro2::Ident::new(stripped, ident.span())
             } else {
-                sig.ident.clone()
+                ident.clone()
             }
         } else {
             match nesteds.into_iter().next().unwrap() {
@@ -315,14 +132,14 @@ impl Class {
         };
         Ok(ClassItem::Slot {
             slot_ident,
-            item_ident: sig.ident.clone(),
+            item_ident: ident.clone(),
         })
     }
 
     fn extract_item_from_syn(
         &mut self,
         attrs: &mut Vec<Attribute>,
-        sig: &Signature,
+        ident: &Ident,
     ) -> Result<(), Diagnostic> {
         let mut attr_idxs = Vec::new();
         for (i, meta) in attrs
@@ -335,17 +152,16 @@ impl Class {
                 Some(name) => name,
                 None => continue,
             };
-            if name == "pymethod" {
-                self.add_item(Self::extract_method(sig, meta)?, meta_span)?;
-            } else if name == "pyclassmethod" {
-                self.add_item(Self::extract_classmethod(sig, meta)?, meta_span)?;
-            } else if name == "pyproperty" {
-                self.add_item(Self::extract_property(sig, meta)?, meta_span)?;
-            } else if name == "pyslot" {
-                self.add_item(Self::extract_slot(sig, meta)?, meta_span)?;
-            } else {
-                continue;
-            }
+            let item = match name.to_string().as_str() {
+                "pymethod" => Self::extract_method(ident, meta)?,
+                "pyclassmethod" => Self::extract_classmethod(ident, meta)?,
+                "pyproperty" => Self::extract_property(ident, meta)?,
+                "pyslot" => Self::extract_slot(ident, meta)?,
+                _ => {
+                    continue;
+                }
+            };
+            self.add_item(item, meta_span)?;
             attr_idxs.push(i);
         }
         let mut i = 0;
@@ -365,12 +181,7 @@ impl Class {
     }
 }
 
-struct ItemSig<'a> {
-    attrs: &'a mut Vec<Attribute>,
-    sig: &'a Signature,
-}
-
-fn extract_impl_items(mut items: Vec<ItemSig>) -> Result<TokenStream2, Diagnostic> {
+fn extract_impl_items(mut items: Vec<ItemIdent>) -> Result<TokenStream2, Diagnostic> {
     let mut diagnostics: Vec<Diagnostic> = Vec::new();
 
     let mut class = Class::default();
@@ -378,7 +189,7 @@ fn extract_impl_items(mut items: Vec<ItemSig>) -> Result<TokenStream2, Diagnosti
     for item in items.iter_mut() {
         push_diag_result!(
             diagnostics,
-            class.extract_item_from_syn(&mut item.attrs, item.sig),
+            class.extract_item_from_syn(&mut item.attrs, &item.ident),
         );
     }
 
@@ -544,7 +355,10 @@ pub fn impl_pyimpl(attr: AttributeArgs, item: Item) -> Result<TokenStream2, Diag
                 .iter_mut()
                 .filter_map(|item| match item {
                     syn::ImplItem::Method(syn::ImplItemMethod { attrs, sig, .. }) => {
-                        Some(ItemSig { attrs, sig })
+                        Some(ItemIdent {
+                            attrs,
+                            ident: &sig.ident,
+                        })
                     }
                     _ => None,
                 })
@@ -574,7 +388,10 @@ pub fn impl_pyimpl(attr: AttributeArgs, item: Item) -> Result<TokenStream2, Diag
                 .iter_mut()
                 .filter_map(|item| match item {
                     syn::TraitItem::Method(syn::TraitItemMethod { attrs, sig, .. }) => {
-                        Some(ItemSig { attrs, sig })
+                        Some(ItemIdent {
+                            attrs,
+                            ident: &sig.ident,
+                        })
                     }
                     _ => None,
                 })
@@ -597,30 +414,9 @@ pub fn impl_pyimpl(attr: AttributeArgs, item: Item) -> Result<TokenStream2, Diag
 
 fn generate_class_def(
     ident: &Ident,
-    attr_name: &'static str,
-    attr: AttributeArgs,
+    name: &str,
     attrs: &[Attribute],
 ) -> Result<TokenStream2, Diagnostic> {
-    let mut class_name = None;
-    for attr in attr {
-        if let NestedMeta::Meta(meta) = attr {
-            if let Meta::NameValue(name_value) = meta {
-                if path_eq(&name_value.path, "name") {
-                    if let Lit::Str(s) = name_value.lit {
-                        class_name = Some(s.value());
-                    } else {
-                        bail_span!(
-                            name_value.lit,
-                            "#[{}(name = ...)] must be a string",
-                            attr_name
-                        );
-                    }
-                }
-            }
-        }
-    }
-    let class_name = class_name.unwrap_or_else(|| ident.to_string());
-
     let mut doc: Option<Vec<String>> = None;
     for attr in attrs.iter() {
         if attr.path.is_ident("doc") {
@@ -646,7 +442,7 @@ fn generate_class_def(
 
     let ret = quote! {
         impl ::rustpython_vm::pyobject::PyClassDef for #ident {
-            const NAME: &'static str = #class_name;
+            const NAME: &'static str = #name;
             const DOC: Option<&'static str> = #doc;
         }
     };
@@ -663,7 +459,8 @@ pub fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStream2, Dia
         ),
     };
 
-    let class_def = generate_class_def(&ident, "pyclass", attr, &attrs)?;
+    let class_name = def_to_name(&ident, "pyclass", attr)?;
+    let class_def = generate_class_def(&ident, &class_name, &attrs)?;
 
     let ret = quote! {
         #item
@@ -681,7 +478,8 @@ pub fn impl_pystruct_sequence(attr: AttributeArgs, item: Item) -> Result<TokenSt
             "#[pystruct_sequence] can only be on a struct declaration"
         )
     };
-    let class_def = generate_class_def(&struc.ident, "pystruct_sequence", attr, &struc.attrs)?;
+    let class_name = def_to_name(&struc.ident, "pystruct_sequence", attr)?;
+    let class_def = generate_class_def(&struc.ident, &class_name, &struc.attrs)?;
     let mut properties = Vec::new();
     let mut field_names = Vec::new();
     for (i, field) in struc.fields.iter().enumerate() {
@@ -745,11 +543,3 @@ pub fn impl_pystruct_sequence(attr: AttributeArgs, item: Item) -> Result<TokenSt
     };
     Ok(ret)
 }
-
-fn strip_prefix<'a>(s: &'a str, prefix: &str) -> Option<&'a str> {
-    if s.starts_with(prefix) {
-        Some(&s[prefix.len()..])
-    } else {
-        None
-    }
-}

derive/src/pymodule.rs

@@ -0,0 +1,217 @@
+use super::Diagnostic;
+use crate::util::{def_to_name, ItemIdent, ItemMeta};
+use proc_macro2::{Span, TokenStream as TokenStream2};
+use quote::{quote, quote_spanned};
+use std::collections::HashSet;
+use syn::{parse_quote, spanned::Spanned, Attribute, AttributeArgs, Ident, Item, Meta, NestedMeta};
+
+fn meta_to_vec(meta: Meta) -> Result<Vec<NestedMeta>, Meta> {
+    match meta {
+        Meta::Path(_) => Ok(Vec::new()),
+        Meta::List(list) => Ok(list.nested.into_iter().collect()),
+        Meta::NameValue(_) => Err(meta),
+    }
+}
+
+#[derive(Default)]
+struct Module {
+    items: HashSet<ModuleItem>,
+}
+
+#[derive(PartialEq, Eq, Hash)]
+enum ModuleItem {
+    Function { item_ident: Ident, py_name: String },
+    Class { item_ident: Ident, py_name: String },
+}
+
+impl Module {
+    fn add_item(&mut self, item: ModuleItem, span: Span) -> Result<(), Diagnostic> {
+        if self.items.insert(item) {
+            Ok(())
+        } else {
+            Err(Diagnostic::span_error(
+                span,
+                "Duplicate #[py*] attribute on pyimpl".to_owned(),
+            ))
+        }
+    }
+
+    fn extract_function(ident: &Ident, meta: Meta) -> Result<ModuleItem, Diagnostic> {
+        let nesteds = meta_to_vec(meta).map_err(|meta| {
+            err_span!(
+                meta,
+                "#[pyfunction = \"...\"] cannot be a name/value, you probably meant \
+                 #[pyfunction(name = \"...\")]",
+            )
+        })?;
+
+        let item_meta =
+            ItemMeta::from_nested_meta("pyfunction", &ident, &nesteds, ItemMeta::SIMPLE_NAMES)?;
+        Ok(ModuleItem::Function {
+            item_ident: ident.clone(),
+            py_name: item_meta.simple_name()?,
+        })
+    }
+
+    fn extract_class(ident: &Ident, meta: Meta) -> Result<ModuleItem, Diagnostic> {
+        let nesteds = meta_to_vec(meta).map_err(|meta| {
+            err_span!(
+                meta,
+                "#[pyclass = \"...\"] cannot be a name/value, you probably meant \
+                 #[pyclass(name = \"...\")]",
+            )
+        })?;
+
+        let item_meta =
+            ItemMeta::from_nested_meta("pyclass", &ident, &nesteds, ItemMeta::SIMPLE_NAMES)?;
+        Ok(ModuleItem::Class {
+            item_ident: ident.clone(),
+            py_name: item_meta.simple_name()?,
+        })
+    }
+
+    fn extract_item_from_syn(
+        &mut self,
+        attrs: &mut Vec<Attribute>,
+        ident: &Ident,
+    ) -> Result<(), Diagnostic> {
+        let mut attr_idxs = Vec::new();
+        for (i, meta) in attrs
+            .iter()
+            .filter_map(|attr| attr.parse_meta().ok())
+            .enumerate()
+        {
+            let meta_span = meta.span();
+            let name = match meta.path().get_ident() {
+                Some(name) => name,
+                None => continue,
+            };
+            let item = match name.to_string().as_str() {
+                "pyfunction" => {
+                    attr_idxs.push(i);
+                    Self::extract_function(ident, meta)?
+                }
+                "pyclass" => Self::extract_class(ident, meta)?,
+                _ => {
+                    continue;
+                }
+            };
+            self.add_item(item, meta_span)?;
+        }
+        let mut i = 0;
+        let mut attr_idxs = &*attr_idxs;
+        attrs.retain(|_| {
+            let drop = attr_idxs.first().copied() == Some(i);
+            if drop {
+                attr_idxs = &attr_idxs[1..];
+            }
+            i += 1;
+            !drop
+        });
+        for (i, idx) in attr_idxs.iter().enumerate() {
+            attrs.remove(idx - i);
+        }
+        Ok(())
+    }
+}
+
+fn extract_module_items(mut items: Vec<ItemIdent>) -> Result<TokenStream2, Diagnostic> {
+    let mut diagnostics: Vec<Diagnostic> = Vec::new();
+
+    let mut module = Module::default();
+
+    for item in items.iter_mut() {
+        push_diag_result!(
+            diagnostics,
+            module.extract_item_from_syn(&mut item.attrs, item.ident),
+        );
+    }
+
+    let functions = module.items.into_iter().map(|item| match item {
+        ModuleItem::Function {
+            item_ident,
+            py_name,
+        } => {
+            let new_func = quote_spanned!(item_ident.span() => .new_function(#item_ident));
+            quote! {
+                vm.__module_set_attr(&module, #py_name, vm.ctx#new_func).unwrap();
+            }
+        }
+        ModuleItem::Class {
+            item_ident,
+            py_name,
+        } => {
+            let new_class = quote_spanned!(item_ident.span() => #item_ident::make_class(&vm.ctx));
+            quote! {
+                vm.__module_set_attr(&module, #py_name, #new_class).unwrap();
+            }
+        }
+    });
+
+    Diagnostic::from_vec(diagnostics)?;
+
+    Ok(quote! {
+        #(#functions)*
+    })
+}
+
+pub fn impl_pymodule(attr: AttributeArgs, item: Item) -> Result<TokenStream2, Diagnostic> {
+    match item {
+        Item::Mod(mut module) => {
+            let module_name = def_to_name(&module.ident, "pymodule", attr)?;
+
+            if let Some(content) = module.content.as_mut() {
+                let items = content
+                    .1
+                    .iter_mut()
+                    .filter_map(|item| match item {
+                        Item::Fn(syn::ItemFn { attrs, sig, .. }) => Some(ItemIdent {
+                            attrs,
+                            ident: &sig.ident,
+                        }),
+                        Item::Struct(syn::ItemStruct { attrs, ident, .. }) => {
+                            Some(ItemIdent { attrs, ident })
+                        }
+                        Item::Enum(syn::ItemEnum { attrs, ident, .. }) => {
+                            Some(ItemIdent { attrs, ident })
+                        }
+                        _ => None,
+                    })
+                    .collect();
+
+                let extend_mod = extract_module_items(items)?;
+                content.1.push(parse_quote! {
+                    const MODULE_NAME: &str = #module_name;
+                });
+                content.1.push(parse_quote! {
+                    pub(crate) fn extend_module(
+                        vm: &::rustpython_vm::vm::VirtualMachine,
+                        module: &::rustpython_vm::pyobject::PyObjectRef,
+                    ) {
+                        #extend_mod
+                    }
+                });
+                content.1.push(parse_quote! {
+                    #[allow(dead_code)]
+                    pub(crate) fn make_module(
+                        vm: &::rustpython_vm::vm::VirtualMachine
+                    ) -> ::rustpython_vm::pyobject::PyObjectRef {
+                        let module = vm.new_module(MODULE_NAME, vm.ctx.new_dict());
+                        extend_module(vm, &module);
+                        module
+                    }
+                });
+
+                Ok(quote! {
+                    #module
+                })
+            } else {
+                bail_span!(
+                    module,
+                    "#[pymodule] can only be on a module declaration with body"
+                )
+            }
+        }
+        other => bail_span!(other, "#[pymodule] can only be on a module declaration"),
+    }
+}

derive/src/util.rs

@@ -1,3 +1,216 @@
-pub fn path_eq(path: &syn::Path, s: &str) -> bool {
+use super::Diagnostic;
+use std::collections::HashMap;
+use syn::{Attribute, AttributeArgs, Ident, Lit, Meta, NestedMeta, Path};
+
+pub fn path_eq(path: &Path, s: &str) -> bool {
     path.get_ident().map_or(false, |id| id == s)
 }
+
+pub fn def_to_name(
+    ident: &Ident,
+    attr_name: &'static str,
+    attr: AttributeArgs,
+) -> Result<String, Diagnostic> {
+    let mut name = None;
+    for attr in attr {
+        if let NestedMeta::Meta(meta) = attr {
+            if let Meta::NameValue(name_value) = meta {
+                if path_eq(&name_value.path, "name") {
+                    if let Lit::Str(s) = name_value.lit {
+                        name = Some(s.value());
+                    } else {
+                        bail_span!(
+                            name_value.lit,
+                            "#[{}(name = ...)] must be a string",
+                            attr_name
+                        );
+                    }
+                }
+            }
+        }
+    }
+    Ok(name.unwrap_or_else(|| ident.to_string()))
+}
+
+pub fn strip_prefix<'a>(s: &'a str, prefix: &str) -> Option<&'a str> {
+    if s.starts_with(prefix) {
+        Some(&s[prefix.len()..])
+    } else {
+        None
+    }
+}
+
+pub struct ItemIdent<'a> {
+    pub attrs: &'a mut Vec<Attribute>,
+    pub ident: &'a Ident,
+}
+
+pub struct ItemMeta<'a> {
+    ident: &'a Ident,
+    parent_type: &'static str,
+    meta: HashMap<String, Option<Lit>>,
+}
+
+impl<'a> ItemMeta<'a> {
+    pub const SIMPLE_NAMES: &'static [&'static str] = &["name"];
+    pub const ATTRIBUTE_NAMES: &'static [&'static str] = &["name", "magic"];
+    pub const PROPERTY_NAMES: &'static [&'static str] = &["name", "magic", "setter"];
+
+    pub fn from_nested_meta(
+        parent_type: &'static str,
+        ident: &'a Ident,
+        nested_meta: &[NestedMeta],
+        names: &[&'static str],
+    ) -> Result<Self, Diagnostic> {
+        let mut extracted = Self {
+            ident,
+            parent_type,
+            meta: HashMap::new(),
+        };
+
+        let validate_name = |name: &str, extracted: &Self| -> Result<(), Diagnostic> {
+            if names.contains(&name) {
+                if extracted.meta.contains_key(name) {
+                    bail_span!(ident, "#[{}] must have only one '{}'", parent_type, name);
+                } else {
+                    Ok(())
+                }
+            } else {
+                bail_span!(
+                    ident,
+                    "#[{}({})] is not one of allowed attributes {}",
+                    parent_type,
+                    name,
+                    names.join(", ")
+                );
+            }
+        };
+
+        for meta in nested_meta {
+            let meta = match meta {
+                NestedMeta::Meta(meta) => meta,
+                NestedMeta::Lit(_) => continue,
+            };
+
+            match meta {
+                Meta::NameValue(name_value) => {
+                    if let Some(ident) = name_value.path.get_ident() {
+                        let name = ident.to_string();
+                        validate_name(&name, &extracted)?;
+                        extracted.meta.insert(name, Some(name_value.lit.clone()));
+                    }
+                }
+                Meta::Path(path) => {
+                    if let Some(ident) = path.get_ident() {
+                        let name = ident.to_string();
+                        validate_name(&name, &extracted)?;
+                        extracted.meta.insert(name, None);
+                    } else {
+                        continue;
+                    }
+                }
+                _ => (),
+            }
+        }
+
+        Ok(extracted)
+    }
+
+    fn _str(&self, key: &str) -> Result<Option<String>, Diagnostic> {
+        Ok(match self.meta.get(key) {
+            Some(Some(lit)) => {
+                if let Lit::Str(s) = lit {
+                    Some(s.value())
+                } else {
+                    bail_span!(
+                        &self.ident,
+                        "#[{}({} = ...)] must be a string",
+                        self.parent_type,
+                        key
+                    );
+                }
+            }
+            Some(None) => {
+                bail_span!(
+                    &self.ident,
+                    "#[{}({} = ...)] is expected",
+                    self.parent_type,
+                    key,
+                );
+            }
+            None => None,
+        })
+    }
+
+    fn _bool(&self, key: &str) -> Result<bool, Diagnostic> {
+        Ok(match self.meta.get(key) {
+            Some(Some(_)) => {
+                bail_span!(&self.ident, "#[{}({})] is expected", self.parent_type, key,);
+            }
+            Some(None) => true,
+            None => false,
+        })
+    }
+
+    pub fn simple_name(&self) -> Result<String, Diagnostic> {
+        Ok(self._str("name")?.unwrap_or_else(|| self.ident.to_string()))
+    }
+
+    pub fn method_name(&self) -> Result<String, Diagnostic> {
+        let name = self._str("name")?;
+        let magic = self._bool("magic")?;
+        Ok(if let Some(name) = name {
+            name
+        } else {
+            let name = self.ident.to_string();
+            if magic {
+                format!("__{}__", name)
+            } else {
+                name
+            }
+        })
+    }
+
+    pub fn property_name(&self) -> Result<String, Diagnostic> {
+        let magic = self._bool("magic")?;
+        let setter = self._bool("setter")?;
+        let name = self._str("name")?;
+
+        Ok(if let Some(name) = name {
+            name
+        } else {
+            let sig_name = self.ident.to_string();
+            let name = if setter {
+                if let Some(name) = strip_prefix(&sig_name, "set_") {
+                    if name.is_empty() {
+                        bail_span!(
+                            &self.ident,
+                            "A #[{}(setter)] fn with a set_* name must \
+                             have something after \"set_\"",
+                            self.parent_type
+                        )
+                    }
+                    name.to_string()
+                } else {
+                    bail_span!(
+                        &self.ident,
+                        "A #[{}(setter)] fn must either have a `name` \
+                         parameter or a fn name along the lines of \"set_*\"",
+                        self.parent_type
+                    )
+                }
+            } else {
+                sig_name
+            };
+            if magic {
+                format!("__{}__", name)
+            } else {
+                name
+            }
+        })
+    }
+
+    pub fn setter(&self) -> Result<bool, Diagnostic> {
+        self._bool("setter")
+    }
+}

parser/build.rs

@@ -1,5 +1,3 @@
-use lalrpop;
-
 fn main() {
     lalrpop::process_root().unwrap()
 }

parser/src/function.rs

@@ -6,7 +6,6 @@ use crate::error::{LexicalError, LexicalErrorType};
 type ParameterDefs = (Vec<ast::Parameter>, Vec<ast::Expression>);
 type ParameterDef = (ast::Parameter, Option<ast::Expression>);
 
-#[allow(clippy::collapsible_if)]
 pub fn parse_params(
     params: (Vec<ParameterDef>, Vec<ParameterDef>),
 ) -> Result<ParameterDefs, LexicalError> {
@@ -16,15 +15,13 @@ pub fn parse_params(
     let mut try_default = |name: &ast::Parameter, default| {
         if let Some(default) = default {
             defaults.push(default);
-        } else {
-            if !defaults.is_empty() {
-                // Once we have started with defaults, all remaining arguments must
-                // have defaults
-                return Err(LexicalError {
-                    error: LexicalErrorType::DefaultArgumentError,
-                    location: name.location,
-                });
-            }
+        } else if !defaults.is_empty() {
+            // Once we have started with defaults, all remaining arguments must
+            // have defaults
+            return Err(LexicalError {
+                error: LexicalErrorType::DefaultArgumentError,
+                location: name.location,
+            });
         }
         Ok(())
     };

tests/snippets/if_expressions.py

@@ -24,3 +24,7 @@ def func2():
     return 20
 
 assert ret(func1() or func2()) == 20
+
+a, b = (1, 2) if True else (3, 4)
+assert a == 1
+assert b == 2

vm/src/dictdatatype.rs

@@ -1,6 +1,6 @@
 use crate::obj::objstr::PyString;
 use crate::pyhash;
-use crate::pyobject::{IdProtocol, IntoPyObject, PyObjectRef, PyResult};
+use crate::pyobject::{IdProtocol, IntoPyObject, PyObjectRef, PyResult, ThreadSafe};
 use crate::vm::VirtualMachine;
 use num_bigint::ToBigInt;
 /// Ordered dictionary implementation.
@@ -10,6 +10,7 @@ use num_bigint::ToBigInt;
 use std::collections::{hash_map::DefaultHasher, HashMap};
 use std::hash::{Hash, Hasher};
 use std::mem::size_of;
+use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
 
 /// hash value of an object returned by __hash__
 type HashValue = pyhash::PyHash;
@@ -18,30 +19,64 @@ type HashIndex = pyhash::PyHash;
 /// entry index mapped in indices
 type EntryIndex = usize;
 
-#[derive(Clone)]
 pub struct Dict<T = PyObjectRef> {
+    inner: RwLock<InnerDict<T>>,
+}
+
+impl ThreadSafe for Dict {}
+
+struct InnerDict<T> {
     size: usize,
     indices: HashMap<HashIndex, EntryIndex>,
     entries: Vec<Option<DictEntry<T>>>,
 }
 
+impl<T: Clone> Clone for InnerDict<T> {
+    fn clone(&self) -> Self {
+        InnerDict {
+            size: self.size,
+            indices: self.indices.clone(),
+            entries: self.entries.clone(),
+        }
+    }
+}
+
+impl<T: Clone> Clone for Dict<T> {
+    fn clone(&self) -> Self {
+        Dict {
+            inner: RwLock::new(self.inner.read().unwrap().clone()),
+        }
+    }
+}
+
 impl<T> Default for Dict<T> {
     fn default() -> Self {
         Dict {
-            size: 0,
-            indices: HashMap::new(),
-            entries: Vec::new(),
+            inner: RwLock::new(InnerDict {
+                size: 0,
+                indices: HashMap::new(),
+                entries: Vec::new(),
+            }),
         }
     }
 }
 
-#[derive(Clone)]
 struct DictEntry<T> {
     hash: HashValue,
     key: PyObjectRef,
     value: T,
 }
 
+impl<T: Clone> Clone for DictEntry<T> {
+    fn clone(&self) -> Self {
+        DictEntry {
+            hash: self.hash,
+            key: self.key.clone(),
+            value: self.value.clone(),
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct DictSize {
     size: usize,
@@ -49,10 +84,19 @@ pub struct DictSize {
 }
 
 impl<T: Clone> Dict<T> {
-    fn resize(&mut self) {
-        let mut new_indices = HashMap::with_capacity(self.size);
-        let mut new_entries = Vec::with_capacity(self.size);
-        for maybe_entry in self.entries.drain(0..) {
+    fn borrow_value(&self) -> RwLockReadGuard<'_, InnerDict<T>> {
+        self.inner.read().unwrap()
+    }
+
+    fn borrow_value_mut(&self) -> RwLockWriteGuard<'_, InnerDict<T>> {
+        self.inner.write().unwrap()
+    }
+
+    fn resize(&self) {
+        let mut inner = self.borrow_value_mut();
+        let mut new_indices = HashMap::with_capacity(inner.size);
+        let mut new_entries = Vec::with_capacity(inner.size);
+        for maybe_entry in inner.entries.drain(0..) {
             if let Some(entry) = maybe_entry {
                 let mut hash_index = entry.hash;
                 // Faster version of lookup. No equality checks here.
@@ -64,12 +108,12 @@ impl<T: Clone> Dict<T> {
                 new_entries.push(Some(entry));
             }
         }
-        self.indices = new_indices;
-        self.entries = new_entries;
+        inner.indices = new_indices;
+        inner.entries = new_entries;
     }
 
     fn unchecked_push(
-        &mut self,
+        &self,
         hash_index: HashIndex,
         hash_value: HashValue,
         key: PyObjectRef,
@@ -80,44 +124,46 @@ impl<T: Clone> Dict<T> {
             key,
             value,
         };
-        let entry_index = self.entries.len();
-        self.entries.push(Some(entry));
-        self.indices.insert(hash_index, entry_index);
-        self.size += 1;
-    }
-
-    fn unchecked_delete(&mut self, entry_index: EntryIndex) {
-        self.entries[entry_index] = None;
-        self.size -= 1;
+        let mut inner = self.borrow_value_mut();
+        let entry_index = inner.entries.len();
+        inner.entries.push(Some(entry));
+        inner.indices.insert(hash_index, entry_index);
+        inner.size += 1;
     }
 
     /// Store a key
     pub fn insert<K: DictKey + IntoPyObject + Copy>(
-        &mut self,
+        &self,
         vm: &VirtualMachine,
         key: K,
         value: T,
     ) -> PyResult<()> {
-        if self.indices.len() > 2 * self.size {
+        // This does not need to be accurate so we can take the lock mutiple times.
+        if self.borrow_value().indices.len() > 2 * self.borrow_value().size {
             self.resize();
         }
-        match self.lookup(vm, key)? {
-            LookupResult::Existing(index) => {
-                // Update existing key
-                if let Some(ref mut entry) = self.entries[index] {
-                    entry.value = value;
-                    Ok(())
-                } else {
-                    panic!("Lookup returned invalid index into entries!");
+        loop {
+            match self.lookup(vm, key)? {
+                LookupResult::Existing(index) => {
+                    let mut inner = self.borrow_value_mut();
+                    // Update existing key
+                    if let Some(ref mut entry) = inner.entries[index] {
+                        // They entry might have changed since we did lookup. Should we update the key?
+                        entry.value = value;
+                        break Ok(());
+                    } else {
+                        // The dict was changed since we did lookup. Let's try again.
+                        continue;
+                    }
+                }
+                LookupResult::NewIndex {
+                    hash_index,
+                    hash_value,
+                } => {
+                    // New key:
+                    self.unchecked_push(hash_index, hash_value, key.into_pyobject(vm)?, value);
+                    break Ok(());
                 }
-            }
-            LookupResult::NewIndex {
-                hash_index,
-                hash_value,
-            } => {
-                // New key:
-                self.unchecked_push(hash_index, hash_value, key.into_pyobject(vm)?, value);
-                Ok(())
             }
         }
     }
@@ -130,32 +176,32 @@ impl<T: Clone> Dict<T> {
         }
     }
 
-    fn unchecked_get(&self, index: EntryIndex) -> T {
-        if let Some(entry) = &self.entries[index] {
-            entry.value.clone()
-        } else {
-            panic!("Lookup returned invalid index into entries!");
-        }
-    }
-
     /// Retrieve a key
     #[cfg_attr(feature = "flame-it", flame("Dict"))]
     pub fn get<K: DictKey + Copy>(&self, vm: &VirtualMachine, key: K) -> PyResult<Option<T>> {
-        if let LookupResult::Existing(index) = self.lookup(vm, key)? {
-            Ok(Some(self.unchecked_get(index)))
-        } else {
-            Ok(None)
+        loop {
+            if let LookupResult::Existing(index) = self.lookup(vm, key)? {
+                if let Some(entry) = &self.borrow_value().entries[index] {
+                    break Ok(Some(entry.value.clone()));
+                } else {
+                    // The dict was changed since we did lookup. Let's try again.
+                    continue;
+                }
+            } else {
+                break Ok(None);
+            }
         }
     }
 
-    pub fn clear(&mut self) {
-        self.entries.clear();
-        self.indices.clear();
-        self.size = 0
+    pub fn clear(&self) {
+        let mut inner = self.borrow_value_mut();
+        inner.entries.clear();
+        inner.indices.clear();
+        inner.size = 0
     }
 
     /// Delete a key
-    pub fn delete(&mut self, vm: &VirtualMachine, key: &PyObjectRef) -> PyResult<()> {
+    pub fn delete(&self, vm: &VirtualMachine, key: &PyObjectRef) -> PyResult<()> {
         if self.delete_if_exists(vm, key)? {
             Ok(())
         } else {
@@ -163,33 +209,56 @@ impl<T: Clone> Dict<T> {
         }
     }
 
-    pub fn delete_if_exists(&mut self, vm: &VirtualMachine, key: &PyObjectRef) -> PyResult<bool> {
-        if let LookupResult::Existing(entry_index) = self.lookup(vm, key)? {
-            self.unchecked_delete(entry_index);
-            Ok(true)
-        } else {
-            Ok(false)
+    pub fn delete_if_exists(&self, vm: &VirtualMachine, key: &PyObjectRef) -> PyResult<bool> {
+        loop {
+            if let LookupResult::Existing(entry_index) = self.lookup(vm, key)? {
+                let mut inner = self.borrow_value_mut();
+                if inner.entries[entry_index].is_some() {
+                    inner.entries[entry_index] = None;
+                    inner.size -= 1;
+                    break Ok(true);
+                } else {
+                    // The dict was changed since we did lookup. Let's try again.
+                    continue;
+                }
+            } else {
+                break Ok(false);
+            }
         }
     }
 
     pub fn delete_or_insert(
-        &mut self,
+        &self,
         vm: &VirtualMachine,
         key: &PyObjectRef,
         value: T,
     ) -> PyResult<()> {
-        match self.lookup(vm, key)? {
-            LookupResult::Existing(entry_index) => self.unchecked_delete(entry_index),
-            LookupResult::NewIndex {
-                hash_value,
-                hash_index,
-            } => self.unchecked_push(hash_index, hash_value, key.clone(), value),
-        };
-        Ok(())
+        loop {
+            match self.lookup(vm, key)? {
+                LookupResult::Existing(entry_index) => {
+                    let mut inner = self.borrow_value_mut();
+                    if inner.entries[entry_index].is_some() {
+                        inner.entries[entry_index] = None;
+                        inner.size -= 1;
+                        break Ok(());
+                    } else {
+                        // The dict was changed since we did lookup. Let's try again.
+                        continue;
+                    }
+                }
+                LookupResult::NewIndex {
+                    hash_value,
+                    hash_index,
+                } => {
+                    self.unchecked_push(hash_index, hash_value, key.clone(), value);
+                    break Ok(());
+                }
+            };
+        }
     }
 
     pub fn len(&self) -> usize {
-        self.size
+        self.borrow_value().size
     }
 
     pub fn is_empty(&self) -> bool {
@@ -197,33 +266,42 @@ impl<T: Clone> Dict<T> {
     }
 
     pub fn size(&self) -> DictSize {
+        let inner = self.borrow_value();
         DictSize {
-            size: self.size,
-            entries_size: self.entries.len(),
+            size: inner.size,
+            entries_size: inner.entries.len(),
         }
     }
 
-    pub fn next_entry(&self, position: &mut EntryIndex) -> Option<(&PyObjectRef, &T)> {
-        self.entries[*position..].iter().find_map(|entry| {
-            *position += 1;
-            entry
-                .as_ref()
-                .map(|DictEntry { key, value, .. }| (key, value))
-        })
+    pub fn next_entry(&self, position: &mut EntryIndex) -> Option<(PyObjectRef, T)> {
+        self.borrow_value().entries[*position..]
+            .iter()
+            .find_map(|entry| {
+                *position += 1;
+                entry
+                    .as_ref()
+                    .map(|DictEntry { key, value, .. }| (key.clone(), value.clone()))
+            })
     }
 
     pub fn len_from_entry_index(&self, position: EntryIndex) -> usize {
-        self.entries[position..].iter().flatten().count()
+        self.borrow_value().entries[position..]
+            .iter()
+            .flatten()
+            .count()
     }
 
     pub fn has_changed_size(&self, position: &DictSize) -> bool {
-        position.size != self.size || self.entries.len() != position.entries_size
+        let inner = self.borrow_value();
+        position.size != inner.size || inner.entries.len() != position.entries_size
     }
 
-    pub fn keys<'a>(&'a self) -> impl Iterator<Item = PyObjectRef> + 'a {
-        self.entries
+    pub fn keys(&self) -> Vec<PyObjectRef> {
+        self.borrow_value()
+            .entries
             .iter()
             .filter_map(|v| v.as_ref().map(|v| v.key.clone()))
+            .collect()
     }
 
     /// Lookup the index for the given key.
@@ -232,33 +310,41 @@ impl<T: Clone> Dict<T> {
         let hash_value = key.do_hash(vm)?;
         let perturb = hash_value;
         let mut hash_index: HashIndex = hash_value;
-        loop {
-            if self.indices.contains_key(&hash_index) {
-                // Now we have an index, lets check the key.
-                let index = self.indices[&hash_index];
-                if let Some(entry) = &self.entries[index] {
-                    // Okay, we have an entry at this place
-                    if key.do_is(&entry.key) {
-                        // Literally the same object
-                        break Ok(LookupResult::Existing(index));
-                    } else if entry.hash == hash_value {
-                        if key.do_eq(vm, &entry.key)? {
-                            break Ok(LookupResult::Existing(index));
+        'outer: loop {
+            let (entry, index) = loop {
+                let inner = self.borrow_value();
+                if inner.indices.contains_key(&hash_index) {
+                    // Now we have an index, lets check the key.
+                    let index = inner.indices[&hash_index];
+                    if let Some(entry) = &inner.entries[index] {
+                        // Okay, we have an entry at this place
+                        if key.do_is(&entry.key) {
+                            // Literally the same object
+                            break 'outer Ok(LookupResult::Existing(index));
+                        } else if entry.hash == hash_value {
+                            break (entry.clone(), index);
                         } else {
                             // entry mismatch.
                         }
                     } else {
-                        // entry mismatch.
+                        // Removed entry, continue search...
                     }
                 } else {
-                    // Removed entry, continue search...
+                    // Hash not in table, we are at free slot now.
+                    break 'outer Ok(LookupResult::NewIndex {
+                        hash_value,
+                        hash_index,
+                    });
                 }
+                // Update i to next probe location:
+                hash_index = Self::next_index(perturb, hash_index)
+                // warn!("Perturb value: {}", i);
+            };
+            // This comparison needs to be done outside the lock.
+            if key.do_eq(vm, &entry.key)? {
+                break Ok(LookupResult::Existing(index));
             } else {
-                // Hash not in table, we are at free slot now.
-                break Ok(LookupResult::NewIndex {
-                    hash_value,
-                    hash_index,
-                });
+                // entry mismatch.
             }
 
             // Update i to next probe location:
@@ -275,30 +361,45 @@ impl<T: Clone> Dict<T> {
     }
 
     /// Retrieve and delete a key
-    pub fn pop<K: DictKey + Copy>(&mut self, vm: &VirtualMachine, key: K) -> PyResult<Option<T>> {
-        if let LookupResult::Existing(index) = self.lookup(vm, key)? {
-            let value = self.unchecked_get(index);
-            self.unchecked_delete(index);
-            Ok(Some(value))
-        } else {
-            Ok(None)
+    pub fn pop<K: DictKey + Copy>(&self, vm: &VirtualMachine, key: K) -> PyResult<Option<T>> {
+        loop {
+            if let LookupResult::Existing(index) = self.lookup(vm, key)? {
+                let mut inner = self.borrow_value_mut();
+                if let Some(entry) = &inner.entries[index] {
+                    let value = entry.value.clone();
+                    inner.entries[index] = None;
+                    inner.size -= 1;
+                    break Ok(Some(value));
+                } else {
+                    // The dict was changed since we did lookup. Let's try again.
+                    continue;
+                }
+            } else {
+                break Ok(None);
+            }
         }
     }
 
-    pub fn pop_front(&mut self) -> Option<(PyObjectRef, T)> {
-        let mut entry_index = 0;
-        match self.next_entry(&mut entry_index) {
-            Some((key, value)) => {
-                let item = (key.clone(), value.clone());
-                self.unchecked_delete(entry_index - 1);
-                Some(item)
-            }
-            None => None,
+    pub fn pop_front(&self) -> Option<(PyObjectRef, T)> {
+        let mut position = 0;
+        let mut inner = self.borrow_value_mut();
+        let first_item = inner.entries.iter().find_map(|entry| {
+            position += 1;
+            entry
+                .as_ref()
+                .map(|DictEntry { key, value, .. }| (key.clone(), value.clone()))
+        });
+        if let Some(item) = first_item {
+            inner.entries[position - 1] = None;
+            inner.size -= 1;
+            Some(item)
+        } else {
+            None
         }
     }
 
     pub fn sizeof(&self) -> usize {
-        size_of::<Self>() + self.size * size_of::<DictEntry<T>>()
+        size_of::<Self>() + self.borrow_value().size * size_of::<DictEntry<T>>()
     }
 }
 
@@ -389,7 +490,7 @@ mod tests {
     #[test]
     fn test_insert() {
         let vm: VirtualMachine = Default::default();
-        let mut dict = Dict::default();
+        let dict = Dict::default();
         assert_eq!(0, dict.len());
 
         let key1 = vm.new_bool(true);

vm/src/lib.rs

@@ -6,11 +6,7 @@
 //! - Base objects
 
 // for methods like vm.to_str(), not the typical use of 'to' as a method prefix
-#![allow(
-    clippy::wrong_self_convention,
-    clippy::let_and_return,
-    clippy::implicit_hasher
-)]
+#![allow(clippy::wrong_self_convention, clippy::implicit_hasher)]
 #![doc(html_logo_url = "https://raw.githubusercontent.com/RustPython/RustPython/master/logo.png")]
 #![doc(html_root_url = "https://docs.rs/rustpython-vm/")]
 

vm/src/obj/objbytearray.rs

@@ -4,16 +4,15 @@ use std::convert::TryFrom;
 use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
 
 use super::objbyteinner::{
-    ByteInnerExpandtabsOptions, ByteInnerFindOptions, ByteInnerNewOptions, ByteInnerPaddingOptions,
-    ByteInnerSplitOptions, ByteInnerSplitlinesOptions, ByteInnerTranslateOptions, ByteOr,
-    PyByteInner,
+    ByteInnerFindOptions, ByteInnerNewOptions, ByteInnerPaddingOptions, ByteInnerSplitOptions,
+    ByteInnerTranslateOptions, ByteOr, PyByteInner,
 };
 use super::objint::PyIntRef;
 use super::objiter;
 use super::objslice::PySliceRef;
 use super::objstr::{PyString, PyStringRef};
 use super::objtype::PyClassRef;
-use super::pystr::PyCommonString;
+use super::pystr::{self, PyCommonString};
 use crate::cformat::CFormatString;
 use crate::function::{OptionalArg, OptionalOption};
 use crate::obj::objstr::do_cformat_string;
@@ -301,17 +300,9 @@ impl PyByteArray {
     }
 
     #[pymethod(name = "endswith")]
-    fn endswith(
-        &self,
-        suffix: PyObjectRef,
-        start: OptionalArg<Option<isize>>,
-        end: OptionalArg<Option<isize>>,
-        vm: &VirtualMachine,
-    ) -> PyResult<bool> {
+    fn endswith(&self, options: pystr::StartsEndsWithArgs, vm: &VirtualMachine) -> PyResult<bool> {
         self.borrow_value().elements[..].py_startsendswith(
-            suffix,
-            start,
-            end,
+            options,
             "endswith",
             "bytes",
             |s, x: &PyByteInner| s.ends_with(&x.elements[..]),
@@ -322,15 +313,11 @@ impl PyByteArray {
     #[pymethod(name = "startswith")]
     fn startswith(
         &self,
-        prefix: PyObjectRef,
-        start: OptionalArg<Option<isize>>,
-        end: OptionalArg<Option<isize>>,
+        options: pystr::StartsEndsWithArgs,
         vm: &VirtualMachine,
     ) -> PyResult<bool> {
         self.borrow_value().elements[..].py_startsendswith(
-            prefix,
-            start,
-            end,
+            options,
             "startswith",
             "bytes",
             |s, x: &PyByteInner| s.starts_with(&x.elements[..]),
@@ -440,12 +427,12 @@ impl PyByteArray {
     }
 
     #[pymethod(name = "expandtabs")]
-    fn expandtabs(&self, options: ByteInnerExpandtabsOptions) -> PyByteArray {
+    fn expandtabs(&self, options: pystr::ExpandTabsArgs) -> PyByteArray {
         self.borrow_value().expandtabs(options).into()
     }
 
     #[pymethod(name = "splitlines")]
-    fn splitlines(&self, options: ByteInnerSplitlinesOptions, vm: &VirtualMachine) -> PyResult {
+    fn splitlines(&self, options: pystr::SplitLinesArgs, vm: &VirtualMachine) -> PyResult {
         let as_bytes = self
             .borrow_value()
             .splitlines(options)
@@ -465,9 +452,10 @@ impl PyByteArray {
         &self,
         old: PyByteInner,
         new: PyByteInner,
-        count: OptionalArg<PyIntRef>,
+        count: OptionalArg<isize>,
+        vm: &VirtualMachine,
     ) -> PyResult<PyByteArray> {
-        Ok(self.borrow_value().replace(old, new, count)?.into())
+        Ok(self.borrow_value().replace(old, new, count, vm)?.into())
     }
 
     #[pymethod(name = "clear")]

vm/src/obj/objbyteinner.rs

@@ -14,7 +14,7 @@ use super::objnone::PyNoneRef;
 use super::objsequence::PySliceableSequence;
 use super::objslice::PySliceRef;
 use super::objstr::{self, PyString, PyStringRef};
-use super::pystr::{self, PyCommonString, StringRange};
+use super::pystr::{self, PyCommonString, PyCommonStringWrapper, StringRange};
 use crate::function::{OptionalArg, OptionalOption};
 use crate::pyhash;
 use crate::pyobject::{
@@ -255,43 +255,7 @@ impl ByteInnerTranslateOptions {
     }
 }
 
-#[derive(FromArgs)]
-pub struct ByteInnerSplitOptions {
-    #[pyarg(positional_or_keyword, default = "None")]
-    sep: Option<PyByteInner>,
-    #[pyarg(positional_or_keyword, default = "-1")]
-    maxsplit: isize,
-}
-
-impl ByteInnerSplitOptions {
-    pub fn get_value(self, vm: &VirtualMachine) -> PyResult<(Option<Vec<u8>>, isize)> {
-        let sep = if let Some(s) = self.sep {
-            let sep = s.elements;
-            if sep.is_empty() {
-                return Err(vm.new_value_error("empty separator".to_owned()));
-            }
-            Some(sep)
-        } else {
-            None
-        };
-        Ok((sep, self.maxsplit))
-    }
-}
-
-#[derive(FromArgs)]
-pub struct ByteInnerExpandtabsOptions {
-    #[pyarg(positional_or_keyword, optional = true)]
-    tabsize: OptionalArg<PyIntRef>,
-}
-
-impl ByteInnerExpandtabsOptions {
-    pub fn get_value(self) -> usize {
-        match self.tabsize.into_option() {
-            Some(int) => int.as_bigint().to_usize().unwrap_or(0),
-            None => 8,
-        }
-    }
-}
+pub type ByteInnerSplitOptions = pystr::SplitArgs<PyByteInner, [u8], u8>;
 
 #[derive(FromArgs)]
 pub struct ByteInnerSplitlinesOptions {
@@ -460,9 +424,12 @@ impl PyByteInner {
             }),
         };
         let items = sec?;
-        let range = self
+        let mut range = self
             .elements
             .get_slice_range(&slice.start_index(vm)?, &slice.stop_index(vm)?);
+        if range.end < range.start {
+            range.end = range.start;
+        }
         self.elements.splice(range, items);
         Ok(vm
             .ctx
@@ -926,36 +893,33 @@ impl PyByteInner {
     }
 
     pub fn strip(&self, chars: OptionalOption<PyByteInner>) -> Vec<u8> {
-        let chars = chars.flat_option();
-        let chars = match chars {
-            Some(ref chars) => &chars.elements,
-            None => return self.elements.trim().to_owned(),
-        };
         self.elements
-            .trim_with(|c| chars.contains(&(c as u8)))
-            .to_owned()
+            .py_strip(
+                chars,
+                |s, chars| s.trim_with(|c| chars.contains(&(c as u8))),
+                |s| s.trim(),
+            )
+            .to_vec()
     }
 
     pub fn lstrip(&self, chars: OptionalOption<PyByteInner>) -> Vec<u8> {
-        let chars = chars.flat_option();
-        let chars = match chars {
-            Some(ref chars) => &chars.elements,
-            None => return self.elements.trim_start().to_owned(),
-        };
         self.elements
-            .trim_start_with(|c| chars.contains(&(c as u8)))
-            .to_owned()
+            .py_strip(
+                chars,
+                |s, chars| s.trim_start_with(|c| chars.contains(&(c as u8))),
+                |s| s.trim_start(),
+            )
+            .to_vec()
     }
 
     pub fn rstrip(&self, chars: OptionalOption<PyByteInner>) -> Vec<u8> {
-        let chars = chars.flat_option();
-        let chars = match chars {
-            Some(ref chars) => &chars.elements,
-            None => return self.elements.trim_end().to_owned(),
-        };
         self.elements
-            .trim_end_with(|c| chars.contains(&(c as u8)))
-            .to_owned()
+            .py_strip(
+                chars,
+                |s, chars| s.trim_end_with(|c| chars.contains(&(c as u8))),
+                |s| s.trim_end(),
+            )
+            .to_vec()
     }
 
     pub fn split<F>(
@@ -967,19 +931,13 @@ impl PyByteInner {
     where
         F: Fn(&[u8], &VirtualMachine) -> PyObjectRef,
     {
-        let (sep, maxsplit) = options.get_value(vm)?;
-        let sep_ref = match sep {
-            Some(ref v) => Some(&v[..]),
-            None => None,
-        };
         let elements = self.elements.py_split(
-            sep_ref,
-            maxsplit,
+            options,
             vm,
             |v, s, vm| v.split_str(s).map(|v| convert(v, vm)).collect(),
             |v, s, n, vm| v.splitn_str(n, s).map(|v| convert(v, vm)).collect(),
             |v, n, vm| v.py_split_whitespace(n, |v| convert(v, vm)),
-        );
+        )?;
         Ok(vm.ctx.new_list(elements))
     }
 
@@ -992,19 +950,13 @@ impl PyByteInner {
     where
         F: Fn(&[u8], &VirtualMachine) -> PyObjectRef,
     {
-        let (sep, maxsplit) = options.get_value(vm)?;
-        let sep_ref = match sep {
-            Some(ref v) => Some(&v[..]),
-            None => None,
-        };
         let mut elements = self.elements.py_split(
-            sep_ref,
-            maxsplit,
+            options,
             vm,
             |v, s, vm| v.rsplit_str(s).map(|v| convert(v, vm)).collect(),
             |v, s, n, vm| v.rsplitn_str(n, s).map(|v| convert(v, vm)).collect(),
             |v, n, vm| v.py_rsplit_whitespace(n, |v| convert(v, vm)),
-        );
+        )?;
         elements.reverse();
         Ok(vm.ctx.new_list(elements))
     }
@@ -1047,8 +999,8 @@ impl PyByteInner {
         Ok((front, has_mid, back))
     }
 
-    pub fn expandtabs(&self, options: ByteInnerExpandtabsOptions) -> Vec<u8> {
-        let tabsize = options.get_value();
+    pub fn expandtabs(&self, options: pystr::ExpandTabsArgs) -> Vec<u8> {
+        let tabsize = options.tabsize();
         let mut counter: usize = 0;
         let mut res = vec![];
 
@@ -1079,9 +1031,7 @@ impl PyByteInner {
         res
     }
 
-    pub fn splitlines(&self, options: ByteInnerSplitlinesOptions) -> Vec<&[u8]> {
-        let keepends = options.get_value();
-
+    pub fn splitlines(&self, options: pystr::SplitLinesArgs) -> Vec<&[u8]> {
         let mut res = vec![];
 
         if self.elements.is_empty() {
@@ -1090,7 +1040,7 @@ impl PyByteInner {
 
         let mut prev_index = 0;
         let mut index = 0;
-        let keep = if keepends { 1 } else { 0 };
+        let keep = if options.keepends { 1 } else { 0 };
         let slice = &self.elements;
 
         while index < slice.len() {
@@ -1127,41 +1077,166 @@ impl PyByteInner {
         bytes_zfill(&self.elements, width.to_usize().unwrap_or(0))
     }
 
-    pub fn replace(
-        &self,
-        old: PyByteInner,
-        new: PyByteInner,
-        count: OptionalArg<PyIntRef>,
-    ) -> PyResult<Vec<u8>> {
-        let count = match count.into_option() {
-            Some(int) => int
-                .as_bigint()
-                .to_u32()
-                .unwrap_or(self.elements.len() as u32),
-            None => self.elements.len() as u32,
-        };
+    // len(self)>=1, from="", len(to)>=1, maxcount>=1
+    fn replace_interleave(&self, to: PyByteInner, maxcount: Option<usize>) -> Vec<u8> {
+        let place_count = self.elements.len() + 1;
+        let count = maxcount.map_or(place_count, |v| std::cmp::min(v, place_count)) - 1;
+        let capacity = self.elements.len() + count * to.len();
+        let mut result = Vec::with_capacity(capacity);
+        let to_slice = to.elements.as_slice();
+        result.extend_from_slice(to_slice);
+        for c in &self.elements[..count] {
+            result.push(*c);
+            result.extend_from_slice(to_slice);
+        }
+        result.extend_from_slice(&self.elements[count..]);
+        result
+    }
 
-        let mut res = vec![];
-        let mut index = 0;
-        let mut done = 0;
-
-        let slice = &self.elements;
-        loop {
-            if done == count || index > slice.len() - old.len() {
-                res.extend_from_slice(&slice[index..]);
-                break;
-            }
-            if &slice[index..index + old.len()] == old.elements.as_slice() {
-                res.extend_from_slice(&new.elements);
-                index += old.len();
-                done += 1;
-            } else {
-                res.push(slice[index]);
-                index += 1
-            }
+    fn replace_delete(&self, from: PyByteInner, maxcount: Option<usize>) -> Vec<u8> {
+        let count = count_substring(self.elements.as_slice(), from.elements.as_slice(), maxcount);
+        if count == 0 {
+            // no matches
+            return self.elements.clone();
         }
 
-        Ok(res)
+        let result_len = self.len() - (count * from.len());
+        debug_assert!(self.len() >= count * from.len());
+
+        let mut result = Vec::with_capacity(result_len);
+        let mut last_end = 0;
+        let mut count = count;
+        for offset in self.elements.find_iter(&from.elements) {
+            result.extend_from_slice(&self.elements[last_end..offset]);
+            last_end = offset + from.len();
+            count -= 1;
+            if count == 0 {
+                break;
+            }
+        }
+        result.extend_from_slice(&self.elements[last_end..]);
+        result
+    }
+
+    pub fn replace_in_place(
+        &self,
+        from: PyByteInner,
+        to: PyByteInner,
+        maxcount: Option<usize>,
+    ) -> Vec<u8> {
+        let len = from.len();
+        let mut iter = self.elements.find_iter(&from.elements);
+
+        let mut new = if let Some(offset) = iter.next() {
+            let mut new = self.elements.clone();
+            new[offset..offset + len].clone_from_slice(to.elements.as_slice());
+            if maxcount == Some(1) {
+                return new;
+            } else {
+                new
+            }
+        } else {
+            return self.elements.clone();
+        };
+
+        let mut count = maxcount.unwrap_or(std::usize::MAX) - 1;
+        for offset in iter {
+            new[offset..offset + len].clone_from_slice(to.elements.as_slice());
+            count -= 1;
+            if count == 0 {
+                break;
+            }
+        }
+        new
+    }
+
+    fn replace_general(
+        &self,
+        from: PyByteInner,
+        to: PyByteInner,
+        maxcount: Option<usize>,
+        vm: &VirtualMachine,
+    ) -> PyResult<Vec<u8>> {
+        let count = count_substring(self.elements.as_slice(), from.elements.as_slice(), maxcount);
+        if count == 0 {
+            // no matches, return unchanged
+            return Ok(self.elements.clone());
+        }
+
+        // Check for overflow
+        //    result_len = self_len + count * (to_len-from_len)
+        debug_assert!(count > 0);
+        if to.len() as isize - from.len() as isize
+            > (std::isize::MAX - self.elements.len() as isize) / count as isize
+        {
+            return Err(vm.new_overflow_error("replace bytes is too long".to_owned()));
+        }
+        let result_len = self.elements.len() + count * (to.len() - from.len());
+
+        let mut result = Vec::with_capacity(result_len);
+        let mut last_end = 0;
+        let mut count = count;
+        for offset in self.elements.find_iter(&from.elements) {
+            result.extend_from_slice(&self.elements[last_end..offset]);
+            result.extend_from_slice(to.elements.as_slice());
+            last_end = offset + from.len();
+            count -= 1;
+            if count == 0 {
+                break;
+            }
+        }
+        result.extend_from_slice(&self.elements[last_end..]);
+        Ok(result)
+    }
+
+    pub fn replace(
+        &self,
+        from: PyByteInner,
+        to: PyByteInner,
+        maxcount: OptionalArg<isize>,
+        vm: &VirtualMachine,
+    ) -> PyResult<Vec<u8>> {
+        // stringlib_replace in CPython
+        let maxcount = match maxcount {
+            OptionalArg::Present(maxcount) if maxcount >= 0 => {
+                if maxcount == 0 || self.elements.is_empty() {
+                    // nothing to do; return the original bytes
+                    return Ok(self.elements.clone());
+                }
+                Some(maxcount as usize)
+            }
+            _ => None,
+        };
+
+        // Handle zero-length special cases
+        if from.elements.is_empty() {
+            if to.elements.is_empty() {
+                // nothing to do; return the original bytes
+                return Ok(self.elements.clone());
+            }
+            // insert the 'to' bytes everywhere.
+            //     >>> b"Python".replace(b"", b".")
+            //     b'.P.y.t.h.o.n.'
+            return Ok(self.replace_interleave(to, maxcount));
+        }
+
+        // Except for b"".replace(b"", b"A") == b"A" there is no way beyond this
+        // point for an empty self bytes to generate a non-empty bytes
+        // Special case so the remaining code always gets a non-empty bytes
+        if self.elements.is_empty() {
+            return Ok(self.elements.clone());
+        }
+
+        if to.elements.is_empty() {
+            // delete all occurrences of 'from' bytes
+            Ok(self.replace_delete(from, maxcount))
+        } else if from.len() == to.len() {
+            // Handle special case where both bytes have the same length
+            Ok(self.replace_in_place(from, to, maxcount))
+        } else {
+            // Otherwise use the more generic algorithms
+            self.replace_general(from, to, maxcount, vm)
+        }
     }
 
     pub fn title(&self) -> Vec<u8> {
@@ -1233,6 +1308,16 @@ pub fn try_as_byte(obj: &PyObjectRef) -> Option<Vec<u8>> {
     })
 }
 
+#[inline]
+fn count_substring(haystack: &[u8], needle: &[u8], maxcount: Option<usize>) -> usize {
+    let substrings = haystack.find_iter(needle);
+    if let Some(maxcount) = maxcount {
+        std::cmp::min(substrings.take(maxcount).count(), maxcount)
+    } else {
+        substrings.count()
+    }
+}
+
 pub trait ByteOr: ToPrimitive {
     fn byte_or(&self, vm: &VirtualMachine) -> PyResult<u8> {
         match self.to_u8() {
@@ -1297,13 +1382,23 @@ pub fn bytes_zfill(bytes: &[u8], width: usize) -> Vec<u8> {
     }
 }
 
+impl PyCommonStringWrapper<[u8]> for PyByteInner {
+    fn as_ref(&self) -> &[u8] {
+        &self.elements
+    }
+}
+
 const ASCII_WHITESPACES: [u8; 6] = [0x20, 0x09, 0x0a, 0x0c, 0x0d, 0x0b];
 
-impl PyCommonString<'_, u8> for [u8] {
+impl PyCommonString<u8> for [u8] {
     fn get_slice(&self, range: std::ops::Range<usize>) -> &Self {
         &self[range]
     }
 
+    fn is_empty(&self) -> bool {
+        Self::is_empty(self)
+    }
+
     fn len(&self) -> usize {
         Self::len(self)
     }

vm/src/obj/objbytes.rs

@@ -4,15 +4,15 @@ use std::ops::Deref;
 use std::str::FromStr;
 
 use super::objbyteinner::{
-    ByteInnerExpandtabsOptions, ByteInnerFindOptions, ByteInnerNewOptions, ByteInnerPaddingOptions,
-    ByteInnerSplitOptions, ByteInnerSplitlinesOptions, ByteInnerTranslateOptions, PyByteInner,
+    ByteInnerFindOptions, ByteInnerNewOptions, ByteInnerPaddingOptions, ByteInnerSplitOptions,
+    ByteInnerTranslateOptions, PyByteInner,
 };
 use super::objint::PyIntRef;
 use super::objiter;
 use super::objslice::PySliceRef;
 use super::objstr::{PyString, PyStringRef};
 use super::objtype::PyClassRef;
-use super::pystr::PyCommonString;
+use super::pystr::{self, PyCommonString};
 use crate::cformat::CFormatString;
 use crate::function::{OptionalArg, OptionalOption};
 use crate::obj::objstr::do_cformat_string;
@@ -273,17 +273,9 @@ impl PyBytes {
     }
 
     #[pymethod(name = "endswith")]
-    fn endswith(
-        &self,
-        suffix: PyObjectRef,
-        start: OptionalArg<Option<isize>>,
-        end: OptionalArg<Option<isize>>,
-        vm: &VirtualMachine,
-    ) -> PyResult<bool> {
+    fn endswith(&self, options: pystr::StartsEndsWithArgs, vm: &VirtualMachine) -> PyResult<bool> {
         self.inner.elements[..].py_startsendswith(
-            suffix,
-            start,
-            end,
+            options,
             "endswith",
             "bytes",
             |s, x: &PyByteInner| s.ends_with(&x.elements[..]),
@@ -294,15 +286,11 @@ impl PyBytes {
     #[pymethod(name = "startswith")]
     fn startswith(
         &self,
-        prefix: PyObjectRef,
-        start: OptionalArg<Option<isize>>,
-        end: OptionalArg<Option<isize>>,
+        options: pystr::StartsEndsWithArgs,
         vm: &VirtualMachine,
     ) -> PyResult<bool> {
         self.inner.elements[..].py_startsendswith(
-            prefix,
-            start,
-            end,
+            options,
             "startswith",
             "bytes",
             |s, x: &PyByteInner| s.starts_with(&x.elements[..]),
@@ -401,12 +389,12 @@ impl PyBytes {
     }
 
     #[pymethod(name = "expandtabs")]
-    fn expandtabs(&self, options: ByteInnerExpandtabsOptions) -> PyBytes {
+    fn expandtabs(&self, options: pystr::ExpandTabsArgs) -> PyBytes {
         self.inner.expandtabs(options).into()
     }
 
     #[pymethod(name = "splitlines")]
-    fn splitlines(&self, options: ByteInnerSplitlinesOptions, vm: &VirtualMachine) -> PyResult {
+    fn splitlines(&self, options: pystr::SplitLinesArgs, vm: &VirtualMachine) -> PyResult {
         let as_bytes = self
             .inner
             .splitlines(options)
@@ -426,9 +414,10 @@ impl PyBytes {
         &self,
         old: PyByteInner,
         new: PyByteInner,
-        count: OptionalArg<PyIntRef>,
+        count: OptionalArg<isize>,
+        vm: &VirtualMachine,
     ) -> PyResult<PyBytes> {
-        Ok(self.inner.replace(old, new, count)?.into())
+        Ok(self.inner.replace(old, new, count, vm)?.into())
     }
 
     #[pymethod(name = "title")]

vm/src/obj/objclassmethod.rs

@@ -1,7 +1,7 @@
 use super::objtype::PyClassRef;
 use crate::function::OptionalArg;
 use crate::pyobject::{
-    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe, TypeProtocol,
 };
 use crate::slots::SlotDescriptor;
 use crate::vm::VirtualMachine;
@@ -32,6 +32,7 @@ pub struct PyClassMethod {
     callable: PyObjectRef,
 }
 pub type PyClassMethodRef = PyRef<PyClassMethod>;
+impl ThreadSafe for PyClassMethod {}
 
 impl PyClassMethod {
     pub fn new(value: PyObjectRef) -> Self {

vm/src/obj/objcode.rs

@@ -7,7 +7,9 @@ use std::ops::Deref;
 
 use super::objtype::PyClassRef;
 use crate::bytecode;
-use crate::pyobject::{IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{
+    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
+};
 use crate::vm::VirtualMachine;
 
 pub type PyCodeRef = PyRef<PyCode>;
@@ -17,6 +19,8 @@ pub struct PyCode {
     pub code: bytecode::CodeObject,
 }
 
+impl ThreadSafe for PyCode {}
+
 impl Deref for PyCode {
     type Target = bytecode::CodeObject;
     fn deref(&self) -> &Self::Target {
@@ -45,8 +49,7 @@ impl PyValue for PyCode {
 #[pyimpl]
 impl PyCodeRef {
     #[pyslot]
-    #[allow(clippy::new_ret_no_self)]
-    fn new(_cls: PyClassRef, vm: &VirtualMachine) -> PyResult {
+    fn new(_cls: PyClassRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
         Err(vm.new_type_error("Cannot directly create code object".to_owned()))
     }
 

vm/src/obj/objdict.rs

@@ -1,4 +1,4 @@
-use std::cell::{Cell, RefCell};
+use std::cell::Cell;
 use std::fmt;
 
 use super::objiter;
@@ -9,7 +9,7 @@ use crate::exceptions::PyBaseExceptionRef;
 use crate::function::{KwArgs, OptionalArg, PyFuncArgs};
 use crate::pyobject::{
     IdProtocol, IntoPyObject, ItemProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable,
-    PyObjectRef, PyRef, PyResult, PyValue,
+    PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
 };
 use crate::vm::{ReprGuard, VirtualMachine};
 
@@ -20,9 +20,10 @@ pub type DictContentType = dictdatatype::Dict;
 #[pyclass]
 #[derive(Default)]
 pub struct PyDict {
-    entries: RefCell<DictContentType>,
+    entries: DictContentType,
 }
 pub type PyDictRef = PyRef<PyDict>;
+impl ThreadSafe for PyDict {}
 
 impl fmt::Debug for PyDict {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -43,7 +44,7 @@ impl PyDictRef {
     #[pyslot]
     fn tp_new(class: PyClassRef, _args: PyFuncArgs, vm: &VirtualMachine) -> PyResult<PyDictRef> {
         PyDict {
-            entries: RefCell::new(DictContentType::default()),
+            entries: DictContentType::default(),
         }
         .into_ref_with_type(vm, class)
     }
@@ -59,7 +60,7 @@ impl PyDictRef {
     }
 
     fn merge(
-        dict: &RefCell<DictContentType>,
+        dict: &DictContentType,
         dict_obj: OptionalArg<PyObjectRef>,
         kwargs: KwArgs,
         vm: &VirtualMachine,
@@ -68,13 +69,13 @@ impl PyDictRef {
             let dicted: Result<PyDictRef, _> = dict_obj.clone().downcast();
             if let Ok(dict_obj) = dicted {
                 for (key, value) in dict_obj {
-                    dict.borrow_mut().insert(vm, &key, value)?;
+                    dict.insert(vm, &key, value)?;
                 }
             } else if let Some(keys) = vm.get_method(dict_obj.clone(), "keys") {
                 let keys = objiter::get_iter(vm, &vm.invoke(&keys?, vec![])?)?;
                 while let Some(key) = objiter::get_next_object(vm, &keys)? {
                     let val = dict_obj.get_item(&key, vm)?;
-                    dict.borrow_mut().insert(vm, &key, val)?;
+                    dict.insert(vm, &key, val)?;
                 }
             } else {
                 let iter = objiter::get_iter(vm, &dict_obj)?;
@@ -92,14 +93,13 @@ impl PyDictRef {
                     if objiter::get_next_object(vm, &elem_iter)?.is_some() {
                         return Err(err(vm));
                     }
-                    dict.borrow_mut().insert(vm, &key, value)?;
+                    dict.insert(vm, &key, value)?;
                 }
             }
         }
 
-        let mut dict_borrowed = dict.borrow_mut();
         for (key, value) in kwargs.into_iter() {
-            dict_borrowed.insert(vm, &vm.new_str(key), value)?;
+            dict.insert(vm, &vm.new_str(key), value)?;
         }
         Ok(())
     }
@@ -111,27 +111,26 @@ impl PyDictRef {
         value: OptionalArg<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult<PyDictRef> {
-        let mut dict = DictContentType::default();
+        let dict = DictContentType::default();
         let value = value.unwrap_or_else(|| vm.ctx.none());
         for elem in iterable.iter(vm)? {
             let elem = elem?;
             dict.insert(vm, &elem, value.clone())?;
         }
-        let entries = RefCell::new(dict);
-        PyDict { entries }.into_ref_with_type(vm, class)
+        PyDict { entries: dict }.into_ref_with_type(vm, class)
     }
 
     #[pymethod(magic)]
     fn bool(self) -> bool {
-        !self.entries.borrow().is_empty()
+        !self.entries.is_empty()
     }
 
     fn inner_eq(self, other: &PyDict, vm: &VirtualMachine) -> PyResult<bool> {
-        if other.entries.borrow().len() != self.entries.borrow().len() {
+        if other.entries.len() != self.entries.len() {
             return Ok(false);
         }
         for (k, v1) in self {
-            match other.entries.borrow().get(vm, &k)? {
+            match other.entries.get(vm, &k)? {
                 Some(v2) => {
                     if v1.is(&v2) {
                         continue;
@@ -170,12 +169,12 @@ impl PyDictRef {
 
     #[pymethod(magic)]
     fn len(self) -> usize {
-        self.entries.borrow().len()
+        self.entries.len()
     }
 
     #[pymethod(magic)]
     fn sizeof(self) -> usize {
-        size_of::<Self>() + self.entries.borrow().sizeof()
+        size_of::<Self>() + self.entries.sizeof()
     }
 
     #[pymethod(magic)]
@@ -197,17 +196,17 @@ impl PyDictRef {
 
     #[pymethod(magic)]
     fn contains(self, key: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
-        self.entries.borrow().contains(vm, &key)
+        self.entries.contains(vm, &key)
     }
 
     #[pymethod(magic)]
     fn delitem(self, key: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.entries.borrow_mut().delete(vm, &key)
+        self.entries.delete(vm, &key)
     }
 
     #[pymethod]
     fn clear(self) {
-        self.entries.borrow_mut().clear()
+        self.entries.clear()
     }
 
     #[pymethod(magic)]
@@ -243,7 +242,7 @@ impl PyDictRef {
         value: PyObjectRef,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        self.entries.borrow_mut().insert(vm, key, value)
+        self.entries.insert(vm, key, value)
     }
 
     #[pymethod(magic)]
@@ -262,7 +261,7 @@ impl PyDictRef {
         key: K,
         vm: &VirtualMachine,
     ) -> PyResult<Option<PyObjectRef>> {
-        if let Some(value) = self.entries.borrow().get(vm, key)? {
+        if let Some(value) = self.entries.get(vm, key)? {
             return Ok(Some(value));
         }
 
@@ -281,7 +280,7 @@ impl PyDictRef {
         default: OptionalArg<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
-        match self.entries.borrow().get(vm, &key)? {
+        match self.entries.get(vm, &key)? {
             Some(value) => Ok(value),
             None => Ok(default.unwrap_or_else(|| vm.ctx.none())),
         }
@@ -294,12 +293,11 @@ impl PyDictRef {
         default: OptionalArg<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
-        let mut entries = self.entries.borrow_mut();
-        match entries.get(vm, &key)? {
+        match self.entries.get(vm, &key)? {
             Some(value) => Ok(value),
             None => {
                 let set_value = default.unwrap_or_else(|| vm.ctx.none());
-                entries.insert(vm, &key, set_value.clone())?;
+                self.entries.insert(vm, &key, set_value.clone())?;
                 Ok(set_value)
             }
         }
@@ -329,7 +327,7 @@ impl PyDictRef {
         default: OptionalArg<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
-        match self.entries.borrow_mut().pop(vm, &key)? {
+        match self.entries.pop(vm, &key)? {
             Some(value) => Ok(value),
             None => match default {
                 OptionalArg::Present(default) => Ok(default),
@@ -340,8 +338,7 @@ impl PyDictRef {
 
     #[pymethod]
     fn popitem(self, vm: &VirtualMachine) -> PyResult {
-        let mut entries = self.entries.borrow_mut();
-        if let Some((key, value)) = entries.pop_front() {
+        if let Some((key, value)) = self.entries.pop_front() {
             Ok(vm.ctx.new_tuple(vec![key, value]))
         } else {
             let err_msg = vm.new_str("popitem(): dictionary is empty".to_owned());
@@ -360,14 +357,13 @@ impl PyDictRef {
     }
 
     pub fn from_attributes(attrs: PyAttributes, vm: &VirtualMachine) -> PyResult<Self> {
-        let mut dict = DictContentType::default();
+        let dict = DictContentType::default();
 
         for (key, value) in attrs {
             dict.insert(vm, &vm.ctx.new_str(key), value)?;
         }
 
-        let entries = RefCell::new(dict);
-        Ok(PyDict { entries }.into_ref(vm))
+        Ok(PyDict { entries: dict }.into_ref(vm))
     }
 
     #[pymethod(magic)]
@@ -377,11 +373,11 @@ impl PyDictRef {
 
     pub fn contains_key<T: IntoPyObject>(&self, key: T, vm: &VirtualMachine) -> bool {
         let key = key.into_pyobject(vm).unwrap();
-        self.entries.borrow().contains(vm, &key).unwrap()
+        self.entries.contains(vm, &key).unwrap()
     }
 
     pub fn size(&self) -> dictdatatype::DictSize {
-        self.entries.borrow().size()
+        self.entries.size()
     }
 
     /// This function can be used to get an item without raising the
@@ -487,8 +483,8 @@ impl Iterator for DictIter {
     type Item = (PyObjectRef, PyObjectRef);
 
     fn next(&mut self) -> Option<Self::Item> {
-        match self.dict.entries.borrow().next_entry(&mut self.position) {
-            Some((key, value)) => Some((key.clone(), value.clone())),
+        match self.dict.entries.next_entry(&mut self.position) {
+            Some((key, value)) => Some((key, value)),
             None => None,
         }
     }
@@ -524,7 +520,7 @@ macro_rules! dict_iterator {
                 let s = if let Some(_guard) = ReprGuard::enter(zelf.as_object()) {
                     let mut str_parts = vec![];
                     for (key, value) in zelf.dict.clone() {
-                        let s = vm.to_repr(&$result_fn(vm, &key, &value))?;
+                        let s = vm.to_repr(&$result_fn(vm, key, value))?;
                         str_parts.push(s.as_str().to_owned());
                     }
                     format!("{}([{}])", $class_name, str_parts.join(", "))
@@ -563,13 +559,12 @@ macro_rules! dict_iterator {
             #[allow(clippy::redundant_closure_call)]
             fn next(&self, vm: &VirtualMachine) -> PyResult {
                 let mut position = self.position.get();
-                let dict = self.dict.entries.borrow();
-                if dict.has_changed_size(&self.size) {
+                if self.dict.entries.has_changed_size(&self.size) {
                     return Err(
                         vm.new_runtime_error("dictionary changed size during iteration".to_owned())
                     );
                 }
-                match dict.next_entry(&mut position) {
+                match self.dict.entries.next_entry(&mut position) {
                     Some((key, value)) => {
                         self.position.set(position);
                         Ok($result_fn(vm, key, value))
@@ -585,10 +580,7 @@ macro_rules! dict_iterator {
 
             #[pymethod(name = "__length_hint__")]
             fn length_hint(&self) -> usize {
-                self.dict
-                    .entries
-                    .borrow()
-                    .len_from_entry_index(self.position.get())
+                self.dict.entries.len_from_entry_index(self.position.get())
             }
         }
 
@@ -607,7 +599,7 @@ dict_iterator! {
     dictkeyiterator_type,
     "dict_keys",
     "dictkeyiterator",
-    |_vm: &VirtualMachine, key: &PyObjectRef, _value: &PyObjectRef| key.clone()
+    |_vm: &VirtualMachine, key: PyObjectRef, _value: PyObjectRef| key
 }
 
 dict_iterator! {
@@ -617,7 +609,7 @@ dict_iterator! {
     dictvalueiterator_type,
     "dict_values",
     "dictvalueiterator",
-    |_vm: &VirtualMachine, _key: &PyObjectRef, value: &PyObjectRef| value.clone()
+    |_vm: &VirtualMachine, _key: PyObjectRef, value: PyObjectRef| value
 }
 
 dict_iterator! {
@@ -627,8 +619,8 @@ dict_iterator! {
     dictitemiterator_type,
     "dict_items",
     "dictitemiterator",
-    |vm: &VirtualMachine, key: &PyObjectRef, value: &PyObjectRef|
-        vm.ctx.new_tuple(vec![key.clone(), value.clone()])
+    |vm: &VirtualMachine, key: PyObjectRef, value: PyObjectRef|
+        vm.ctx.new_tuple(vec![key, value])
 }
 
 pub(crate) fn init(context: &PyContext) {

vm/src/obj/objenumerate.rs

@@ -1,5 +1,5 @@
-use std::cell::RefCell;
 use std::ops::AddAssign;
+use std::sync::RwLock;
 
 use num_bigint::BigInt;
 use num_traits::Zero;
@@ -8,16 +8,17 @@ use super::objint::PyIntRef;
 use super::objiter;
 use super::objtype::PyClassRef;
 use crate::function::OptionalArg;
-use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe};
 use crate::vm::VirtualMachine;
 
 #[pyclass]
 #[derive(Debug)]
 pub struct PyEnumerate {
-    counter: RefCell<BigInt>,
+    counter: RwLock<BigInt>,
     iterator: PyObjectRef,
 }
 type PyEnumerateRef = PyRef<PyEnumerate>;
+impl ThreadSafe for PyEnumerate {}
 
 impl PyValue for PyEnumerate {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -41,24 +42,19 @@ impl PyEnumerate {
 
         let iterator = objiter::get_iter(vm, &iterable)?;
         PyEnumerate {
-            counter: RefCell::new(counter),
+            counter: RwLock::new(counter),
             iterator,
         }
         .into_ref_with_type(vm, cls)
     }
 
     #[pymethod(name = "__next__")]
-    fn next(&self, vm: &VirtualMachine) -> PyResult {
-        let iterator = &self.iterator;
-        let counter = &self.counter;
-        let next_obj = objiter::call_next(vm, iterator)?;
-        let result = vm
-            .ctx
-            .new_tuple(vec![vm.ctx.new_bigint(&counter.borrow()), next_obj]);
-
-        AddAssign::add_assign(&mut counter.borrow_mut() as &mut BigInt, 1);
-
-        Ok(result)
+    fn next(&self, vm: &VirtualMachine) -> PyResult<(BigInt, PyObjectRef)> {
+        let next_obj = objiter::call_next(vm, &self.iterator)?;
+        let mut counter = self.counter.write().unwrap();
+        let position = counter.clone();
+        AddAssign::add_assign(&mut counter as &mut BigInt, 1);
+        Ok((position, next_obj))
     }
 
     #[pymethod(name = "__iter__")]

vm/src/obj/objfilter.rs

@@ -1,7 +1,9 @@
 use super::objbool;
 use super::objiter;
 use super::objtype::PyClassRef;
-use crate::pyobject::{IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{
+    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
+};
 use crate::vm::VirtualMachine;
 
 pub type PyFilterRef = PyRef<PyFilter>;
@@ -16,6 +18,7 @@ pub struct PyFilter {
     predicate: PyObjectRef,
     iterator: PyObjectRef,
 }
+impl ThreadSafe for PyFilter {}
 
 impl PyValue for PyFilter {
     fn class(vm: &VirtualMachine) -> PyClassRef {

vm/src/obj/objfloat.rs

@@ -1,4 +1,3 @@
-use hexf_parse;
 use num_bigint::{BigInt, ToBigInt};
 use num_rational::Ratio;
 use num_traits::{float::Float, pow, sign::Signed, ToPrimitive, Zero};

vm/src/obj/objint.rs

@@ -120,7 +120,6 @@ impl_try_from_object_int!(
     (u64, to_u64),
 );
 
-#[allow(clippy::collapsible_if)]
 fn inner_pow(int1: &BigInt, int2: &BigInt, vm: &VirtualMachine) -> PyResult {
     if int2.is_negative() {
         let v1 = try_float(int1, vm)?;

vm/src/obj/objiter.rs

@@ -10,8 +10,8 @@ use super::objsequence;
 use super::objtype::{self, PyClassRef};
 use crate::exceptions::PyBaseExceptionRef;
 use crate::pyobject::{
-    PyCallable, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
-    TypeProtocol,
+    PyCallable, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
+    TryFromObject, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -141,6 +141,7 @@ pub struct PySequenceIterator {
     pub obj: PyObjectRef,
     pub reversed: bool,
 }
+impl ThreadSafe for PySequenceIterator {}
 
 impl PyValue for PySequenceIterator {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -214,6 +215,7 @@ pub struct PyCallableIterator {
     sentinel: PyObjectRef,
     done: AtomicCell<bool>,
 }
+impl ThreadSafe for PyCallableIterator {}
 
 impl PyValue for PyCallableIterator {
     fn class(vm: &VirtualMachine) -> PyClassRef {

vm/src/obj/objmap.rs

@@ -1,7 +1,7 @@
 use super::objiter;
 use super::objtype::PyClassRef;
 use crate::function::Args;
-use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe};
 use crate::vm::VirtualMachine;
 
 /// map(func, *iterables) --> map object
@@ -15,6 +15,7 @@ pub struct PyMap {
     iterators: Vec<PyObjectRef>,
 }
 type PyMapRef = PyRef<PyMap>;
+impl ThreadSafe for PyMap {}
 
 impl PyValue for PyMap {
     fn class(vm: &VirtualMachine) -> PyClassRef {

vm/src/obj/objmemory.rs

@@ -1,7 +1,7 @@
 use super::objbyteinner::try_as_byte;
 use super::objtype::{issubclass, PyClassRef};
 use crate::pyobject::{
-    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe, TypeProtocol,
 };
 use crate::stdlib::array::PyArray;
 use crate::vm::VirtualMachine;
@@ -11,6 +11,7 @@ use crate::vm::VirtualMachine;
 pub struct PyMemoryView {
     obj_ref: PyObjectRef,
 }
+impl ThreadSafe for PyMemoryView {}
 
 pub type PyMemoryViewRef = PyRef<PyMemoryView>;
 

vm/src/obj/objproperty.rs

@@ -1,12 +1,13 @@
 /*! Python `property` descriptor class.
 
 */
-use std::cell::RefCell;
+use std::sync::RwLock;
 
 use super::objtype::PyClassRef;
 use crate::function::OptionalArg;
 use crate::pyobject::{
-    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TypeProtocol,
+    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
+    TypeProtocol,
 };
 use crate::slots::SlotDescriptor;
 use crate::vm::VirtualMachine;
@@ -49,8 +50,9 @@ pub struct PyProperty {
     getter: Option<PyObjectRef>,
     setter: Option<PyObjectRef>,
     deleter: Option<PyObjectRef>,
-    doc: RefCell<Option<PyObjectRef>>,
+    doc: RwLock<Option<PyObjectRef>>,
 }
+impl ThreadSafe for PyProperty {}
 
 impl PyValue for PyProperty {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -73,7 +75,6 @@ struct PropertyArgs {
 }
 
 impl SlotDescriptor for PyProperty {
-    #[allow(clippy::collapsible_if)]
     fn descr_get(
         vm: &VirtualMachine,
         zelf: PyObjectRef,
@@ -83,12 +84,10 @@ impl SlotDescriptor for PyProperty {
         let (zelf, obj) = Self::_unwrap(zelf, obj, vm)?;
         if vm.is_none(&obj) {
             Ok(zelf.into_object())
+        } else if let Some(getter) = zelf.getter.as_ref() {
+            vm.invoke(&getter, obj)
         } else {
-            if let Some(getter) = zelf.getter.as_ref() {
-                vm.invoke(&getter, obj)
-            } else {
-                Err(vm.new_attribute_error("unreadable attribute".to_string()))
-            }
+            Err(vm.new_attribute_error("unreadable attribute".to_string()))
         }
     }
 }
@@ -101,7 +100,7 @@ impl PyProperty {
             getter: args.fget,
             setter: args.fset,
             deleter: args.fdel,
-            doc: RefCell::new(args.doc),
+            doc: RwLock::new(args.doc),
         }
         .into_ref_with_type(vm, cls)
     }
@@ -144,11 +143,11 @@ impl PyProperty {
     }
 
     fn doc_getter(&self) -> Option<PyObjectRef> {
-        self.doc.borrow().clone()
+        self.doc.read().unwrap().clone()
     }
 
     fn doc_setter(&self, value: PyObjectRef, vm: &VirtualMachine) {
-        self.doc.replace(py_none_to_option(vm, &value));
+        *self.doc.write().unwrap() = py_none_to_option(vm, &value);
     }
 
     // Python builder functions
@@ -163,7 +162,7 @@ impl PyProperty {
             getter: getter.or_else(|| zelf.getter.clone()),
             setter: zelf.setter.clone(),
             deleter: zelf.deleter.clone(),
-            doc: RefCell::new(None),
+            doc: RwLock::new(None),
         }
         .into_ref_with_type(vm, TypeProtocol::class(&zelf))
     }
@@ -178,7 +177,7 @@ impl PyProperty {
             getter: zelf.getter.clone(),
             setter: setter.or_else(|| zelf.setter.clone()),
             deleter: zelf.deleter.clone(),
-            doc: RefCell::new(None),
+            doc: RwLock::new(None),
         }
         .into_ref_with_type(vm, TypeProtocol::class(&zelf))
     }
@@ -193,7 +192,7 @@ impl PyProperty {
             getter: zelf.getter.clone(),
             setter: zelf.setter.clone(),
             deleter: deleter.or_else(|| zelf.deleter.clone()),
-            doc: RefCell::new(None),
+            doc: RwLock::new(None),
         }
         .into_ref_with_type(vm, TypeProtocol::class(&zelf))
     }

vm/src/obj/objrange.rs

@@ -1,5 +1,4 @@
-use std::cell::Cell;
-
+use crossbeam_utils::atomic::AtomicCell;
 use num_bigint::{BigInt, Sign};
 use num_integer::Integer;
 use num_traits::{One, Signed, Zero};
@@ -13,7 +12,8 @@ use super::objtype::PyClassRef;
 use crate::function::{OptionalArg, PyFuncArgs};
 use crate::pyhash;
 use crate::pyobject::{
-    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
+    PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe, TryFromObject,
+    TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -32,6 +32,7 @@ pub struct PyRange {
     pub stop: PyIntRef,
     pub step: PyIntRef,
 }
+impl ThreadSafe for PyRange {}
 
 impl PyValue for PyRange {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -171,7 +172,7 @@ impl PyRange {
     #[pymethod(name = "__iter__")]
     fn iter(zelf: PyRef<Self>) -> PyRangeIterator {
         PyRangeIterator {
-            position: Cell::new(0),
+            position: AtomicCell::new(0),
             range: zelf,
         }
     }
@@ -204,7 +205,7 @@ impl PyRange {
         };
 
         PyRangeIterator {
-            position: Cell::new(0),
+            position: AtomicCell::new(0),
             range: reversed.into_ref(vm),
         }
     }
@@ -401,9 +402,10 @@ impl PyRange {
 #[pyclass]
 #[derive(Debug)]
 pub struct PyRangeIterator {
-    position: Cell<usize>,
+    position: AtomicCell<usize>,
     range: PyRangeRef,
 }
+impl ThreadSafe for PyRangeIterator {}
 
 impl PyValue for PyRangeIterator {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -417,9 +419,8 @@ type PyRangeIteratorRef = PyRef<PyRangeIterator>;
 impl PyRangeIterator {
     #[pymethod(name = "__next__")]
     fn next(&self, vm: &VirtualMachine) -> PyResult<BigInt> {
-        let position = BigInt::from(self.position.get());
+        let position = BigInt::from(self.position.fetch_add(1));
         if let Some(int) = self.range.get(&position) {
-            self.position.set(self.position.get() + 1);
             Ok(int)
         } else {
             Err(objiter::new_stop_iteration(vm))

vm/src/obj/objset.rs

@@ -1,8 +1,6 @@
 /*
  * Builtin set type with a sequence of unique items.
  */
-
-use std::cell::RefCell;
 use std::fmt;
 
 use super::objlist::PyListIterator;
@@ -11,8 +9,8 @@ use crate::dictdatatype;
 use crate::function::{Args, OptionalArg};
 use crate::pyhash;
 use crate::pyobject::{
-    PyClassImpl, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
-    TypeProtocol,
+    PyClassImpl, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
+    TryFromObject, TypeProtocol,
 };
 use crate::vm::{ReprGuard, VirtualMachine};
 
@@ -25,9 +23,10 @@ pub type SetContentType = dictdatatype::Dict<()>;
 #[pyclass]
 #[derive(Default)]
 pub struct PySet {
-    inner: RefCell<PySetInner>,
+    inner: PySetInner,
 }
 pub type PySetRef = PyRef<PySet>;
+impl ThreadSafe for PySet {}
 
 /// frozenset() -> empty frozenset object
 /// frozenset(iterable) -> frozenset object
@@ -39,6 +38,7 @@ pub struct PyFrozenSet {
     inner: PySetInner,
 }
 pub type PyFrozenSetRef = PyRef<PyFrozenSet>;
+impl ThreadSafe for PyFrozenSet {}
 
 impl fmt::Debug for PySet {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -73,7 +73,7 @@ struct PySetInner {
 
 impl PySetInner {
     fn new(iterable: PyIterable, vm: &VirtualMachine) -> PyResult<PySetInner> {
-        let mut set = PySetInner::default();
+        let set = PySetInner::default();
         for item in iterable.iter(vm)? {
             set.add(&item?, vm)?;
         }
@@ -177,7 +177,7 @@ impl PySetInner {
     }
 
     fn union(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<PySetInner> {
-        let mut set = self.clone();
+        let set = self.clone();
         for item in other.iter(vm)? {
             set.add(&item?, vm)?;
         }
@@ -186,7 +186,7 @@ impl PySetInner {
     }
 
     fn intersection(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<PySetInner> {
-        let mut set = PySetInner::default();
+        let set = PySetInner::default();
         for item in other.iter(vm)? {
             let obj = item?;
             if self.contains(&obj, vm)? {
@@ -197,7 +197,7 @@ impl PySetInner {
     }
 
     fn difference(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<PySetInner> {
-        let mut set = self.copy();
+        let set = self.copy();
         for item in other.iter(vm)? {
             set.content.delete_if_exists(vm, &item?)?;
         }
@@ -205,7 +205,7 @@ impl PySetInner {
     }
 
     fn symmetric_difference(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<PySetInner> {
-        let mut new_inner = self.clone();
+        let new_inner = self.clone();
 
         // We want to remove duplicates in other
         let other_set = Self::new(other, vm)?;
@@ -241,8 +241,7 @@ impl PySetInner {
     }
 
     fn iter(&self, vm: &VirtualMachine) -> PyListIterator {
-        let items = self.content.keys().collect();
-        let set_list = vm.ctx.new_list(items);
+        let set_list = vm.ctx.new_list(self.content.keys());
         PyListIterator {
             position: crossbeam_utils::atomic::AtomicCell::new(0),
             list: set_list.downcast().unwrap(),
@@ -259,23 +258,23 @@ impl PySetInner {
         Ok(format!("{{{}}}", str_parts.join(", ")))
     }
 
-    fn add(&mut self, item: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
+    fn add(&self, item: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         self.content.insert(vm, item, ())
     }
 
-    fn remove(&mut self, item: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
+    fn remove(&self, item: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
         self.content.delete(vm, item)
     }
 
-    fn discard(&mut self, item: &PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
+    fn discard(&self, item: &PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
         self.content.delete_if_exists(vm, item)
     }
 
-    fn clear(&mut self) {
+    fn clear(&self) {
         self.content.clear()
     }
 
-    fn pop(&mut self, vm: &VirtualMachine) -> PyResult {
+    fn pop(&self, vm: &VirtualMachine) -> PyResult {
         if let Some((key, _)) = self.content.pop_front() {
             Ok(key)
         } else {
@@ -284,7 +283,7 @@ impl PySetInner {
         }
     }
 
-    fn update(&mut self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult<()> {
+    fn update(&self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult<()> {
         for iterable in others {
             for item in iterable.iter(vm)? {
                 self.add(&item?, vm)?;
@@ -293,11 +292,7 @@ impl PySetInner {
         Ok(())
     }
 
-    fn intersection_update(
-        &mut self,
-        others: Args<PyIterable>,
-        vm: &VirtualMachine,
-    ) -> PyResult<()> {
+    fn intersection_update(&self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult<()> {
         let mut temp_inner = self.copy();
         self.clear();
         for iterable in others {
@@ -312,7 +307,7 @@ impl PySetInner {
         Ok(())
     }
 
-    fn difference_update(&mut self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult<()> {
+    fn difference_update(&self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult<()> {
         for iterable in others {
             for item in iterable.iter(vm)? {
                 self.content.delete_if_exists(vm, &item?)?;
@@ -322,7 +317,7 @@ impl PySetInner {
     }
 
     fn symmetric_difference_update(
-        &mut self,
+        &self,
         others: Args<PyIterable>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
@@ -337,14 +332,14 @@ impl PySetInner {
     }
 
     fn hash(&self, vm: &VirtualMachine) -> PyResult<pyhash::PyHash> {
-        pyhash::hash_iter_unordered(self.content.keys(), vm)
+        pyhash::hash_iter_unordered(self.content.keys().iter(), vm)
     }
 }
 
 macro_rules! try_set_cmp {
     ($vm:expr, $other:expr, $op:expr) => {
         Ok(match_class!(match ($other) {
-            set @ PySet => ($vm.new_bool($op(&*set.inner.borrow())?)),
+            set @ PySet => ($vm.new_bool($op(&set.inner)?)),
             frozen @ PyFrozenSet => ($vm.new_bool($op(&frozen.inner)?)),
             _ => $vm.ctx.not_implemented(),
         }));
@@ -353,13 +348,11 @@ macro_rules! try_set_cmp {
 
 macro_rules! multi_args_set {
     ($vm:expr, $others:expr, $zelf:expr, $op:tt) => {{
-        let mut res = $zelf.inner.borrow().copy();
+        let mut res = $zelf.inner.copy();
         for other in $others {
             res = res.$op(other, $vm)?
         }
-        Ok(Self {
-            inner: RefCell::new(res),
-        })
+        Ok(Self { inner: res })
     }};
 }
 
@@ -372,61 +365,61 @@ impl PySet {
         vm: &VirtualMachine,
     ) -> PyResult<PySetRef> {
         Self {
-            inner: RefCell::new(PySetInner::from_arg(iterable, vm)?),
+            inner: PySetInner::from_arg(iterable, vm)?,
         }
         .into_ref_with_type(vm, cls)
     }
 
     #[pymethod(name = "__len__")]
     fn len(&self) -> usize {
-        self.inner.borrow().len()
+        self.inner.len()
     }
 
     #[pymethod(name = "__sizeof__")]
     fn sizeof(&self) -> usize {
-        std::mem::size_of::<Self>() + self.inner.borrow().sizeof()
+        std::mem::size_of::<Self>() + self.inner.sizeof()
     }
 
     #[pymethod]
     fn copy(&self) -> Self {
         Self {
-            inner: RefCell::new(self.inner.borrow().copy()),
+            inner: self.inner.copy(),
         }
     }
 
     #[pymethod(name = "__contains__")]
     fn contains(&self, needle: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
-        self.inner.borrow().contains(&needle, vm)
+        self.inner.contains(&needle, vm)
     }
 
     #[pymethod(name = "__eq__")]
     fn eq(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        try_set_cmp!(vm, other, |other| self.inner.borrow().eq(other, vm))
+        try_set_cmp!(vm, other, |other| self.inner.eq(other, vm))
     }
 
     #[pymethod(name = "__ne__")]
     fn ne(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        try_set_cmp!(vm, other, |other| self.inner.borrow().ne(other, vm))
+        try_set_cmp!(vm, other, |other| self.inner.ne(other, vm))
     }
 
     #[pymethod(name = "__ge__")]
     fn ge(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        try_set_cmp!(vm, other, |other| self.inner.borrow().ge(other, vm))
+        try_set_cmp!(vm, other, |other| self.inner.ge(other, vm))
     }
 
     #[pymethod(name = "__gt__")]
     fn gt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        try_set_cmp!(vm, other, |other| self.inner.borrow().gt(other, vm))
+        try_set_cmp!(vm, other, |other| self.inner.gt(other, vm))
     }
 
     #[pymethod(name = "__le__")]
     fn le(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        try_set_cmp!(vm, other, |other| self.inner.borrow().le(other, vm))
+        try_set_cmp!(vm, other, |other| self.inner.le(other, vm))
     }
 
     #[pymethod(name = "__lt__")]
     fn lt(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        try_set_cmp!(vm, other, |other| self.inner.borrow().lt(other, vm))
+        try_set_cmp!(vm, other, |other| self.inner.lt(other, vm))
     }
 
     #[pymethod]
@@ -455,17 +448,17 @@ impl PySet {
 
     #[pymethod]
     fn issubset(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<bool> {
-        self.inner.borrow().issubset(other, vm)
+        self.inner.issubset(other, vm)
     }
 
     #[pymethod]
     fn issuperset(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<bool> {
-        self.inner.borrow().issuperset(other, vm)
+        self.inner.issuperset(other, vm)
     }
 
     #[pymethod]
     fn isdisjoint(&self, other: PyIterable, vm: &VirtualMachine) -> PyResult<bool> {
-        self.inner.borrow().isdisjoint(other, vm)
+        self.inner.isdisjoint(other, vm)
     }
 
     #[pymethod(name = "__or__")]
@@ -510,16 +503,15 @@ impl PySet {
 
     #[pymethod(name = "__iter__")]
     fn iter(&self, vm: &VirtualMachine) -> PyListIterator {
-        self.inner.borrow().iter(vm)
+        self.inner.iter(vm)
     }
 
     #[pymethod(name = "__repr__")]
     fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
-        let inner = zelf.inner.borrow();
-        let s = if inner.len() == 0 {
+        let s = if zelf.inner.len() == 0 {
             "set()".to_owned()
         } else if let Some(_guard) = ReprGuard::enter(zelf.as_object()) {
-            inner.repr(vm)?
+            zelf.inner.repr(vm)?
         } else {
             "set(...)".to_owned()
         };
@@ -528,68 +520,64 @@ impl PySet {
 
     #[pymethod]
     pub fn add(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.inner.borrow_mut().add(&item, vm)?;
+        self.inner.add(&item, vm)?;
         Ok(())
     }
 
     #[pymethod]
     fn remove(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.inner.borrow_mut().remove(&item, vm)
+        self.inner.remove(&item, vm)
     }
 
     #[pymethod]
     fn discard(&self, item: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        self.inner.borrow_mut().discard(&item, vm)?;
+        self.inner.discard(&item, vm)?;
         Ok(())
     }
 
     #[pymethod]
     fn clear(&self) {
-        self.inner.borrow_mut().clear()
+        self.inner.clear()
     }
 
     #[pymethod]
     fn pop(&self, vm: &VirtualMachine) -> PyResult {
-        self.inner.borrow_mut().pop(vm)
+        self.inner.pop(vm)
     }
 
     #[pymethod(name = "__ior__")]
     fn ior(zelf: PyRef<Self>, iterable: SetIterable, vm: &VirtualMachine) -> PyResult {
-        zelf.inner.borrow_mut().update(iterable.iterable, vm)?;
+        zelf.inner.update(iterable.iterable, vm)?;
         Ok(zelf.as_object().clone())
     }
 
     #[pymethod]
     fn update(&self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult {
-        self.inner.borrow_mut().update(others, vm)?;
+        self.inner.update(others, vm)?;
         Ok(vm.get_none())
     }
 
     #[pymethod]
     fn intersection_update(&self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult {
-        self.inner.borrow_mut().intersection_update(others, vm)?;
+        self.inner.intersection_update(others, vm)?;
         Ok(vm.get_none())
     }
 
     #[pymethod(name = "__iand__")]
     fn iand(zelf: PyRef<Self>, iterable: SetIterable, vm: &VirtualMachine) -> PyResult {
-        zelf.inner
-            .borrow_mut()
-            .intersection_update(iterable.iterable, vm)?;
+        zelf.inner.intersection_update(iterable.iterable, vm)?;
         Ok(zelf.as_object().clone())
     }
 
     #[pymethod]
     fn difference_update(&self, others: Args<PyIterable>, vm: &VirtualMachine) -> PyResult {
-        self.inner.borrow_mut().difference_update(others, vm)?;
+        self.inner.difference_update(others, vm)?;
         Ok(vm.get_none())
     }
 
     #[pymethod(name = "__isub__")]
     fn isub(zelf: PyRef<Self>, iterable: SetIterable, vm: &VirtualMachine) -> PyResult {
-        zelf.inner
-            .borrow_mut()
-            .difference_update(iterable.iterable, vm)?;
+        zelf.inner.difference_update(iterable.iterable, vm)?;
         Ok(zelf.as_object().clone())
     }
 
@@ -599,16 +587,13 @@ impl PySet {
         others: Args<PyIterable>,
         vm: &VirtualMachine,
     ) -> PyResult {
-        self.inner
-            .borrow_mut()
-            .symmetric_difference_update(others, vm)?;
+        self.inner.symmetric_difference_update(others, vm)?;
         Ok(vm.get_none())
     }
 
     #[pymethod(name = "__ixor__")]
     fn ixor(zelf: PyRef<Self>, iterable: SetIterable, vm: &VirtualMachine) -> PyResult {
         zelf.inner
-            .borrow_mut()
             .symmetric_difference_update(iterable.iterable, vm)?;
         Ok(zelf.as_object().clone())
     }
@@ -635,7 +620,7 @@ impl PyFrozenSet {
         vm: &VirtualMachine,
         it: impl IntoIterator<Item = PyObjectRef>,
     ) -> PyResult<Self> {
-        let mut inner = PySetInner::default();
+        let inner = PySetInner::default();
         for elem in it {
             inner.add(&elem, vm)?;
         }

vm/src/obj/objslice.rs

@@ -2,7 +2,8 @@ use super::objint::PyInt;
 use super::objtype::PyClassRef;
 use crate::function::{OptionalArg, PyFuncArgs};
 use crate::pyobject::{
-    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, TryIntoRef,
+    IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
+    TryIntoRef,
 };
 use crate::vm::VirtualMachine;
 use num_bigint::{BigInt, ToBigInt};
@@ -15,6 +16,7 @@ pub struct PySlice {
     pub stop: PyObjectRef,
     pub step: Option<PyObjectRef>,
 }
+impl ThreadSafe for PySlice {}
 
 impl PyValue for PySlice {
     fn class(vm: &VirtualMachine) -> PyClassRef {

vm/src/obj/objstaticmethod.rs

@@ -1,6 +1,6 @@
 use super::objtype::PyClassRef;
 use crate::function::OptionalArg;
-use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue};
+use crate::pyobject::{PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe};
 use crate::slots::SlotDescriptor;
 use crate::vm::VirtualMachine;
 
@@ -10,6 +10,7 @@ pub struct PyStaticMethod {
     pub callable: PyObjectRef,
 }
 pub type PyStaticMethodRef = PyRef<PyStaticMethod>;
+impl ThreadSafe for PyStaticMethod {}
 
 impl PyValue for PyStaticMethod {
     fn class(vm: &VirtualMachine) -> PyClassRef {

vm/src/obj/objstr.rs

@@ -21,17 +21,17 @@ use super::objsequence::PySliceableSequence;
 use super::objslice::PySliceRef;
 use super::objtuple;
 use super::objtype::{self, PyClassRef};
-use super::pystr::{adjust_indices, PyCommonString, StringRange};
+use super::pystr::{self, adjust_indices, PyCommonString, PyCommonStringWrapper, StringRange};
 use crate::cformat::{
     CFormatPart, CFormatPreconversor, CFormatQuantity, CFormatSpec, CFormatString, CFormatType,
     CNumberType,
 };
 use crate::format::{FormatParseError, FormatPart, FormatPreconversor, FormatSpec, FormatString};
-use crate::function::{OptionalArg, PyFuncArgs};
+use crate::function::{OptionalArg, OptionalOption, PyFuncArgs};
 use crate::pyhash;
 use crate::pyobject::{
     Either, IdProtocol, IntoPyObject, ItemProtocol, PyClassImpl, PyContext, PyIterable,
-    PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TryIntoRef, TypeProtocol,
+    PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe, TryFromObject, TryIntoRef, TypeProtocol,
 };
 use crate::vm::VirtualMachine;
 
@@ -51,6 +51,7 @@ pub struct PyString {
     value: String,
     hash: AtomicCell<Option<pyhash::PyHash>>,
 }
+impl ThreadSafe for PyString {}
 
 impl PyString {
     #[inline]
@@ -100,6 +101,7 @@ pub struct PyStringIterator {
     pub string: PyStringRef,
     position: AtomicCell<usize>,
 }
+impl ThreadSafe for PyStringIterator {}
 
 impl PyValue for PyStringIterator {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -133,6 +135,7 @@ pub struct PyStringReverseIterator {
     pub position: AtomicCell<isize>,
     pub string: PyStringRef,
 }
+impl ThreadSafe for PyStringReverseIterator {}
 
 impl PyValue for PyStringReverseIterator {
     fn class(vm: &VirtualMachine) -> PyClassRef {
@@ -144,12 +147,13 @@ impl PyValue for PyStringReverseIterator {
 impl PyStringReverseIterator {
     #[pymethod(name = "__next__")]
     fn next(&self, vm: &VirtualMachine) -> PyResult<String> {
-        let pos = self.position.fetch_sub(1) as usize - 1;
-        if let Some(c) = self.string.value.chars().nth(pos) {
-            Ok(c.to_string())
-        } else {
-            Err(objiter::new_stop_iteration(vm))
+        let pos = self.position.fetch_sub(1);
+        if pos > 0 {
+            if let Some(c) = self.string.value.chars().nth(pos as usize - 1) {
+                return Ok(c.to_string());
+            }
         }
+        Err(objiter::new_stop_iteration(vm))
     }
 
     #[pymethod(name = "__iter__")]
@@ -168,18 +172,6 @@ struct StrArgs {
     errors: OptionalArg<PyStringRef>,
 }
 
-#[derive(FromArgs)]
-struct SplitLineArgs {
-    #[pyarg(positional_or_keyword, optional = true)]
-    keepends: OptionalArg<bool>,
-}
-
-#[derive(FromArgs)]
-struct ExpandtabsArgs {
-    #[pyarg(positional_or_keyword, optional = true)]
-    tabsize: OptionalArg<usize>,
-}
-
 #[pyimpl(flags(BASETYPE))]
 impl PyString {
     #[pyslot]
@@ -457,26 +449,24 @@ impl PyString {
     #[pymethod]
     fn split(&self, args: SplitArgs, vm: &VirtualMachine) -> PyResult {
         let elements = self.value.py_split(
-            args.non_empty_sep(vm)?,
-            args.maxsplit,
+            args,
             vm,
             |v, s, vm| v.split(s).map(|s| vm.ctx.new_str(s)).collect(),
             |v, s, n, vm| v.splitn(n, s).map(|s| vm.ctx.new_str(s)).collect(),
             |v, n, vm| v.py_split_whitespace(n, |s| vm.ctx.new_str(s)),
-        );
+        )?;
         Ok(vm.ctx.new_list(elements))
     }
 
     #[pymethod]
     fn rsplit(&self, args: SplitArgs, vm: &VirtualMachine) -> PyResult {
         let mut elements = self.value.py_split(
-            args.non_empty_sep(vm)?,
-            args.maxsplit,
+            args,
             vm,
             |v, s, vm| v.rsplit(s).map(|s| vm.ctx.new_str(s)).collect(),
             |v, s, n, vm| v.rsplitn(n, s).map(|s| vm.ctx.new_str(s)).collect(),
             |v, n, vm| v.py_rsplit_whitespace(n, |s| vm.ctx.new_str(s)),
-        );
+        )?;
         // Unlike Python rsplit, Rust rsplitn returns an iterator that
         // starts from the end of the string.
         elements.reverse();
@@ -484,51 +474,42 @@ impl PyString {
     }
 
     #[pymethod]
-    fn strip(&self, chars: OptionalArg<Option<PyStringRef>>) -> String {
-        let chars = chars.flat_option();
-        let chars = match chars {
-            Some(ref chars) => &chars.value,
-            None => return self.value.trim().to_owned(),
-        };
-        self.value.trim_matches(|c| chars.contains(c)).to_owned()
-    }
-
-    #[pymethod]
-    fn lstrip(&self, chars: OptionalArg<Option<PyStringRef>>) -> String {
-        let chars = chars.flat_option();
-        let chars = match chars {
-            Some(ref chars) => &chars.value,
-            None => return self.value.trim_start().to_owned(),
-        };
+    fn strip(&self, chars: OptionalOption<PyStringRef>) -> String {
         self.value
-            .trim_start_matches(|c| chars.contains(c))
+            .py_strip(
+                chars,
+                |s, chars| s.trim_matches(|c| chars.contains(c)),
+                |s| s.trim(),
+            )
             .to_owned()
     }
 
     #[pymethod]
-    fn rstrip(&self, chars: OptionalArg<Option<PyStringRef>>) -> String {
-        let chars = chars.flat_option();
-        let chars = match chars {
-            Some(ref chars) => &chars.value,
-            None => return self.value.trim_end().to_owned(),
-        };
+    fn lstrip(&self, chars: OptionalOption<PyStringRef>) -> String {
         self.value
-            .trim_end_matches(|c| chars.contains(c))
+            .py_strip(
+                chars,
+                |s, chars| s.trim_start_matches(|c| chars.contains(c)),
+                |s| s.trim_start(),
+            )
             .to_owned()
     }
 
     #[pymethod]
-    fn endswith(
-        &self,
-        suffix: PyObjectRef,
-        start: OptionalArg<Option<isize>>,
-        end: OptionalArg<Option<isize>>,
-        vm: &VirtualMachine,
-    ) -> PyResult<bool> {
+    fn rstrip(&self, chars: OptionalOption<PyStringRef>) -> String {
+        self.value
+            .py_strip(
+                chars,
+                |s, chars| s.trim_end_matches(|c| chars.contains(c)),
+                |s| s.trim_end(),
+            )
+            .to_owned()
+    }
+
+    #[pymethod]
+    fn endswith(&self, args: pystr::StartsEndsWithArgs, vm: &VirtualMachine) -> PyResult<bool> {
         self.value.as_str().py_startsendswith(
-            suffix,
-            start,
-            end,
+            args,
             "endswith",
             "str",
             |s, x: &PyStringRef| s.ends_with(x.as_str()),
@@ -537,17 +518,9 @@ impl PyString {
     }
 
     #[pymethod]
-    fn startswith(
-        &self,
-        prefix: PyObjectRef,
-        start: OptionalArg<Option<isize>>,
-        end: OptionalArg<Option<isize>>,
-        vm: &VirtualMachine,
-    ) -> PyResult<bool> {
+    fn startswith(&self, args: pystr::StartsEndsWithArgs, vm: &VirtualMachine) -> PyResult<bool> {
         self.value.as_str().py_startsendswith(
-            prefix,
-            start,
-            end,
+            args,
             "startswith",
             "str",
             |s, x: &PyStringRef| s.starts_with(x.as_str()),
@@ -799,14 +772,13 @@ impl PyString {
     }
 
     #[pymethod]
-    fn splitlines(&self, args: SplitLineArgs, vm: &VirtualMachine) -> PyObjectRef {
-        let keepends = args.keepends.unwrap_or(false);
+    fn splitlines(&self, args: pystr::SplitLinesArgs, vm: &VirtualMachine) -> PyObjectRef {
         let mut elements = vec![];
         let mut curr = "".to_owned();
         let mut chars = self.value.chars().peekable();
         while let Some(ch) = chars.next() {
             if ch == '\n' || ch == '\r' {
-                if keepends {
+                if args.keepends {
                     curr.push(ch);
                 }
                 if ch == '\r' && chars.peek() == Some(&'\n') {
@@ -1082,8 +1054,8 @@ impl PyString {
     }
 
     #[pymethod]
-    fn expandtabs(&self, args: ExpandtabsArgs) -> String {
-        let tab_stop = args.tabsize.unwrap_or(8);
+    fn expandtabs(&self, args: pystr::ExpandTabsArgs) -> String {
+        let tab_stop = args.tabsize();
         let mut expanded_str = String::with_capacity(self.value.len());
         let mut tab_size = tab_stop;
         let mut col_count = 0 as usize;
@@ -1298,28 +1270,7 @@ impl TryFromObject for std::ffi::CString {
     }
 }
 
-#[derive(FromArgs)]
-struct SplitArgs {
-    #[pyarg(positional_or_keyword, default = "None")]
-    sep: Option<PyStringRef>,
-    #[pyarg(positional_or_keyword, default = "-1")]
-    maxsplit: isize,
-}
-
-impl SplitArgs {
-    fn non_empty_sep<'a>(&'a self, vm: &VirtualMachine) -> PyResult<Option<&'a str>> {
-        let sep = if let Some(s) = self.sep.as_ref() {
-            let sep = s.as_str();
-            if sep.is_empty() {
-                return Err(vm.new_value_error("empty separator".to_owned()));
-            }
-            Some(sep)
-        } else {
-            None
-        };
-        Ok(sep)
-    }
-}
+type SplitArgs = pystr::SplitArgs<PyStringRef, str, char>;
 
 pub fn init(ctx: &PyContext) {
     PyString::extend_class(ctx, &ctx.types.str_type);
@@ -1799,11 +1750,21 @@ mod tests {
     }
 }
 
-impl PyCommonString<'_, char> for str {
+impl PyCommonStringWrapper<str> for PyStringRef {
+    fn as_ref(&self) -> &str {
+        self.value.as_str()
+    }
+}
+
+impl PyCommonString<char> for str {
     fn get_slice(&self, range: std::ops::Range<usize>) -> &Self {
         &self[range]
     }
 
+    fn is_empty(&self) -> bool {
+        Self::is_empty(self)
+    }
+
     fn len(&self) -> usize {
         Self::len(self)
     }

vm/src/obj/objtuple.rs

@@ -11,7 +11,7 @@ use crate::pyobject::{
     PyArithmaticValue::{self, *},
     PyClassImpl, PyComparisonValue, PyContext, PyObjectRef, PyRef, PyResult, PyValue, ThreadSafe,
 };
-use crate::sequence::{self, SimpleSeq};
+use crate::sequence;
 use crate::vm::{ReprGuard, VirtualMachine};
 
 /// tuple() -> empty tuple

vm/src/obj/pystr.rs

@@ -1,6 +1,75 @@
 use crate::function::{single_or_tuple_any, OptionalOption};
 use crate::pyobject::{PyObjectRef, PyResult, TryFromObject, TypeProtocol};
 use crate::vm::VirtualMachine;
+use num_traits::cast::ToPrimitive;
+
+#[derive(FromArgs)]
+pub struct SplitArgs<T, S, E>
+where
+    T: TryFromObject + PyCommonStringWrapper<S>,
+    S: ?Sized + PyCommonString<E>,
+{
+    #[pyarg(positional_or_keyword, default = "None")]
+    sep: Option<T>,
+    #[pyarg(positional_or_keyword, default = "-1")]
+    maxsplit: isize,
+    _phantom1: std::marker::PhantomData<S>,
+    _phantom2: std::marker::PhantomData<E>,
+}
+
+impl<T, S, E> SplitArgs<T, S, E>
+where
+    T: TryFromObject + PyCommonStringWrapper<S>,
+    S: ?Sized + PyCommonString<E>,
+{
+    pub fn get_value(self, vm: &VirtualMachine) -> PyResult<(Option<T>, isize)> {
+        let sep = if let Some(s) = self.sep {
+            let sep = s.as_ref();
+            if sep.is_empty() {
+                return Err(vm.new_value_error("empty separator".to_owned()));
+            }
+            Some(s)
+        } else {
+            None
+        };
+        Ok((sep, self.maxsplit))
+    }
+}
+
+#[derive(FromArgs)]
+pub struct SplitLinesArgs {
+    #[pyarg(positional_or_keyword, default = "false")]
+    pub keepends: bool,
+}
+
+#[derive(FromArgs)]
+pub struct ExpandTabsArgs {
+    #[pyarg(positional_or_keyword, default = "8")]
+    tabsize: isize,
+}
+
+impl ExpandTabsArgs {
+    pub fn tabsize(&self) -> usize {
+        self.tabsize.to_usize().unwrap_or(0)
+    }
+}
+
+#[derive(FromArgs)]
+pub struct StartsEndsWithArgs {
+    #[pyarg(positional_only, optional = false)]
+    affix: PyObjectRef,
+    #[pyarg(positional_only, optional = true)]
+    start: OptionalOption<isize>,
+    #[pyarg(positional_only, optional = true)]
+    end: OptionalOption<isize>,
+}
+
+impl StartsEndsWithArgs {
+    fn get_value(self, len: usize) -> (PyObjectRef, std::ops::Range<usize>) {
+        let range = adjust_indices(self.start, self.end, len);
+        (self.affix, range)
+    }
+}
 
 // help get optional string indices
 pub fn adjust_indices(
@@ -37,36 +106,43 @@ impl StringRange for std::ops::Range<usize> {
     }
 }
 
-pub trait PyCommonString<'a, E>
+pub trait PyCommonStringWrapper<S>
 where
-    Self: 'a,
+    S: ?Sized,
 {
+    fn as_ref(&self) -> &S;
+}
+
+pub trait PyCommonString<E> {
     fn get_slice(&self, range: std::ops::Range<usize>) -> &Self;
     fn len(&self) -> usize;
+    fn is_empty(&self) -> bool;
 
-    fn py_split<SP, SN, SW, R>(
+    fn py_split<T, SP, SN, SW, R>(
         &self,
-        sep: Option<&Self>,
-        maxsplit: isize,
+        args: SplitArgs<T, Self, E>,
         vm: &VirtualMachine,
         split: SP,
         splitn: SN,
         splitw: SW,
-    ) -> Vec<R>
+    ) -> PyResult<Vec<R>>
     where
+        T: TryFromObject + PyCommonStringWrapper<Self>,
         SP: Fn(&Self, &Self, &VirtualMachine) -> Vec<R>,
         SN: Fn(&Self, &Self, usize, &VirtualMachine) -> Vec<R>,
         SW: Fn(&Self, isize, &VirtualMachine) -> Vec<R>,
     {
-        if let Some(pattern) = sep {
+        let (sep, maxsplit) = args.get_value(vm)?;
+        let splited = if let Some(pattern) = sep {
             if maxsplit < 0 {
-                split(self, pattern, vm)
+                split(self, pattern.as_ref(), vm)
             } else {
-                splitn(self, pattern, (maxsplit + 1) as usize, vm)
+                splitn(self, pattern.as_ref(), (maxsplit + 1) as usize, vm)
             }
         } else {
             splitw(self, maxsplit, vm)
-        }
+        };
+        Ok(splited)
     }
     fn py_split_whitespace<F>(&self, maxsplit: isize, convert: F) -> Vec<PyObjectRef>
     where
@@ -75,13 +151,10 @@ where
     where
         F: Fn(&Self) -> PyObjectRef;
 
-    #[allow(clippy::too_many_arguments)]
     #[inline]
     fn py_startsendswith<T, F>(
         &self,
-        affix: PyObjectRef,
-        start: OptionalOption<isize>,
-        end: OptionalOption<isize>,
+        args: StartsEndsWithArgs,
         func_name: &str,
         py_type_name: &str,
         func: F,
@@ -91,7 +164,7 @@ where
         T: TryFromObject,
         F: Fn(&Self, &T) -> bool,
     {
-        let range = adjust_indices(start, end, self.len());
+        let (affix, range) = args.get_value(self.len());
         if range.is_normal() {
             let value = self.get_slice(range);
             single_or_tuple_any(
@@ -112,4 +185,22 @@ where
             Ok(false)
         }
     }
+
+    fn py_strip<'a, S, FC, FD>(
+        &'a self,
+        chars: OptionalOption<S>,
+        func_chars: FC,
+        func_default: FD,
+    ) -> &'a Self
+    where
+        S: PyCommonStringWrapper<Self>,
+        FC: Fn(&'a Self, &Self) -> &'a Self,
+        FD: Fn(&'a Self) -> &'a Self,
+    {
+        let chars = chars.flat_option();
+        match chars {
+            Some(chars) => func_chars(self, chars.as_ref()),
+            None => func_default(self),
+        }
+    }
 }

vm/src/py_serde.rs

@@ -1,6 +1,5 @@
-use std::fmt;
-
-use serde;
+use num_traits::cast::ToPrimitive;
+use num_traits::sign::Signed;
 use serde::de::{DeserializeSeed, Visitor};
 use serde::ser::{Serialize, SerializeMap, SerializeSeq};
 
@@ -10,8 +9,6 @@ use crate::obj::{
 };
 use crate::pyobject::{IdProtocol, ItemProtocol, PyObjectRef, TypeProtocol};
 use crate::VirtualMachine;
-use num_traits::cast::ToPrimitive;
-use num_traits::sign::Signed;
 
 #[inline]
 pub fn serialize<S>(
@@ -137,7 +134,7 @@ impl<'de> DeserializeSeed<'de> for PyObjectDeserializer<'de> {
 impl<'de> Visitor<'de> for PyObjectDeserializer<'de> {
     type Value = PyObjectRef;
 
-    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+    fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
         formatter.write_str("a type that can deserialise in Python")
     }
 

vm/src/pyhash.rs

@@ -84,13 +84,13 @@ pub fn hash_iter<'a, I: std::iter::Iterator<Item = &'a PyObjectRef>>(
     Ok(hasher.finish() as PyHash)
 }
 
-pub fn hash_iter_unordered<I: std::iter::Iterator<Item = PyObjectRef>>(
+pub fn hash_iter_unordered<'a, I: std::iter::Iterator<Item = &'a PyObjectRef>>(
     iter: I,
     vm: &VirtualMachine,
 ) -> PyResult<PyHash> {
     let mut hash: PyHash = 0;
     for element in iter {
-        let item_hash = vm._hash(&element)?;
+        let item_hash = vm._hash(element)?;
         // xor is commutative and hash should be independent of order
         hash ^= item_hash;
     }

vm/src/stdlib/binascii.rs

@@ -1,147 +1,144 @@
-use crate::function::OptionalArg;
-use crate::obj::objbytearray::{PyByteArray, PyByteArrayRef};
-use crate::obj::objbyteinner::PyBytesLike;
-use crate::obj::objbytes::{PyBytes, PyBytesRef};
-use crate::obj::objstr::{PyString, PyStringRef};
-use crate::pyobject::{PyObjectRef, PyResult, TryFromObject, TypeProtocol};
-use crate::vm::VirtualMachine;
+pub(crate) use decl::make_module;
 
-use crc::{crc32, Hasher32};
-use itertools::Itertools;
+#[pymodule(name = "binascii")]
+mod decl {
+    use crate::function::OptionalArg;
+    use crate::obj::objbytearray::{PyByteArray, PyByteArrayRef};
+    use crate::obj::objbyteinner::PyBytesLike;
+    use crate::obj::objbytes::{PyBytes, PyBytesRef};
+    use crate::obj::objstr::{PyString, PyStringRef};
+    use crate::pyobject::{PyObjectRef, PyResult, TryFromObject, TypeProtocol};
+    use crate::vm::VirtualMachine;
+    use crc::{crc32, Hasher32};
+    use itertools::Itertools;
 
-enum SerializedData {
-    Bytes(PyBytesRef),
-    Buffer(PyByteArrayRef),
-    Ascii(PyStringRef),
-}
+    enum SerializedData {
+        Bytes(PyBytesRef),
+        Buffer(PyByteArrayRef),
+        Ascii(PyStringRef),
+    }
 
-impl TryFromObject for SerializedData {
-    fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
-        match_class!(match obj {
-            b @ PyBytes => Ok(SerializedData::Bytes(b)),
-            b @ PyByteArray => Ok(SerializedData::Buffer(b)),
-            a @ PyString => {
-                if a.as_str().is_ascii() {
-                    Ok(SerializedData::Ascii(a))
-                } else {
-                    Err(vm.new_value_error(
-                        "string argument should contain only ASCII characters".to_owned(),
-                    ))
+    impl TryFromObject for SerializedData {
+        fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
+            match_class!(match obj {
+                b @ PyBytes => Ok(SerializedData::Bytes(b)),
+                b @ PyByteArray => Ok(SerializedData::Buffer(b)),
+                a @ PyString => {
+                    if a.as_str().is_ascii() {
+                        Ok(SerializedData::Ascii(a))
+                    } else {
+                        Err(vm.new_value_error(
+                            "string argument should contain only ASCII characters".to_owned(),
+                        ))
+                    }
                 }
+                obj => Err(vm.new_type_error(format!(
+                    "argument should be bytes, buffer or ASCII string, not '{}'",
+                    obj.class().name,
+                ))),
+            })
+        }
+    }
+
+    impl SerializedData {
+        #[inline]
+        pub fn with_ref<R>(&self, f: impl FnOnce(&[u8]) -> R) -> R {
+            match self {
+                SerializedData::Bytes(b) => f(b.get_value()),
+                SerializedData::Buffer(b) => f(&b.borrow_value().elements),
+                SerializedData::Ascii(a) => f(a.as_str().as_bytes()),
             }
-            obj => Err(vm.new_type_error(format!(
-                "argument should be bytes, buffer or ASCII string, not '{}'",
-                obj.class().name,
-            ))),
+        }
+    }
+
+    fn hex_nibble(n: u8) -> u8 {
+        match n {
+            0..=9 => b'0' + n,
+            10..=15 => b'a' + n,
+            _ => unreachable!(),
+        }
+    }
+
+    #[pyfunction(name = "b2a_hex")]
+    #[pyfunction]
+    fn hexlify(data: PyBytesLike) -> Vec<u8> {
+        data.with_ref(|bytes| {
+            let mut hex = Vec::<u8>::with_capacity(bytes.len() * 2);
+            for b in bytes.iter() {
+                hex.push(hex_nibble(b >> 4));
+                hex.push(hex_nibble(b & 0xf));
+            }
+            hex
         })
     }
-}
 
-impl SerializedData {
-    #[inline]
-    pub fn with_ref<R>(&self, f: impl FnOnce(&[u8]) -> R) -> R {
-        match self {
-            SerializedData::Bytes(b) => f(b.get_value()),
-            SerializedData::Buffer(b) => f(&b.borrow_value().elements),
-            SerializedData::Ascii(a) => f(a.as_str().as_bytes()),
+    fn unhex_nibble(c: u8) -> Option<u8> {
+        match c {
+            b'0'..=b'9' => Some(c - b'0'),
+            b'a'..=b'f' => Some(c - b'a' + 10),
+            b'A'..=b'F' => Some(c - b'A' + 10),
+            _ => None,
         }
     }
-}
 
-fn hex_nibble(n: u8) -> u8 {
-    match n {
-        0..=9 => b'0' + n,
-        10..=15 => b'a' + n,
-        _ => unreachable!(),
-    }
-}
-
-fn binascii_hexlify(data: PyBytesLike) -> Vec<u8> {
-    data.with_ref(|bytes| {
-        let mut hex = Vec::<u8>::with_capacity(bytes.len() * 2);
-        for b in bytes.iter() {
-            hex.push(hex_nibble(b >> 4));
-            hex.push(hex_nibble(b & 0xf));
-        }
-        hex
-    })
-}
-
-fn unhex_nibble(c: u8) -> Option<u8> {
-    match c {
-        b'0'..=b'9' => Some(c - b'0'),
-        b'a'..=b'f' => Some(c - b'a' + 10),
-        b'A'..=b'F' => Some(c - b'A' + 10),
-        _ => None,
-    }
-}
-
-fn binascii_unhexlify(data: SerializedData, vm: &VirtualMachine) -> PyResult<Vec<u8>> {
-    data.with_ref(|hex_bytes| {
-        if hex_bytes.len() % 2 != 0 {
-            return Err(vm.new_value_error("Odd-length string".to_owned()));
-        }
-
-        let mut unhex = Vec::<u8>::with_capacity(hex_bytes.len() / 2);
-        for (n1, n2) in hex_bytes.iter().tuples() {
-            if let (Some(n1), Some(n2)) = (unhex_nibble(*n1), unhex_nibble(*n2)) {
-                unhex.push(n1 << 4 | n2);
-            } else {
-                return Err(vm.new_value_error("Non-hexadecimal digit found".to_owned()));
+    #[pyfunction(name = "a2b_hex")]
+    #[pyfunction]
+    fn unhexlify(data: SerializedData, vm: &VirtualMachine) -> PyResult<Vec<u8>> {
+        data.with_ref(|hex_bytes| {
+            if hex_bytes.len() % 2 != 0 {
+                return Err(vm.new_value_error("Odd-length string".to_owned()));
             }
+
+            let mut unhex = Vec::<u8>::with_capacity(hex_bytes.len() / 2);
+            for (n1, n2) in hex_bytes.iter().tuples() {
+                if let (Some(n1), Some(n2)) = (unhex_nibble(*n1), unhex_nibble(*n2)) {
+                    unhex.push(n1 << 4 | n2);
+                } else {
+                    return Err(vm.new_value_error("Non-hexadecimal digit found".to_owned()));
+                }
+            }
+
+            Ok(unhex)
+        })
+    }
+
+    #[pyfunction]
+    fn crc32(data: SerializedData, value: OptionalArg<u32>, vm: &VirtualMachine) -> PyResult {
+        let crc = value.unwrap_or(0);
+
+        let mut digest = crc32::Digest::new_with_initial(crc32::IEEE, crc);
+        data.with_ref(|bytes| digest.write(&bytes));
+
+        Ok(vm.ctx.new_int(digest.sum32()))
+    }
+
+    #[derive(FromArgs)]
+    struct NewlineArg {
+        #[pyarg(keyword_only, default = "true")]
+        newline: bool,
+    }
+
+    /// trim a newline from the end of the bytestring, if it exists
+    fn trim_newline(b: &[u8]) -> &[u8] {
+        if b.ends_with(b"\n") {
+            &b[..b.len() - 1]
+        } else {
+            b
         }
+    }
 
-        Ok(unhex)
-    })
-}
+    #[pyfunction]
+    fn a2b_base64(s: SerializedData, vm: &VirtualMachine) -> PyResult<Vec<u8>> {
+        s.with_ref(|b| base64::decode(trim_newline(b)))
+            .map_err(|err| vm.new_value_error(format!("error decoding base64: {}", err)))
+    }
 
-fn binascii_crc32(data: SerializedData, value: OptionalArg<u32>, vm: &VirtualMachine) -> PyResult {
-    let crc = value.unwrap_or(0);
-
-    let mut digest = crc32::Digest::new_with_initial(crc32::IEEE, crc);
-    data.with_ref(|bytes| digest.write(&bytes));
-
-    Ok(vm.ctx.new_int(digest.sum32()))
-}
-
-#[derive(FromArgs)]
-struct NewlineArg {
-    #[pyarg(keyword_only, default = "true")]
-    newline: bool,
-}
-
-/// trim a newline from the end of the bytestring, if it exists
-fn trim_newline(b: &[u8]) -> &[u8] {
-    if b.ends_with(b"\n") {
-        &b[..b.len() - 1]
-    } else {
-        b
+    #[pyfunction]
+    fn b2a_base64(data: PyBytesLike, NewlineArg { newline }: NewlineArg) -> Vec<u8> {
+        let mut encoded = data.with_ref(base64::encode).into_bytes();
+        if newline {
+            encoded.push(b'\n');
+        }
+        encoded
     }
 }
-
-fn binascii_a2b_base64(s: SerializedData, vm: &VirtualMachine) -> PyResult<Vec<u8>> {
-    s.with_ref(|b| base64::decode(trim_newline(b)))
-        .map_err(|err| vm.new_value_error(format!("error decoding base64: {}", err)))
-}
-
-fn binascii_b2a_base64(data: PyBytesLike, NewlineArg { newline }: NewlineArg) -> Vec<u8> {
-    let mut encoded = data.with_ref(base64::encode).into_bytes();
-    if newline {
-        encoded.push(b'\n');
-    }
-    encoded
-}
-
-pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
-    let ctx = &vm.ctx;
-
-    py_module!(vm, "binascii", {
-        "hexlify" => ctx.new_function(binascii_hexlify),
-        "b2a_hex" => ctx.new_function(binascii_hexlify),
-        "unhexlify" => ctx.new_function(binascii_unhexlify),
-        "a2b_hex" => ctx.new_function(binascii_unhexlify),
-        "crc32" => ctx.new_function(binascii_crc32),
-        "a2b_base64" => ctx.new_function(binascii_a2b_base64),
-        "b2a_base64" => ctx.new_function(binascii_b2a_base64),
-    })
-}

vm/src/stdlib/collections.rs

@@ -1,395 +1,398 @@
-use crate::function::OptionalArg;
-use crate::obj::{objiter, objtype::PyClassRef};
-use crate::pyobject::{
-    IdProtocol, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyIterable, PyObjectRef,
-    PyRef, PyResult, PyValue,
-};
-use crate::sequence::{self, SimpleSeq};
-use crate::vm::ReprGuard;
-use crate::VirtualMachine;
-use itertools::Itertools;
-use std::cell::{Cell, RefCell};
-use std::collections::VecDeque;
+pub(crate) use _collections::make_module;
 
-#[pyclass(name = "deque")]
-#[derive(Debug, Clone)]
-struct PyDeque {
-    deque: RefCell<VecDeque<PyObjectRef>>,
-    maxlen: Cell<Option<usize>>,
-}
-type PyDequeRef = PyRef<PyDeque>;
+#[pymodule]
+mod _collections {
+    use crate::function::OptionalArg;
+    use crate::obj::{objiter, objtype::PyClassRef};
+    use crate::pyobject::{
+        IdProtocol, PyArithmaticValue::*, PyClassImpl, PyComparisonValue, PyIterable, PyObjectRef,
+        PyRef, PyResult, PyValue,
+    };
+    use crate::sequence;
+    use crate::vm::ReprGuard;
+    use crate::VirtualMachine;
+    use itertools::Itertools;
+    use std::cell::{Cell, RefCell};
+    use std::collections::VecDeque;
 
-impl PyValue for PyDeque {
-    fn class(vm: &VirtualMachine) -> PyClassRef {
-        vm.class("_collections", "deque")
+    #[pyclass(name = "deque")]
+    #[derive(Debug, Clone)]
+    struct PyDeque {
+        deque: RefCell<VecDeque<PyObjectRef>>,
+        maxlen: Cell<Option<usize>>,
     }
-}
+    type PyDequeRef = PyRef<PyDeque>;
 
-#[derive(FromArgs)]
-struct PyDequeOptions {
-    #[pyarg(positional_or_keyword, default = "None")]
-    maxlen: Option<usize>,
-}
-
-impl PyDeque {
-    fn borrow_deque<'a>(&'a self) -> impl std::ops::Deref<Target = VecDeque<PyObjectRef>> + 'a {
-        self.deque.borrow()
-    }
-}
-
-#[pyimpl(flags(BASETYPE))]
-impl PyDeque {
-    #[pyslot]
-    fn tp_new(
-        cls: PyClassRef,
-        iter: OptionalArg<PyIterable>,
-        PyDequeOptions { maxlen }: PyDequeOptions,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyRef<Self>> {
-        let py_deque = PyDeque {
-            deque: RefCell::default(),
-            maxlen: maxlen.into(),
-        };
-        if let OptionalArg::Present(iter) = iter {
-            py_deque.extend(iter, vm)?;
+    impl PyValue for PyDeque {
+        fn class(vm: &VirtualMachine) -> PyClassRef {
+            vm.class("_collections", "deque")
         }
-        py_deque.into_ref_with_type(vm, cls)
     }
 
-    #[pymethod]
-    fn append(&self, obj: PyObjectRef) {
-        let mut deque = self.deque.borrow_mut();
-        if self.maxlen.get() == Some(deque.len()) {
-            deque.pop_front();
+    #[derive(FromArgs)]
+    struct PyDequeOptions {
+        #[pyarg(positional_or_keyword, default = "None")]
+        maxlen: Option<usize>,
+    }
+
+    impl PyDeque {
+        fn borrow_deque<'a>(&'a self) -> impl std::ops::Deref<Target = VecDeque<PyObjectRef>> + 'a {
+            self.deque.borrow()
         }
-        deque.push_back(obj);
     }
 
-    #[pymethod]
-    fn appendleft(&self, obj: PyObjectRef) {
-        let mut deque = self.deque.borrow_mut();
-        if self.maxlen.get() == Some(deque.len()) {
-            deque.pop_back();
-        }
-        deque.push_front(obj);
-    }
-
-    #[pymethod]
-    fn clear(&self) {
-        self.deque.borrow_mut().clear()
-    }
-
-    #[pymethod]
-    fn copy(&self) -> Self {
-        self.clone()
-    }
-
-    #[pymethod]
-    fn count(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
-        let mut count = 0;
-        for elem in self.deque.borrow().iter() {
-            if vm.identical_or_equal(elem, &obj)? {
-                count += 1;
+    #[pyimpl(flags(BASETYPE))]
+    impl PyDeque {
+        #[pyslot]
+        fn tp_new(
+            cls: PyClassRef,
+            iter: OptionalArg<PyIterable>,
+            PyDequeOptions { maxlen }: PyDequeOptions,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyRef<Self>> {
+            let py_deque = PyDeque {
+                deque: RefCell::default(),
+                maxlen: maxlen.into(),
+            };
+            if let OptionalArg::Present(iter) = iter {
+                py_deque.extend(iter, vm)?;
             }
+            py_deque.into_ref_with_type(vm, cls)
         }
-        Ok(count)
-    }
 
-    #[pymethod]
-    fn extend(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> {
-        // TODO: use length_hint here and for extendleft
-        for elem in iter.iter(vm)? {
-            self.append(elem?);
-        }
-        Ok(())
-    }
-
-    #[pymethod]
-    fn extendleft(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> {
-        for elem in iter.iter(vm)? {
-            self.appendleft(elem?);
-        }
-        Ok(())
-    }
-
-    #[pymethod]
-    fn index(
-        &self,
-        obj: PyObjectRef,
-        start: OptionalArg<usize>,
-        stop: OptionalArg<usize>,
-        vm: &VirtualMachine,
-    ) -> PyResult<usize> {
-        let deque = self.deque.borrow();
-        let start = start.unwrap_or(0);
-        let stop = stop.unwrap_or_else(|| deque.len());
-        for (i, elem) in deque.iter().skip(start).take(stop - start).enumerate() {
-            if vm.identical_or_equal(elem, &obj)? {
-                return Ok(i);
+        #[pymethod]
+        fn append(&self, obj: PyObjectRef) {
+            let mut deque = self.deque.borrow_mut();
+            if self.maxlen.get() == Some(deque.len()) {
+                deque.pop_front();
             }
-        }
-        Err(vm.new_value_error(
-            vm.to_repr(&obj)
-                .map(|repr| format!("{} is not in deque", repr))
-                .unwrap_or_else(|_| String::new()),
-        ))
-    }
-
-    #[pymethod]
-    fn insert(&self, idx: i32, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
-        let mut deque = self.deque.borrow_mut();
-
-        if self.maxlen.get() == Some(deque.len()) {
-            return Err(vm.new_index_error("deque already at its maximum size".to_owned()));
+            deque.push_back(obj);
         }
 
-        let idx = if idx < 0 {
-            if -idx as usize > deque.len() {
-                0
+        #[pymethod]
+        fn appendleft(&self, obj: PyObjectRef) {
+            let mut deque = self.deque.borrow_mut();
+            if self.maxlen.get() == Some(deque.len()) {
+                deque.pop_back();
+            }
+            deque.push_front(obj);
+        }
+
+        #[pymethod]
+        fn clear(&self) {
+            self.deque.borrow_mut().clear()
+        }
+
+        #[pymethod]
+        fn copy(&self) -> Self {
+            self.clone()
+        }
+
+        #[pymethod]
+        fn count(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<usize> {
+            let mut count = 0;
+            for elem in self.deque.borrow().iter() {
+                if vm.identical_or_equal(elem, &obj)? {
+                    count += 1;
+                }
+            }
+            Ok(count)
+        }
+
+        #[pymethod]
+        fn extend(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> {
+            // TODO: use length_hint here and for extendleft
+            for elem in iter.iter(vm)? {
+                self.append(elem?);
+            }
+            Ok(())
+        }
+
+        #[pymethod]
+        fn extendleft(&self, iter: PyIterable, vm: &VirtualMachine) -> PyResult<()> {
+            for elem in iter.iter(vm)? {
+                self.appendleft(elem?);
+            }
+            Ok(())
+        }
+
+        #[pymethod]
+        fn index(
+            &self,
+            obj: PyObjectRef,
+            start: OptionalArg<usize>,
+            stop: OptionalArg<usize>,
+            vm: &VirtualMachine,
+        ) -> PyResult<usize> {
+            let deque = self.deque.borrow();
+            let start = start.unwrap_or(0);
+            let stop = stop.unwrap_or_else(|| deque.len());
+            for (i, elem) in deque.iter().skip(start).take(stop - start).enumerate() {
+                if vm.identical_or_equal(elem, &obj)? {
+                    return Ok(i);
+                }
+            }
+            Err(vm.new_value_error(
+                vm.to_repr(&obj)
+                    .map(|repr| format!("{} is not in deque", repr))
+                    .unwrap_or_else(|_| String::new()),
+            ))
+        }
+
+        #[pymethod]
+        fn insert(&self, idx: i32, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
+            let mut deque = self.deque.borrow_mut();
+
+            if self.maxlen.get() == Some(deque.len()) {
+                return Err(vm.new_index_error("deque already at its maximum size".to_owned()));
+            }
+
+            let idx = if idx < 0 {
+                if -idx as usize > deque.len() {
+                    0
+                } else {
+                    deque.len() - ((-idx) as usize)
+                }
+            } else if idx as usize >= deque.len() {
+                deque.len() - 1
             } else {
-                deque.len() - ((-idx) as usize)
+                idx as usize
+            };
+
+            deque.insert(idx, obj);
+
+            Ok(())
+        }
+
+        #[pymethod]
+        fn pop(&self, vm: &VirtualMachine) -> PyResult {
+            self.deque
+                .borrow_mut()
+                .pop_back()
+                .ok_or_else(|| vm.new_index_error("pop from an empty deque".to_owned()))
+        }
+
+        #[pymethod]
+        fn popleft(&self, vm: &VirtualMachine) -> PyResult {
+            self.deque
+                .borrow_mut()
+                .pop_front()
+                .ok_or_else(|| vm.new_index_error("pop from an empty deque".to_owned()))
+        }
+
+        #[pymethod]
+        fn remove(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
+            let mut deque = self.deque.borrow_mut();
+            let mut idx = None;
+            for (i, elem) in deque.iter().enumerate() {
+                if vm.identical_or_equal(elem, &obj)? {
+                    idx = Some(i);
+                    break;
+                }
             }
-        } else if idx as usize >= deque.len() {
-            deque.len() - 1
-        } else {
-            idx as usize
-        };
+            idx.map(|idx| deque.remove(idx).unwrap())
+                .ok_or_else(|| vm.new_value_error("deque.remove(x): x not in deque".to_owned()))
+        }
 
-        deque.insert(idx, obj);
+        #[pymethod]
+        fn reverse(&self) {
+            self.deque
+                .replace_with(|deque| deque.iter().cloned().rev().collect());
+        }
 
-        Ok(())
-    }
-
-    #[pymethod]
-    fn pop(&self, vm: &VirtualMachine) -> PyResult {
-        self.deque
-            .borrow_mut()
-            .pop_back()
-            .ok_or_else(|| vm.new_index_error("pop from an empty deque".to_owned()))
-    }
-
-    #[pymethod]
-    fn popleft(&self, vm: &VirtualMachine) -> PyResult {
-        self.deque
-            .borrow_mut()
-            .pop_front()
-            .ok_or_else(|| vm.new_index_error("pop from an empty deque".to_owned()))
-    }
-
-    #[pymethod]
-    fn remove(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
-        let mut deque = self.deque.borrow_mut();
-        let mut idx = None;
-        for (i, elem) in deque.iter().enumerate() {
-            if vm.identical_or_equal(elem, &obj)? {
-                idx = Some(i);
-                break;
+        #[pymethod]
+        fn rotate(&self, mid: OptionalArg<isize>) {
+            let mut deque = self.deque.borrow_mut();
+            let mid = mid.unwrap_or(1);
+            if mid < 0 {
+                deque.rotate_left(-mid as usize);
+            } else {
+                deque.rotate_right(mid as usize);
             }
         }
-        idx.map(|idx| deque.remove(idx).unwrap())
-            .ok_or_else(|| vm.new_value_error("deque.remove(x): x not in deque".to_owned()))
-    }
 
-    #[pymethod]
-    fn reverse(&self) {
-        self.deque
-            .replace_with(|deque| deque.iter().cloned().rev().collect());
-    }
+        #[pyproperty]
+        fn maxlen(&self) -> Option<usize> {
+            self.maxlen.get()
+        }
 
-    #[pymethod]
-    fn rotate(&self, mid: OptionalArg<isize>) {
-        let mut deque = self.deque.borrow_mut();
-        let mid = mid.unwrap_or(1);
-        if mid < 0 {
-            deque.rotate_left(-mid as usize);
-        } else {
-            deque.rotate_right(mid as usize);
+        #[pyproperty(setter)]
+        fn set_maxlen(&self, maxlen: Option<usize>) {
+            self.maxlen.set(maxlen);
+        }
+
+        #[pymethod(name = "__repr__")]
+        fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
+            let repr = if let Some(_guard) = ReprGuard::enter(zelf.as_object()) {
+                let elements = zelf
+                    .deque
+                    .borrow()
+                    .iter()
+                    .map(|obj| vm.to_repr(obj))
+                    .collect::<Result<Vec<_>, _>>()?;
+                let maxlen = zelf
+                    .maxlen
+                    .get()
+                    .map(|maxlen| format!(", maxlen={}", maxlen))
+                    .unwrap_or_default();
+                format!("deque([{}]{})", elements.into_iter().format(", "), maxlen)
+            } else {
+                "[...]".to_owned()
+            };
+            Ok(repr)
+        }
+
+        #[inline]
+        fn cmp<F>(
+            &self,
+            other: PyObjectRef,
+            op: F,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue>
+        where
+            F: Fn(&VecDeque<PyObjectRef>, &VecDeque<PyObjectRef>) -> PyResult<bool>,
+        {
+            let r = if let Some(other) = other.payload_if_subclass::<PyDeque>(vm) {
+                Implemented(op(&*self.borrow_deque(), &*other.borrow_deque())?)
+            } else {
+                NotImplemented
+            };
+            Ok(r)
+        }
+
+        #[pymethod(name = "__eq__")]
+        fn eq(
+            zelf: PyRef<Self>,
+            other: PyObjectRef,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue> {
+            if zelf.as_object().is(&other) {
+                Ok(Implemented(true))
+            } else {
+                zelf.cmp(other, |a, b| sequence::eq(vm, a, b), vm)
+            }
+        }
+
+        #[pymethod(name = "__ne__")]
+        fn ne(
+            zelf: PyRef<Self>,
+            other: PyObjectRef,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue> {
+            Ok(PyDeque::eq(zelf, other, vm)?.map(|v| !v))
+        }
+
+        #[pymethod(name = "__lt__")]
+        fn lt(
+            zelf: PyRef<Self>,
+            other: PyObjectRef,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue> {
+            if zelf.as_object().is(&other) {
+                Ok(Implemented(false))
+            } else {
+                zelf.cmp(other, |a, b| sequence::lt(vm, a, b), vm)
+            }
+        }
+
+        #[pymethod(name = "__gt__")]
+        fn gt(
+            zelf: PyRef<Self>,
+            other: PyObjectRef,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue> {
+            if zelf.as_object().is(&other) {
+                Ok(Implemented(false))
+            } else {
+                zelf.cmp(other, |a, b| sequence::gt(vm, a, b), vm)
+            }
+        }
+
+        #[pymethod(name = "__le__")]
+        fn le(
+            zelf: PyRef<Self>,
+            other: PyObjectRef,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue> {
+            if zelf.as_object().is(&other) {
+                Ok(Implemented(true))
+            } else {
+                zelf.cmp(other, |a, b| sequence::le(vm, a, b), vm)
+            }
+        }
+
+        #[pymethod(name = "__ge__")]
+        fn ge(
+            zelf: PyRef<Self>,
+            other: PyObjectRef,
+            vm: &VirtualMachine,
+        ) -> PyResult<PyComparisonValue> {
+            if zelf.as_object().is(&other) {
+                Ok(Implemented(true))
+            } else {
+                zelf.cmp(other, |a, b| sequence::ge(vm, a, b), vm)
+            }
+        }
+
+        #[pymethod(name = "__mul__")]
+        fn mul(&self, n: isize) -> Self {
+            let deque: &VecDeque<_> = &self.deque.borrow();
+            let mul = sequence::seq_mul(deque, n);
+            let skipped = if let Some(maxlen) = self.maxlen.get() {
+                mul.len() - maxlen
+            } else {
+                0
+            };
+            let deque = mul.skip(skipped).cloned().collect();
+            PyDeque {
+                deque: RefCell::new(deque),
+                maxlen: self.maxlen.clone(),
+            }
+        }
+
+        #[pymethod(name = "__len__")]
+        fn len(&self) -> usize {
+            self.deque.borrow().len()
+        }
+
+        #[pymethod(name = "__iter__")]
+        fn iter(zelf: PyRef<Self>) -> PyDequeIterator {
+            PyDequeIterator {
+                position: Cell::new(0),
+                deque: zelf,
+            }
         }
     }
 
-    #[pyproperty]
-    fn maxlen(&self) -> Option<usize> {
-        self.maxlen.get()
+    #[pyclass(name = "_deque_iterator")]
+    #[derive(Debug)]
+    struct PyDequeIterator {
+        position: Cell<usize>,
+        deque: PyDequeRef,
     }
 
-    #[pyproperty(setter)]
-    fn set_maxlen(&self, maxlen: Option<usize>) {
-        self.maxlen.set(maxlen);
-    }
-
-    #[pymethod(name = "__repr__")]
-    fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<String> {
-        let repr = if let Some(_guard) = ReprGuard::enter(zelf.as_object()) {
-            let elements = zelf
-                .deque
-                .borrow()
-                .iter()
-                .map(|obj| vm.to_repr(obj))
-                .collect::<Result<Vec<_>, _>>()?;
-            let maxlen = zelf
-                .maxlen
-                .get()
-                .map(|maxlen| format!(", maxlen={}", maxlen))
-                .unwrap_or_default();
-            format!("deque([{}]{})", elements.into_iter().format(", "), maxlen)
-        } else {
-            "[...]".to_owned()
-        };
-        Ok(repr)
-    }
-
-    #[inline]
-    fn cmp<F>(&self, other: PyObjectRef, op: F, vm: &VirtualMachine) -> PyResult<PyComparisonValue>
-    where
-        F: Fn(&VecDeque<PyObjectRef>, &VecDeque<PyObjectRef>) -> PyResult<bool>,
-    {
-        let r = if let Some(other) = other.payload_if_subclass::<PyDeque>(vm) {
-            Implemented(op(&*self.borrow_deque(), &*other.borrow_deque())?)
-        } else {
-            NotImplemented
-        };
-        Ok(r)
-    }
-
-    #[pymethod(name = "__eq__")]
-    fn eq(
-        zelf: PyRef<Self>,
-        other: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyComparisonValue> {
-        if zelf.as_object().is(&other) {
-            Ok(Implemented(true))
-        } else {
-            zelf.cmp(other, |a, b| sequence::eq(vm, a, b), vm)
+    impl PyValue for PyDequeIterator {
+        fn class(vm: &VirtualMachine) -> PyClassRef {
+            vm.class("_collections", "_deque_iterator")
         }
     }
 
-    #[pymethod(name = "__ne__")]
-    fn ne(
-        zelf: PyRef<Self>,
-        other: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyComparisonValue> {
-        Ok(PyDeque::eq(zelf, other, vm)?.map(|v| !v))
-    }
-
-    #[pymethod(name = "__lt__")]
-    fn lt(
-        zelf: PyRef<Self>,
-        other: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyComparisonValue> {
-        if zelf.as_object().is(&other) {
-            Ok(Implemented(false))
-        } else {
-            zelf.cmp(other, |a, b| sequence::lt(vm, a, b), vm)
+    #[pyimpl]
+    impl PyDequeIterator {
+        #[pymethod(name = "__next__")]
+        fn next(&self, vm: &VirtualMachine) -> PyResult {
+            if self.position.get() < self.deque.deque.borrow().len() {
+                let ret = self.deque.deque.borrow()[self.position.get()].clone();
+                self.position.set(self.position.get() + 1);
+                Ok(ret)
+            } else {
+                Err(objiter::new_stop_iteration(vm))
+            }
         }
-    }
 
-    #[pymethod(name = "__gt__")]
-    fn gt(
-        zelf: PyRef<Self>,
-        other: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyComparisonValue> {
-        if zelf.as_object().is(&other) {
-            Ok(Implemented(false))
-        } else {
-            zelf.cmp(other, |a, b| sequence::gt(vm, a, b), vm)
-        }
-    }
-
-    #[pymethod(name = "__le__")]
-    fn le(
-        zelf: PyRef<Self>,
-        other: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyComparisonValue> {
-        if zelf.as_object().is(&other) {
-            Ok(Implemented(true))
-        } else {
-            zelf.cmp(other, |a, b| sequence::le(vm, a, b), vm)
-        }
-    }
-
-    #[pymethod(name = "__ge__")]
-    fn ge(
-        zelf: PyRef<Self>,
-        other: PyObjectRef,
-        vm: &VirtualMachine,
-    ) -> PyResult<PyComparisonValue> {
-        if zelf.as_object().is(&other) {
-            Ok(Implemented(true))
-        } else {
-            zelf.cmp(other, |a, b| sequence::ge(vm, a, b), vm)
-        }
-    }
-
-    #[pymethod(name = "__mul__")]
-    fn mul(&self, n: isize) -> Self {
-        let deque: &VecDeque<_> = &self.deque.borrow();
-        let mul = sequence::seq_mul(deque, n);
-        let skipped = if let Some(maxlen) = self.maxlen.get() {
-            mul.len() - maxlen
-        } else {
-            0
-        };
-        let deque = mul.skip(skipped).cloned().collect();
-        PyDeque {
-            deque: RefCell::new(deque),
-            maxlen: self.maxlen.clone(),
-        }
-    }
-
-    #[pymethod(name = "__len__")]
-    fn len(&self) -> usize {
-        self.deque.borrow().len()
-    }
-
-    #[pymethod(name = "__iter__")]
-    fn iter(zelf: PyRef<Self>) -> PyDequeIterator {
-        PyDequeIterator {
-            position: Cell::new(0),
-            deque: zelf,
+        #[pymethod(name = "__iter__")]
+        fn iter(zelf: PyRef<Self>) -> PyRef<Self> {
+            zelf
         }
     }
 }
-
-#[pyclass(name = "_deque_iterator")]
-#[derive(Debug)]
-struct PyDequeIterator {
-    position: Cell<usize>,
-    deque: PyDequeRef,
-}
-
-impl PyValue for PyDequeIterator {
-    fn class(vm: &VirtualMachine) -> PyClassRef {
-        vm.class("_collections", "_deque_iterator")
-    }
-}
-
-#[pyimpl]
-impl PyDequeIterator {
-    #[pymethod(name = "__next__")]
-    fn next(&self, vm: &VirtualMachine) -> PyResult {
-        if self.position.get() < self.deque.deque.borrow().len() {
-            let ret = self.deque.deque.borrow()[self.position.get()].clone();
-            self.position.set(self.position.get() + 1);
-            Ok(ret)
-        } else {
-            Err(objiter::new_stop_iteration(vm))
-        }
-    }
-
-    #[pymethod(name = "__iter__")]
-    fn iter(zelf: PyRef<Self>) -> PyRef<Self> {
-        zelf
-    }
-}
-
-pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
-    py_module!(vm, "_collections", {
-        "deque" => PyDeque::make_class(&vm.ctx),
-        "_deque_iterator" => PyDequeIterator::make_class(&vm.ctx),
-    })
-}

vm/src/stdlib/imp.rs

@@ -86,7 +86,7 @@ fn imp_fix_co_filename(_code: PyObjectRef, _path: PyStringRef) {
 
 pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
     let ctx = &vm.ctx;
-    let module = py_module!(vm, "_imp", {
+    py_module!(vm, "_imp", {
         "extension_suffixes" => ctx.new_function(imp_extension_suffixes),
         "acquire_lock" => ctx.new_function(imp_acquire_lock),
         "release_lock" => ctx.new_function(imp_release_lock),
@@ -99,7 +99,5 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
         "init_frozen" => ctx.new_function(imp_init_frozen),
         "is_frozen_package" => ctx.new_function(imp_is_frozen_package),
         "_fix_co_filename" => ctx.new_function(imp_fix_co_filename),
-    });
-
-    module
+    })
 }

vm/src/stdlib/io.rs

@@ -911,7 +911,7 @@ pub fn io_open(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
         _ => unimplemented!("'a' mode is not yet implemented"),
     };
 
-    let io_obj = match typ.chars().next().unwrap() {
+    match typ.chars().next().unwrap() {
         // If the mode is text this buffer type is consumed on construction of
         // a TextIOWrapper which is subsequently returned.
         't' => {
@@ -923,8 +923,7 @@ pub fn io_open(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult {
         // For Buffered class construct "raw" IO class e.g. FileIO and pass this into corresponding field
         'b' => buffered,
         _ => unreachable!(),
-    };
-    io_obj
+    }
 }
 
 pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {

vm/src/stdlib/os.rs

@@ -1,4 +1,3 @@
-use num_cpus;
 use std::cell::{Cell, RefCell};
 use std::ffi;
 use std::fs::File;
@@ -13,8 +12,6 @@ use std::{env, fs};
 
 use bitflags::bitflags;
 #[cfg(unix)]
-use exitcode;
-#[cfg(unix)]
 use nix::errno::Errno;
 #[cfg(all(unix, not(target_os = "redox")))]
 use nix::pty::openpty;
@@ -24,8 +21,6 @@ use nix::unistd::{self, Gid, Pid, Uid};
 use std::os::unix::io::RawFd;
 #[cfg(windows)]
 use std::os::windows::io::RawHandle;
-#[cfg(unix)]
-use uname;
 
 use super::errno::errors;
 use crate::exceptions::PyBaseExceptionRef;

vm/src/stdlib/platform.rs

@@ -1,39 +1,37 @@
-use crate::pyobject::PyObjectRef;
-use crate::version;
-use crate::vm::VirtualMachine;
+pub(crate) use decl::make_module;
 
-pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
-    let ctx = &vm.ctx;
-    py_module!(vm, "platform", {
-        "python_branch" => ctx.new_function(platform_python_branch),
-        "python_build" => ctx.new_function(platform_python_build),
-        "python_compiler" => ctx.new_function(platform_python_compiler),
-        "python_implementation" => ctx.new_function(platform_python_implementation),
-        "python_revision" => ctx.new_function(platform_python_revision),
-        "python_version" => ctx.new_function(platform_python_version),
-    })
-}
+#[pymodule(name = "platform")]
+mod decl {
+    use crate::version;
+    use crate::vm::VirtualMachine;
 
-fn platform_python_implementation(_vm: &VirtualMachine) -> String {
-    "RustPython".to_owned()
-}
+    #[pyfunction]
+    fn python_implementation(_vm: &VirtualMachine) -> String {
+        "RustPython".to_owned()
+    }
 
-fn platform_python_version(_vm: &VirtualMachine) -> String {
-    version::get_version_number()
-}
+    #[pyfunction]
+    fn python_version(_vm: &VirtualMachine) -> String {
+        version::get_version_number()
+    }
 
-fn platform_python_compiler(_vm: &VirtualMachine) -> String {
-    version::get_compiler()
-}
+    #[pyfunction]
+    fn python_compiler(_vm: &VirtualMachine) -> String {
+        version::get_compiler()
+    }
 
-fn platform_python_build(_vm: &VirtualMachine) -> (String, String) {
-    (version::get_git_identifier(), version::get_git_datetime())
-}
+    #[pyfunction]
+    fn python_build(_vm: &VirtualMachine) -> (String, String) {
+        (version::get_git_identifier(), version::get_git_datetime())
+    }
 
-fn platform_python_branch(_vm: &VirtualMachine) -> String {
-    version::get_git_branch()
-}
+    #[pyfunction]
+    fn python_branch(_vm: &VirtualMachine) -> String {
+        version::get_git_branch()
+    }
 
-fn platform_python_revision(_vm: &VirtualMachine) -> String {
-    version::get_git_revision()
+    #[pyfunction]
+    fn python_revision(_vm: &VirtualMachine) -> String {
+        version::get_git_revision()
+    }
 }

vm/src/stdlib/random.rs

@@ -1,127 +1,123 @@
 //! Random module.
 
-use std::cell::RefCell;
+pub(crate) use _random::make_module;
 
-use num_bigint::{BigInt, Sign};
-use num_traits::Signed;
-use rand::RngCore;
+#[pymodule]
+mod _random {
+    use crate::function::OptionalOption;
+    use crate::obj::objint::PyIntRef;
+    use crate::obj::objtype::PyClassRef;
+    use crate::pyobject::{PyClassImpl, PyRef, PyResult, PyValue};
+    use crate::VirtualMachine;
+    use num_bigint::{BigInt, Sign};
+    use num_traits::Signed;
+    use rand::RngCore;
+    use std::cell::RefCell;
 
-use crate::function::OptionalOption;
-use crate::obj::objint::PyIntRef;
-use crate::obj::objtype::PyClassRef;
-use crate::pyobject::{PyClassImpl, PyObjectRef, PyRef, PyResult, PyValue};
-use crate::VirtualMachine;
-
-#[derive(Debug)]
-enum PyRng {
-    Std(rand::rngs::ThreadRng),
-    MT(Box<mt19937::MT19937>),
-}
-
-impl Default for PyRng {
-    fn default() -> Self {
-        PyRng::Std(rand::thread_rng())
+    #[derive(Debug)]
+    enum PyRng {
+        Std(rand::rngs::ThreadRng),
+        MT(Box<mt19937::MT19937>),
     }
-}
 
-impl RngCore for PyRng {
-    fn next_u32(&mut self) -> u32 {
-        match self {
-            Self::Std(s) => s.next_u32(),
-            Self::MT(m) => m.next_u32(),
+    impl Default for PyRng {
+        fn default() -> Self {
+            PyRng::Std(rand::thread_rng())
         }
     }
-    fn next_u64(&mut self) -> u64 {
-        match self {
-            Self::Std(s) => s.next_u64(),
-            Self::MT(m) => m.next_u64(),
-        }
-    }
-    fn fill_bytes(&mut self, dest: &mut [u8]) {
-        match self {
-            Self::Std(s) => s.fill_bytes(dest),
-            Self::MT(m) => m.fill_bytes(dest),
-        }
-    }
-    fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand::Error> {
-        match self {
-            Self::Std(s) => s.try_fill_bytes(dest),
-            Self::MT(m) => m.try_fill_bytes(dest),
-        }
-    }
-}
 
-#[pyclass(name = "Random")]
-#[derive(Debug)]
-struct PyRandom {
-    rng: RefCell<PyRng>,
-}
-
-impl PyValue for PyRandom {
-    fn class(vm: &VirtualMachine) -> PyClassRef {
-        vm.class("_random", "Random")
-    }
-}
-
-#[pyimpl(flags(BASETYPE))]
-impl PyRandom {
-    #[pyslot(new)]
-    fn new(cls: PyClassRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
-        PyRandom {
-            rng: RefCell::new(PyRng::default()),
-        }
-        .into_ref_with_type(vm, cls)
-    }
-
-    #[pymethod]
-    fn random(&self) -> f64 {
-        mt19937::gen_res53(&mut *self.rng.borrow_mut())
-    }
-
-    #[pymethod]
-    fn seed(&self, n: OptionalOption<PyIntRef>) {
-        let new_rng = match n.flat_option() {
-            None => PyRng::default(),
-            Some(n) => {
-                let (_, mut key) = n.as_bigint().abs().to_u32_digits();
-                if cfg!(target_endian = "big") {
-                    key.reverse();
-                }
-                PyRng::MT(Box::new(mt19937::MT19937::new_with_slice_seed(&key)))
+    impl RngCore for PyRng {
+        fn next_u32(&mut self) -> u32 {
+            match self {
+                Self::Std(s) => s.next_u32(),
+                Self::MT(m) => m.next_u32(),
             }
-        };
-
-        *self.rng.borrow_mut() = new_rng;
+        }
+        fn next_u64(&mut self) -> u64 {
+            match self {
+                Self::Std(s) => s.next_u64(),
+                Self::MT(m) => m.next_u64(),
+            }
+        }
+        fn fill_bytes(&mut self, dest: &mut [u8]) {
+            match self {
+                Self::Std(s) => s.fill_bytes(dest),
+                Self::MT(m) => m.fill_bytes(dest),
+            }
+        }
+        fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand::Error> {
+            match self {
+                Self::Std(s) => s.try_fill_bytes(dest),
+                Self::MT(m) => m.try_fill_bytes(dest),
+            }
+        }
     }
 
-    #[pymethod]
-    fn getrandbits(&self, mut k: usize) -> BigInt {
-        let mut rng = self.rng.borrow_mut();
+    #[pyclass(name = "Random")]
+    #[derive(Debug)]
+    struct PyRandom {
+        rng: RefCell<PyRng>,
+    }
 
-        let mut gen_u32 = |k| rng.next_u32() >> (32 - k) as u32;
+    impl PyValue for PyRandom {
+        fn class(vm: &VirtualMachine) -> PyClassRef {
+            vm.class("_random", "Random")
+        }
+    }
 
-        if k <= 32 {
-            return gen_u32(k).into();
+    #[pyimpl(flags(BASETYPE))]
+    impl PyRandom {
+        #[pyslot(new)]
+        fn new(cls: PyClassRef, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
+            PyRandom {
+                rng: RefCell::new(PyRng::default()),
+            }
+            .into_ref_with_type(vm, cls)
         }
 
-        let words = (k - 1) / 8 + 1;
-        let mut wordarray = vec![0u32; words];
-
-        let it = wordarray.iter_mut();
-        #[cfg(target_endian = "big")]
-        let it = it.rev();
-        for word in it {
-            *word = gen_u32(k);
-            k -= 32;
+        #[pymethod]
+        fn random(&self) -> f64 {
+            mt19937::gen_res53(&mut *self.rng.borrow_mut())
         }
 
-        BigInt::from_slice(Sign::NoSign, &wordarray)
+        #[pymethod]
+        fn seed(&self, n: OptionalOption<PyIntRef>) {
+            let new_rng = match n.flat_option() {
+                None => PyRng::default(),
+                Some(n) => {
+                    let (_, mut key) = n.as_bigint().abs().to_u32_digits();
+                    if cfg!(target_endian = "big") {
+                        key.reverse();
+                    }
+                    PyRng::MT(Box::new(mt19937::MT19937::new_with_slice_seed(&key)))
+                }
+            };
+
+            *self.rng.borrow_mut() = new_rng;
+        }
+
+        #[pymethod]
+        fn getrandbits(&self, mut k: usize) -> BigInt {
+            let mut rng = self.rng.borrow_mut();
+
+            let mut gen_u32 = |k| rng.next_u32() >> (32 - k) as u32;
+
+            if k <= 32 {
+                return gen_u32(k).into();
+            }
+
+            let words = (k - 1) / 8 + 1;
+            let mut wordarray = vec![0u32; words];
+
+            let it = wordarray.iter_mut();
+            #[cfg(target_endian = "big")]
+            let it = it.rev();
+            for word in it {
+                *word = gen_u32(k);
+                k -= 32;
+            }
+
+            BigInt::from_slice(Sign::NoSign, &wordarray)
+        }
     }
 }
-
-pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
-    let ctx = &vm.ctx;
-    py_module!(vm, "_random", {
-        "Random" => PyRandom::make_class(ctx),
-    })
-}

vm/src/stdlib/signal.rs

@@ -8,7 +8,6 @@ use arr_macro::arr;
 #[cfg(unix)]
 use nix::unistd::alarm as sig_alarm;
 
-use libc;
 #[cfg(not(windows))]
 use libc::{SIG_DFL, SIG_ERR, SIG_IGN};
 

vm/src/stdlib/subprocess.rs

@@ -4,8 +4,6 @@ use std::fs::File;
 use std::io::ErrorKind;
 use std::time::Duration;
 
-use subprocess;
-
 use crate::function::OptionalArg;
 use crate::obj::objbytes::PyBytesRef;
 use crate::obj::objlist::PyListRef;

vm/src/stdlib/warnings.rs

@@ -34,9 +34,7 @@ fn warnings_warn(args: WarnArgs, vm: &VirtualMachine) -> PyResult<()> {
 
 pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
     let ctx = &vm.ctx;
-    let module = py_module!(vm, "_warnings", {
+    py_module!(vm, "_warnings", {
          "warn" => ctx.new_function(warnings_warn),
-    });
-
-    module
+    })
 }

vm/src/version.rs

@@ -102,9 +102,8 @@ fn get_git_timestamp_datetime() -> DateTime<Local> {
     let timestamp = timestamp.parse::<u64>().unwrap_or(0);
 
     let datetime = UNIX_EPOCH + Duration::from_secs(timestamp);
-    let datetime = DateTime::<Local>::from(datetime);
 
-    datetime
+    datetime.into()
 }
 
 pub fn get_git_date() -> String {

vm/src/vm.rs

@@ -791,8 +791,7 @@ impl VirtualMachine {
             self.trace_event(TraceEvent::Return)?;
             result
         } else if class.has_attr("__call__") {
-            let result = self.call_method(&callable, "__call__", args);
-            result
+            self.call_method(&callable, "__call__", args)
         } else {
             Err(self.new_type_error(format!(
                 "'{}' object is not callable",
@@ -806,8 +805,7 @@ impl VirtualMachine {
     where
         T: Into<PyFuncArgs>,
     {
-        let res = self._invoke(func_ref, args.into());
-        res
+        self._invoke(func_ref, args.into())
     }
 
     /// Call registered trace function.

wasm/lib/src/convert.rs

@@ -1,5 +1,4 @@
 use js_sys::{Array, ArrayBuffer, Object, Promise, Reflect, SyntaxError, Uint8Array};
-use serde_wasm_bindgen;
 use wasm_bindgen::{closure::Closure, prelude::*, JsCast};
 
 use rustpython_compiler::error::{CompileError, CompileErrorType};