Merge pull request RustPython#2518 from RustPython/get-method-opt

Add optimized method lookup to avoid creating intermediate method objects

Author: coolreader18

Lib/test/test_fileio.py

@@ -427,6 +427,7 @@ class PyAutoFileTests(AutoFileTests, unittest.TestCase):
 
 class OtherFileTests:
 
+    @unittest.skip("TODO: non-deterministic failures, FileIO.seekable()?")
     def testAbles(self):
         try:
             f = self.FileIO(TESTFN, "w")
@@ -673,12 +674,6 @@ class PyOtherFileTests(OtherFileTests, unittest.TestCase):
     FileIO = _pyio.FileIO
     modulename = '_pyio'
 
-    # TODO: RUSTPYTHON
-    if sys.platform == "win32":
-        @unittest.expectedFailure
-        def testAbles(self):
-            super().testAbles()
-
     # TODO: RUSTPYTHON
     if sys.platform == "win32":
         @unittest.expectedFailure

Lib/test/test_with.py

@@ -109,8 +109,6 @@ def fooNotDeclared():
             with foo: pass
         self.assertRaises(NameError, fooNotDeclared)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def testEnterAttributeError1(self):
         class LacksEnter(object):
             def __exit__(self, type, value, traceback):

bytecode/src/lib.rs

