Skip to content

Commit 605a353

Browse files
authored
Merge pull request RustPython#2518 from RustPython/get-method-opt
Add optimized method lookup to avoid creating intermediate method objects
2 parents 843895c + 54ab9cb commit 605a353

25 files changed

+868
-454
lines changed

Lib/test/test_fileio.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,7 @@ class PyAutoFileTests(AutoFileTests, unittest.TestCase):
427427

428428
class OtherFileTests:
429429

430+
@unittest.skip("TODO: non-deterministic failures, FileIO.seekable()?")
430431
def testAbles(self):
431432
try:
432433
f = self.FileIO(TESTFN, "w")
@@ -673,12 +674,6 @@ class PyOtherFileTests(OtherFileTests, unittest.TestCase):
673674
FileIO = _pyio.FileIO
674675
modulename = '_pyio'
675676

676-
# TODO: RUSTPYTHON
677-
if sys.platform == "win32":
678-
@unittest.expectedFailure
679-
def testAbles(self):
680-
super().testAbles()
681-
682677
# TODO: RUSTPYTHON
683678
if sys.platform == "win32":
684679
@unittest.expectedFailure

Lib/test/test_with.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ def fooNotDeclared():
109109
with foo: pass
110110
self.assertRaises(NameError, fooNotDeclared)
111111

112-
# TODO: RUSTPYTHON
113-
@unittest.expectedFailure
114112
def testEnterAttributeError1(self):
115113
class LacksEnter(object):
116114
def __exit__(self, type, value, traceback):