@@ -247,9 +247,7 @@ pub enum Instruction {
     Continue {
         target: Label,
     },
-    Break {
-        target: Label,
-    },
+    Break,
     Jump {
         target: Label,
     },
@@ -281,14 +279,28 @@ pub enum Instruction {
     CallFunctionEx {
         has_kwargs: bool,
     },
+    LoadMethod {
+        idx: NameIdx,
+    },
+    CallMethodPositional {
+        nargs: u32,
+    },
+    CallMethodKeyword {
+        nargs: u32,
+    },
+    CallMethodEx {
+        has_kwargs: bool,
+    },
     ForIter {
         target: Label,
     },
     ReturnValue,
     YieldValue,
     YieldFrom,
     SetupAnnotation,
-    SetupLoop,
+    SetupLoop {
+        break_target: Label,
+    },
 
     /// Setup a finally handler, which will be called whenever one of this events occurs:
     /// - the block is popped
@@ -824,7 +836,7 @@ impl Instruction {
             | SetupExcept { handler: l }
             | SetupWith { end: l }
             | SetupAsyncWith { end: l }
-            | Break { target: l }
+            | SetupLoop { break_target: l }
             | Continue { target: l } => Some(l),
             _ => None,
         }
@@ -844,7 +856,7 @@ impl Instruction {
             | SetupExcept { handler: l }
             | SetupWith { end: l }
             | SetupAsyncWith { end: l }
-            | Break { target: l }
+            | SetupLoop { break_target: l }
             | Continue { target: l } => Some(l),
             _ => None,
         }
@@ -853,7 +865,7 @@ impl Instruction {
     pub fn unconditional_branch(&self) -> bool {
         matches!(
             self,
-            Jump { .. } | Continue { .. } | Break { .. } | ReturnValue | Raise { .. }
+            Jump { .. } | Continue { .. } | Break | ReturnValue | Raise { .. }
         )
     }
 
@@ -880,7 +892,7 @@ impl Instruction {
             Duplicate => 1,
             GetIter => 0,
             Continue { .. } => 0,
-            Break { .. } => 0,
+            Break => 0,
             Jump { .. } => 0,
             JumpIfTrue { .. } | JumpIfFalse { .. } => -1,
             JumpIfTrueOrPop { .. } | JumpIfFalseOrPop { .. } => {
@@ -897,9 +909,13 @@ impl Instruction {
                     - flags.contains(MakeFunctionFlags::DEFAULTS) as i32
                     + 1
             }
-            CallFunctionPositional { nargs } => -1 - (*nargs as i32) + 1,
-            CallFunctionKeyword { nargs } => -2 - (*nargs as i32) + 1,
-            CallFunctionEx { has_kwargs } => -2 - *has_kwargs as i32 + 1,
+            CallFunctionPositional { nargs } => -(*nargs as i32) - 1 + 1,
+            CallMethodPositional { nargs } => -(*nargs as i32) - 3 + 1,
+            CallFunctionKeyword { nargs } => -1 - (*nargs as i32) - 1 + 1,
+            CallMethodKeyword { nargs } => -1 - (*nargs as i32) - 3 + 1,
+            CallFunctionEx { has_kwargs } => -1 - (*has_kwargs as i32) - 1 + 1,
+            CallMethodEx { has_kwargs } => -1 - (*has_kwargs as i32) - 3 + 1,
+            LoadMethod { .. } => -1 + 3,
             ForIter { .. } => {
                 if jump {
                     -1
@@ -910,7 +926,11 @@ impl Instruction {
             ReturnValue => -1,
             YieldValue => 0,
             YieldFrom => -1,
-            SetupAnnotation | SetupLoop | SetupFinally { .. } | EnterFinally | EndFinally => 0,
+            SetupAnnotation
+            | SetupLoop { .. }
+            | SetupFinally { .. }
+            | EnterFinally
+            | EndFinally => 0,
             SetupExcept { .. } => {
                 if jump {
                     1
@@ -1056,7 +1076,7 @@ impl Instruction {
             Duplicate => w!(Duplicate),
             GetIter => w!(GetIter),
             Continue { target } => w!(Continue, target),
-            Break { target } => w!(Break, target),
+            Break => w!(Break),
             Jump { target } => w!(Jump, target),
             JumpIfTrue { target } => w!(JumpIfTrue, target),
             JumpIfFalse { target } => w!(JumpIfFalse, target),
@@ -1066,12 +1086,16 @@ impl Instruction {
             CallFunctionPositional { nargs } => w!(CallFunctionPositional, nargs),
             CallFunctionKeyword { nargs } => w!(CallFunctionKeyword, nargs),
             CallFunctionEx { has_kwargs } => w!(CallFunctionEx, has_kwargs),
+            LoadMethod { idx } => w!(LoadMethod, name(*idx)),
+            CallMethodPositional { nargs } => w!(CallMethodPositional, nargs),
+            CallMethodKeyword { nargs } => w!(CallMethodKeyword, nargs),
+            CallMethodEx { has_kwargs } => w!(CallMethodEx, has_kwargs),
             ForIter { target } => w!(ForIter, target),
             ReturnValue => w!(ReturnValue),
             YieldValue => w!(YieldValue),
             YieldFrom => w!(YieldFrom),
             SetupAnnotation => w!(SetupAnnotation),
-            SetupLoop => w!(SetupLoop),
+            SetupLoop { break_target } => w!(SetupLoop, break_target),
             SetupExcept { handler } => w!(SetupExcept, handler),
             SetupFinally { handler } => w!(SetupFinally, handler),
             EnterFinally => w!(EnterFinally),

compiler/src/compile.rs

@@ -28,6 +28,28 @@ enum NameUsage {
     Delete,
 }
 
+enum CallType {
+    Positional { nargs: u32 },
+    Keyword { nargs: u32 },
+    Ex { has_kwargs: bool },
+}
+impl CallType {
+    fn normal_call(self) -> Instruction {
+        match self {
+            CallType::Positional { nargs } => Instruction::CallFunctionPositional { nargs },
+            CallType::Keyword { nargs } => Instruction::CallFunctionKeyword { nargs },
+            CallType::Ex { has_kwargs } => Instruction::CallFunctionEx { has_kwargs },
+        }
+    }
+    fn method_call(self) -> Instruction {
+        match self {
+            CallType::Positional { nargs } => Instruction::CallMethodPositional { nargs },
+            CallType::Keyword { nargs } => Instruction::CallMethodKeyword { nargs },
+            CallType::Ex { has_kwargs } => Instruction::CallMethodEx { has_kwargs },
+        }
+    }
+}
+
 /// Main structure holding the state of compilation.
 struct Compiler {
     code_stack: Vec<CodeInfo>,
@@ -667,14 +689,13 @@ impl Compiler {
                     self.switch_to_block(after_block);
                 }
             }
-            Break => match self.ctx.loop_data {
-                Some((_, end)) => {
-                    self.emit(Instruction::Break { target: end });
-                }
-                None => {
-                    return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location))
+            Break => {
+                if self.ctx.loop_data.is_some() {
+                    self.emit(Instruction::Break);
+                } else {
+                    return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location));
                 }
-            },
+            }
             Continue => match self.ctx.loop_data {
                 Some((start, _)) => {
                     self.emit(Instruction::Continue { target: start });
@@ -1242,7 +1263,8 @@ impl Compiler {
             value: qualified_name,
         });
 
-        self.compile_call(2, bases, keywords)?;
+        let call = self.compile_call_inner(2, bases, keywords)?;
+        self.emit(call.normal_call());
 
         self.apply_decorators(decorator_list);
 
@@ -1271,7 +1293,9 @@ impl Compiler {
         let else_block = self.new_block();
         let after_block = self.new_block();
 
-        self.emit(Instruction::SetupLoop);
+        self.emit(Instruction::SetupLoop {
+            break_target: after_block,
+        });
         self.switch_to_block(while_block);
 
         self.compile_jump_if(test, false, else_block)?;
@@ -1360,7 +1384,9 @@ impl Compiler {
         let else_block = self.new_block();
         let after_block = self.new_block();
 
-        self.emit(Instruction::SetupLoop);
+        self.emit(Instruction::SetupLoop {
+            break_target: after_block,
+        });
 
         // The thing iterated:
         self.compile_expression(iter)?;
@@ -1794,10 +1820,7 @@ impl Compiler {
                 func,
                 args,
                 keywords,
-            } => {
-                self.compile_expression(func)?;
-                self.compile_call(0, args, keywords)?;
-            }
+            } => self.compile_call(func, args, keywords)?,
             BoolOp { op, values } => self.compile_bool_op(op, values)?,
             BinOp { left, op, right } => {
                 self.compile_expression(left)?;
@@ -2103,17 +2126,41 @@ impl Compiler {
 
     fn compile_call(
         &mut self,
-        additional_positional: u32,
+        func: &ast::Expr,
         args: &[ast::Expr],
         keywords: &[ast::Keyword],
     ) -> CompileResult<()> {
+        let method = if let ast::ExprKind::Attribute { value, attr, .. } = &func.node {
+            self.compile_expression(value)?;
+            let idx = self.name(attr);
+            self.emit(Instruction::LoadMethod { idx });
+            true
+        } else {
+            self.compile_expression(func)?;
+            false
+        };
+        let call = self.compile_call_inner(0, args, keywords)?;
+        self.emit(if method {
+            call.method_call()
+        } else {
+            call.normal_call()
+        });
+        Ok(())
+    }
+
+    fn compile_call_inner(
+        &mut self,
+        additional_positional: u32,
+        args: &[ast::Expr],
+        keywords: &[ast::Keyword],
+    ) -> CompileResult<CallType> {
         let count = (args.len() + keywords.len()) as u32 + additional_positional;
 
         // Normal arguments:
         let (size, unpack) = self.gather_elements(additional_positional, args)?;
         let has_double_star = keywords.iter().any(|k| k.node.arg.is_none());
 
-        if unpack || has_double_star {
+        let call = if unpack || has_double_star {
             // Create a tuple with positional args:
             self.emit(Instruction::BuildTuple { size, unpack });
 
@@ -2122,7 +2169,7 @@ impl Compiler {
             if has_kwargs {
                 self.compile_keywords(keywords)?;
             }
-            self.emit(Instruction::CallFunctionEx { has_kwargs });
+            CallType::Ex { has_kwargs }
         } else if !keywords.is_empty() {
             let mut kwarg_names = vec![];
             for keyword in keywords {
@@ -2140,12 +2187,12 @@ impl Compiler {
             self.emit_constant(ConstantData::Tuple {
                 elements: kwarg_names,
             });
-            self.emit(Instruction::CallFunctionKeyword { nargs: count });
+            CallType::Keyword { nargs: count }
         } else {
-            self.emit(Instruction::CallFunctionPositional { nargs: count });
-        }
+            CallType::Positional { nargs: count }
+        };
 
-        Ok(())
+        Ok(call)
     }
 
     // Given a vector of expr / star expr generate code which gives either
@@ -2262,8 +2309,13 @@ impl Compiler {
                 unimplemented!("async for comprehensions");
             }
 
+            let loop_block = self.new_block();
+            let after_block = self.new_block();
+
             // Setup for loop:
-            self.emit(Instruction::SetupLoop);
+            self.emit(Instruction::SetupLoop {
+                break_target: after_block,
+            });
 
             if loop_labels.is_empty() {
                 // Load iterator onto stack (passed as first argument):
@@ -2276,8 +2328,6 @@ impl Compiler {
                 self.emit(Instruction::GetIter);
             }
 
-            let loop_block = self.new_block();
-            let after_block = self.new_block();
             loop_labels.push((loop_block, after_block));
 
             self.switch_to_block(loop_block);

compiler/src/ir.rs

@@ -167,25 +167,29 @@ impl CodeInfo {
         let mut startdepths = vec![u32::MAX; self.blocks.len()];
         startdepths[0] = 0;
         stack.push(Label(0));
+        let debug = false;
         'process_blocks: while let Some(block) = stack.pop() {
             let mut depth = startdepths[block.0 as usize];
+            if debug {
+                eprintln!("===BLOCK {}===", block.0);
+            }
             let block = &self.blocks[block.0 as usize];
             for i in &block.instructions {
                 let instr = &i.instr;
                 let effect = instr.stack_effect(false);
                 let new_depth = add_ui(depth, effect);
+                if debug {
+                    eprintln!("{:?}: {:+}, {:+} = {}", instr, effect, depth, new_depth);
+                }
                 if new_depth > maxdepth {
                     maxdepth = new_depth
                 }
-                // we don't want to worry about Continue or Break, they use unwinding to jump to
-                // their targets and as such the stack size is taken care of in frame.rs by setting
+                // we don't want to worry about Continue, it uses unwinding to jump to
+                // its targets and as such the stack size is taken care of in frame.rs by setting
                 // it back to the level it was at when SetupLoop was run
-                let jump_label = instr.label_arg().filter(|_| {
-                    !matches!(
-                        instr,
-                        Instruction::Continue { .. } | Instruction::Break { .. }
-                    )
-                });
+                let jump_label = instr
+                    .label_arg()
+                    .filter(|_| !matches!(instr, Instruction::Continue { .. }));
                 if let Some(&target_block) = jump_label {
                     let effect = instr.stack_effect(true);
                     let target_depth = add_ui(depth, effect);
@@ -215,7 +219,7 @@ fn stackdepth_push(stack: &mut Vec<Label>, startdepths: &mut [u32], target: Labe
 
 fn add_ui(a: u32, b: i32) -> u32 {
     if b < 0 {
-        a - b.abs() as u32
+        a - b.wrapping_abs() as u32
     } else {
         a + b as u32
     }