bytecode/src/lib.rs

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,7 @@ pub enum Instruction {
247247
Continue {
248248
target: Label,
249249
},
250-
Break {
251-
target: Label,
252-
},
250+
Break,
253251
Jump {
254252
target: Label,
255253
},
@@ -281,14 +279,28 @@ pub enum Instruction {
281279
CallFunctionEx {
282280
has_kwargs: bool,
283281
},
282+
LoadMethod {
283+
idx: NameIdx,
284+
},
285+
CallMethodPositional {
286+
nargs: u32,
287+
},
288+
CallMethodKeyword {
289+
nargs: u32,
290+
},
291+
CallMethodEx {
292+
has_kwargs: bool,
293+
},
284294
ForIter {
285295
target: Label,
286296
},
287297
ReturnValue,
288298
YieldValue,
289299
YieldFrom,
290300
SetupAnnotation,
291-
SetupLoop,
301+
SetupLoop {
302+
break_target: Label,
303+
},
292304

293305
/// Setup a finally handler, which will be called whenever one of this events occurs:
294306
/// - the block is popped
@@ -824,7 +836,7 @@ impl Instruction {
824836
| SetupExcept { handler: l }
825837
| SetupWith { end: l }
826838
| SetupAsyncWith { end: l }
827-
| Break { target: l }
839+
| SetupLoop { break_target: l }
828840
| Continue { target: l } => Some(l),
829841
_ => None,
830842
}
@@ -844,7 +856,7 @@ impl Instruction {
844856
| SetupExcept { handler: l }
845857
| SetupWith { end: l }
846858
| SetupAsyncWith { end: l }
847-
| Break { target: l }
859+
| SetupLoop { break_target: l }
848860
| Continue { target: l } => Some(l),
849861
_ => None,
850862
}
@@ -853,7 +865,7 @@ impl Instruction {
853865
pub fn unconditional_branch(&self) -> bool {
854866
matches!(
855867
self,
856-
Jump { .. } | Continue { .. } | Break { .. } | ReturnValue | Raise { .. }
868+
Jump { .. } | Continue { .. } | Break | ReturnValue | Raise { .. }
857869
)
858870
}
859871

@@ -880,7 +892,7 @@ impl Instruction {
880892
Duplicate => 1,
881893
GetIter => 0,
882894
Continue { .. } => 0,
883-
Break { .. } => 0,
895+
Break => 0,
884896
Jump { .. } => 0,
885897
JumpIfTrue { .. } | JumpIfFalse { .. } => -1,
886898
JumpIfTrueOrPop { .. } | JumpIfFalseOrPop { .. } => {
@@ -897,9 +909,13 @@ impl Instruction {
897909
- flags.contains(MakeFunctionFlags::DEFAULTS) as i32
898910
+ 1
899911
}
900-
CallFunctionPositional { nargs } => -1 - (*nargs as i32) + 1,
901-
CallFunctionKeyword { nargs } => -2 - (*nargs as i32) + 1,
902-
CallFunctionEx { has_kwargs } => -2 - *has_kwargs as i32 + 1,
912+
CallFunctionPositional { nargs } => -(*nargs as i32) - 1 + 1,
913+
CallMethodPositional { nargs } => -(*nargs as i32) - 3 + 1,
914+
CallFunctionKeyword { nargs } => -1 - (*nargs as i32) - 1 + 1,
915+
CallMethodKeyword { nargs } => -1 - (*nargs as i32) - 3 + 1,
916+
CallFunctionEx { has_kwargs } => -1 - (*has_kwargs as i32) - 1 + 1,
917+
CallMethodEx { has_kwargs } => -1 - (*has_kwargs as i32) - 3 + 1,
918+
LoadMethod { .. } => -1 + 3,
903919
ForIter { .. } => {
904920
if jump {
905921
-1
@@ -910,7 +926,11 @@ impl Instruction {
910926
ReturnValue => -1,
911927
YieldValue => 0,
912928
YieldFrom => -1,
913-
SetupAnnotation | SetupLoop | SetupFinally { .. } | EnterFinally | EndFinally => 0,
929+
SetupAnnotation
930+
| SetupLoop { .. }
931+
| SetupFinally { .. }
932+
| EnterFinally
933+
| EndFinally => 0,
914934
SetupExcept { .. } => {
915935
if jump {
916936
1
@@ -1056,7 +1076,7 @@ impl Instruction {
10561076
Duplicate => w!(Duplicate),
10571077
GetIter => w!(GetIter),
10581078
Continue { target } => w!(Continue, target),
1059-
Break { target } => w!(Break, target),
1079+
Break => w!(Break),
10601080
Jump { target } => w!(Jump, target),
10611081
JumpIfTrue { target } => w!(JumpIfTrue, target),
10621082
JumpIfFalse { target } => w!(JumpIfFalse, target),
@@ -1066,12 +1086,16 @@ impl Instruction {
10661086
CallFunctionPositional { nargs } => w!(CallFunctionPositional, nargs),
10671087
CallFunctionKeyword { nargs } => w!(CallFunctionKeyword, nargs),
10681088
CallFunctionEx { has_kwargs } => w!(CallFunctionEx, has_kwargs),
1089+
LoadMethod { idx } => w!(LoadMethod, name(*idx)),
1090+
CallMethodPositional { nargs } => w!(CallMethodPositional, nargs),
1091+
CallMethodKeyword { nargs } => w!(CallMethodKeyword, nargs),
1092+
CallMethodEx { has_kwargs } => w!(CallMethodEx, has_kwargs),
10691093
ForIter { target } => w!(ForIter, target),
10701094
ReturnValue => w!(ReturnValue),
10711095
YieldValue => w!(YieldValue),
10721096
YieldFrom => w!(YieldFrom),
10731097
SetupAnnotation => w!(SetupAnnotation),
1074-
SetupLoop => w!(SetupLoop),
1098+
SetupLoop { break_target } => w!(SetupLoop, break_target),
10751099
SetupExcept { handler } => w!(SetupExcept, handler),
10761100
SetupFinally { handler } => w!(SetupFinally, handler),
10771101
EnterFinally => w!(EnterFinally),

compiler/src/compile.rs

Lines changed: 74 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,28 @@ enum NameUsage {
2828
Delete,
2929
}
3030

31+
enum CallType {
32+
Positional { nargs: u32 },
33+
Keyword { nargs: u32 },
34+
Ex { has_kwargs: bool },
35+
}
36+
impl CallType {
37+
fn normal_call(self) -> Instruction {
38+
match self {
39+
CallType::Positional { nargs } => Instruction::CallFunctionPositional { nargs },
40+
CallType::Keyword { nargs } => Instruction::CallFunctionKeyword { nargs },
41+
CallType::Ex { has_kwargs } => Instruction::CallFunctionEx { has_kwargs },
42+
}
43+
}
44+
fn method_call(self) -> Instruction {
45+
match self {
46+
CallType::Positional { nargs } => Instruction::CallMethodPositional { nargs },
47+
CallType::Keyword { nargs } => Instruction::CallMethodKeyword { nargs },
48+
CallType::Ex { has_kwargs } => Instruction::CallMethodEx { has_kwargs },
49+
}
50+
}
51+
}
52+
3153
/// Main structure holding the state of compilation.
3254
struct Compiler {
3355
code_stack: Vec<CodeInfo>,
@@ -667,14 +689,13 @@ impl Compiler {
667689
self.switch_to_block(after_block);
668690
}
669691
}
670-
Break => match self.ctx.loop_data {
671-
Some((_, end)) => {
672-
self.emit(Instruction::Break { target: end });
673-
}
674-
None => {
675-
return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location))
692+
Break => {
693+
if self.ctx.loop_data.is_some() {
694+
self.emit(Instruction::Break);
695+
} else {
696+
return Err(self.error_loc(CompileErrorType::InvalidBreak, statement.location));
676697
}
677-
},
698+
}
678699
Continue => match self.ctx.loop_data {
679700
Some((start, _)) => {
680701
self.emit(Instruction::Continue { target: start });
@@ -1242,7 +1263,8 @@ impl Compiler {
12421263
value: qualified_name,
12431264
});
12441265

1245-
self.compile_call(2, bases, keywords)?;
1266+
let call = self.compile_call_inner(2, bases, keywords)?;
1267+
self.emit(call.normal_call());
12461268

12471269
self.apply_decorators(decorator_list);
12481270

@@ -1271,7 +1293,9 @@ impl Compiler {
12711293
let else_block = self.new_block();
12721294
let after_block = self.new_block();
12731295

1274-
self.emit(Instruction::SetupLoop);
1296+
self.emit(Instruction::SetupLoop {
1297+
break_target: after_block,
1298+
});
12751299
self.switch_to_block(while_block);
12761300

12771301
self.compile_jump_if(test, false, else_block)?;
@@ -1360,7 +1384,9 @@ impl Compiler {
13601384
let else_block = self.new_block();
13611385
let after_block = self.new_block();
13621386

1363-
self.emit(Instruction::SetupLoop);
1387+
self.emit(Instruction::SetupLoop {
1388+
break_target: after_block,
1389+
});
13641390

13651391
// The thing iterated:
13661392
self.compile_expression(iter)?;
@@ -1794,10 +1820,7 @@ impl Compiler {
17941820
func,
17951821
args,
17961822
keywords,
1797-
} => {
1798-
self.compile_expression(func)?;
1799-
self.compile_call(0, args, keywords)?;
1800-
}
1823+
} => self.compile_call(func, args, keywords)?,
18011824
BoolOp { op, values } => self.compile_bool_op(op, values)?,
18021825
BinOp { left, op, right } => {
18031826
self.compile_expression(left)?;
@@ -2103,17 +2126,41 @@ impl Compiler {
21032126

21042127
fn compile_call(
21052128
&mut self,
2106-
additional_positional: u32,
2129+
func: &ast::Expr,
21072130
args: &[ast::Expr],
21082131
keywords: &[ast::Keyword],
21092132
) -> CompileResult<()> {
2133+
let method = if let ast::ExprKind::Attribute { value, attr, .. } = &func.node {
2134+
self.compile_expression(value)?;
2135+
let idx = self.name(attr);
2136+
self.emit(Instruction::LoadMethod { idx });
2137+
true
2138+
} else {
2139+
self.compile_expression(func)?;
2140+
false
2141+
};
2142+
let call = self.compile_call_inner(0, args, keywords)?;
2143+
self.emit(if method {
2144+
call.method_call()
2145+
} else {
2146+
call.normal_call()
2147+
});
2148+
Ok(())
2149+
}
2150+
2151+
fn compile_call_inner(
2152+
&mut self,
2153+
additional_positional: u32,
2154+
args: &[ast::Expr],
2155+
keywords: &[ast::Keyword],
2156+
) -> CompileResult<CallType> {
21102157
let count = (args.len() + keywords.len()) as u32 + additional_positional;
21112158

21122159
// Normal arguments:
21132160
let (size, unpack) = self.gather_elements(additional_positional, args)?;
21142161
let has_double_star = keywords.iter().any(|k| k.node.arg.is_none());
21152162

2116-
if unpack || has_double_star {
2163+
let call = if unpack || has_double_star {
21172164
// Create a tuple with positional args:
21182165
self.emit(Instruction::BuildTuple { size, unpack });
21192166

@@ -2122,7 +2169,7 @@ impl Compiler {
21222169
if has_kwargs {
21232170
self.compile_keywords(keywords)?;
21242171
}
2125-
self.emit(Instruction::CallFunctionEx { has_kwargs });
2172+
CallType::Ex { has_kwargs }
21262173
} else if !keywords.is_empty() {
21272174
let mut kwarg_names = vec![];
21282175
for keyword in keywords {
@@ -2140,12 +2187,12 @@ impl Compiler {
21402187
self.emit_constant(ConstantData::Tuple {
21412188
elements: kwarg_names,
21422189
});
2143-
self.emit(Instruction::CallFunctionKeyword { nargs: count });
2190+
CallType::Keyword { nargs: count }
21442191
} else {
2145-
self.emit(Instruction::CallFunctionPositional { nargs: count });
2146-
}
2192+
CallType::Positional { nargs: count }
2193+
};
21472194

2148-
Ok(())
2195+
Ok(call)
21492196
}
21502197

21512198
// Given a vector of expr / star expr generate code which gives either
@@ -2262,8 +2309,13 @@ impl Compiler {
22622309
unimplemented!("async for comprehensions");
22632310
}
22642311

2312+
let loop_block = self.new_block();
2313+
let after_block = self.new_block();
2314+
22652315
// Setup for loop:
2266-
self.emit(Instruction::SetupLoop);
2316+
self.emit(Instruction::SetupLoop {
2317+
break_target: after_block,
2318+
});
22672319

22682320
if loop_labels.is_empty() {
22692321
// Load iterator onto stack (passed as first argument):
@@ -2276,8 +2328,6 @@ impl Compiler {
22762328
self.emit(Instruction::GetIter);
22772329
}
22782330

2279-
let loop_block = self.new_block();
2280-
let after_block = self.new_block();
22812331
loop_labels.push((loop_block, after_block));
22822332

22832333
self.switch_to_block(loop_block);

compiler/src/ir.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -167,25 +167,29 @@ impl CodeInfo {
167167
let mut startdepths = vec![u32::MAX; self.blocks.len()];
168168
startdepths[0] = 0;
169169
stack.push(Label(0));
170+
let debug = false;
170171
'process_blocks: while let Some(block) = stack.pop() {
171172
let mut depth = startdepths[block.0 as usize];
173+
if debug {
174+
eprintln!("===BLOCK {}===", block.0);
175+
}
172176
let block = &self.blocks[block.0 as usize];
173177
for i in &block.instructions {
174178
let instr = &i.instr;
175179
let effect = instr.stack_effect(false);
176180
let new_depth = add_ui(depth, effect);
181+
if debug {
182+
eprintln!("{:?}: {:+}, {:+} = {}", instr, effect, depth, new_depth);
183+
}
177184
if new_depth > maxdepth {
178185
maxdepth = new_depth
179186
}
180-
// we don't want to worry about Continue or Break, they use unwinding to jump to
181-
// their targets and as such the stack size is taken care of in frame.rs by setting
187+
// we don't want to worry about Continue, it uses unwinding to jump to
188+
// its targets and as such the stack size is taken care of in frame.rs by setting
182189
// it back to the level it was at when SetupLoop was run
183-
let jump_label = instr.label_arg().filter(|_| {
184-
!matches!(
185-
instr,
186-
Instruction::Continue { .. } | Instruction::Break { .. }
187-
)
188-
});
190+
let jump_label = instr
191+
.label_arg()
192+
.filter(|_| !matches!(instr, Instruction::Continue { .. }));
189193
if let Some(&target_block) = jump_label {
190194
let effect = instr.stack_effect(true);
191195
let target_depth = add_ui(depth, effect);
@@ -215,7 +219,7 @@ fn stackdepth_push(stack: &mut Vec<Label>, startdepths: &mut [u32], target: Labe
215219

216220
fn add_ui(a: u32, b: i32) -> u32 {
217221
if b < 0 {
218-
a - b.abs() as u32
222+
a - b.wrapping_abs() as u32
219223
} else {
220224
a + b as u32
221225
}

0 commit comments

Comments
 (0)