From 1eab9165bf4df9e91e8144c6bcdbadd4d0045c38 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Mon, 10 Sep 2018 20:10:31 +0200 Subject: [PATCH 1/4] Initial work on 'with' context manager logic --- parser/src/ast.rs | 8 +++++++- parser/src/python.lalrpop | 18 ++++++++++++++++-- tests/snippets/with.py | 22 ++++++++++++++++++++++ vm/src/bytecode.rs | 3 +++ vm/src/compile.rs | 17 +++++++++++++++-- vm/src/frame.rs | 1 + vm/src/vm.rs | 19 ++++++++++++++++++- 7 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 tests/snippets/with.py diff --git a/parser/src/ast.rs b/parser/src/ast.rs index 2f811322e..4f821d2fe 100644 --- a/parser/src/ast.rs +++ b/parser/src/ast.rs @@ -74,7 +74,7 @@ pub enum Statement { orelse: Option>, }, With { - items: Expression, + items: Vec, body: Vec, }, For { @@ -106,6 +106,12 @@ pub enum Statement { }, } +#[derive(Debug, PartialEq)] +pub struct WithItem { + pub context_expr: Expression, + pub optional_vars: Option, +} + #[derive(Debug, PartialEq, Clone)] pub enum Expression { BoolOp { diff --git a/parser/src/python.lalrpop b/parser/src/python.lalrpop index b7238e8b7..3135d7d15 100644 --- a/parser/src/python.lalrpop +++ b/parser/src/python.lalrpop @@ -291,14 +291,28 @@ ExceptClause: ast::ExceptHandler = { }; WithStatement: ast::LocatedStatement = { - "with" "as" <_e:Expression> ":" => { + "with" ":" => { + let mut items = vec![i1]; + for item in i2 { + items.push(item.1); + } ast::LocatedStatement { location: loc, - node: ast::Statement::With { items: t, body: s }, + node: ast::Statement::With { items: items, body: s }, } }, }; +WithItem: ast::WithItem = { + => { + let optional_vars = match n { + Some(val) => Some(val.1), + None => None, + }; + ast::WithItem { context_expr: t, optional_vars } + }, +}; + FuncDef: ast::LocatedStatement = { "def" ":" => { ast::LocatedStatement { diff --git a/tests/snippets/with.py b/tests/snippets/with.py new file mode 100644 index 000000000..c26a1c724 --- /dev/null +++ b/tests/snippets/with.py @@ -0,0 +1,22 @@ + +ls = [] + +class ContextManager: + def __enter__(self): + print('Entrada') + ls.append(1) + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + ls.append(2) + print('Wiedersehen') + + def __str__(self): + ls.append(3) + return "c'est moi!" + + +with ContextManager() as c: + print(c) + +assert ls == [1, 3, 2] diff --git a/vm/src/bytecode.rs b/vm/src/bytecode.rs index c0d5d286d..f01f6ae4a 100644 --- a/vm/src/bytecode.rs +++ b/vm/src/bytecode.rs @@ -118,6 +118,9 @@ pub enum Instruction { SetupExcept { handler: Label, }, + SetupWith { + end: Label, + }, PopBlock, Raise { argc: usize, diff --git a/vm/src/compile.rs b/vm/src/compile.rs index 3208f9700..c9494a0e7 100644 --- a/vm/src/compile.rs +++ b/vm/src/compile.rs @@ -199,8 +199,21 @@ impl Compiler { }); self.set_label(end_label); } - ast::Statement::With { items: _, body: _ } => { - // TODO + ast::Statement::With { items, body } => { + for item in items { + self.compile_expression(&item.context_expr); + match &item.optional_vars { + Some(var) => { + }, + None => { + self.emit(Instruction::Pop); + } + } + } + let end_label = self.new_label(); + self.emit(Instruction::SetupWith { end: end_label }); + self.compile_statements(body); + self.set_label(end_label); } ast::Statement::For { target, diff --git a/vm/src/frame.rs b/vm/src/frame.rs index fb76f48a4..4e4120944 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -16,6 +16,7 @@ pub enum Block { #[allow(dead_code)] // TODO: Implement try/except blocks TryExcept { handler: bytecode::Label }, + With { end: bytecode::Label }, } pub struct Frame { diff --git a/vm/src/vm.rs b/vm/src/vm.rs index a50b2eb4b..b165678a1 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -175,12 +175,24 @@ impl VirtualMachine { self.current_frame().last_block() } + fn with_exit(&mut self) { + // Assume top of stack is __exit__ method: + let exit_method = self.pop_value(); + let args = PyFuncArgs { + args: vec![], + kwargs: vec![], + }; + // TODO: what happens when we got an error during handling exception? + self.invoke(exit_method, args).unwrap(); + } + fn unwind_loop(&mut self) -> Block { loop { let block = self.pop_block(); match block { Some(Block::Loop { start: _, end: __ }) => break block.unwrap(), Some(Block::TryExcept { .. }) => {} + Some(Block::With { .. }) => { self.with_exit(); } None => panic!("No block to break / continue"), } } @@ -196,7 +208,8 @@ impl VirtualMachine { self.jump(&handler); return None; } - Some(_) => {} + Some(Block::With { .. }) => { self.with_exit(); } + Some(Block::Loop { .. }) => {} None => break, } } @@ -823,6 +836,10 @@ impl VirtualMachine { self.push_block(Block::TryExcept { handler: *handler }); None } + bytecode::Instruction::SetupWith { end } => { + self.push_block(Block::With { end: *end }); + None + } bytecode::Instruction::PopBlock => { self.pop_block(); None From 9bf1cb3db8c680067777b33995c664298ecd1412 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Mon, 10 Sep 2018 21:20:23 +0200 Subject: [PATCH 2/4] First simple variant of with-statement operational. --- vm/src/bytecode.rs | 3 +++ vm/src/compile.rs | 11 ++++++++--- vm/src/frame.rs | 5 ++++- vm/src/vm.rs | 47 ++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/vm/src/bytecode.rs b/vm/src/bytecode.rs index 255d245ee..debd63c37 100644 --- a/vm/src/bytecode.rs +++ b/vm/src/bytecode.rs @@ -121,6 +121,9 @@ pub enum Instruction { SetupWith { end: Label, }, + CleanupWith { + end: Label, + }, PopBlock, Raise { argc: usize, diff --git a/vm/src/compile.rs b/vm/src/compile.rs index d902e9380..576d4c985 100644 --- a/vm/src/compile.rs +++ b/vm/src/compile.rs @@ -200,19 +200,24 @@ impl Compiler { self.set_label(end_label); } ast::Statement::With { items, body } => { + let end_label = self.new_label(); for item in items { self.compile_expression(&item.context_expr); + self.emit(Instruction::SetupWith { end: end_label }); match &item.optional_vars { Some(var) => { - }, + self.compile_store(var); + } None => { self.emit(Instruction::Pop); } } } - let end_label = self.new_label(); - self.emit(Instruction::SetupWith { end: end_label }); + self.compile_statements(body); + for _ in 0..items.len() { + self.emit(Instruction::CleanupWith { end: end_label }); + } self.set_label(end_label); } ast::Statement::For { diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 4e4120944..9fc0e8d34 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -16,7 +16,10 @@ pub enum Block { #[allow(dead_code)] // TODO: Implement try/except blocks TryExcept { handler: bytecode::Label }, - With { end: bytecode::Label }, + With { + end: bytecode::Label, + context_manager: PyObjectRef, + }, } pub struct Frame { diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 5c98144fc..afe3abae6 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -177,6 +177,7 @@ impl VirtualMachine { fn with_exit(&mut self) { // Assume top of stack is __exit__ method: + // TODO: do we want to put the exit call on the stack? let exit_method = self.pop_value(); let args = PyFuncArgs { args: vec![], @@ -192,7 +193,9 @@ impl VirtualMachine { match block { Some(Block::Loop { start: _, end: __ }) => break block.unwrap(), Some(Block::TryExcept { .. }) => {} - Some(Block::With { .. }) => { self.with_exit(); } + Some(Block::With { .. }) => { + self.with_exit(); + } None => panic!("No block to break / continue"), } } @@ -208,7 +211,9 @@ impl VirtualMachine { self.jump(&handler); return None; } - Some(Block::With { .. }) => { self.with_exit(); } + Some(Block::With { .. }) => { + self.with_exit(); + } Some(Block::Loop { .. }) => {} None => break, } @@ -837,8 +842,42 @@ impl VirtualMachine { None } bytecode::Instruction::SetupWith { end } => { - self.push_block(Block::With { end: *end }); - None + let obj = self.pop_value(); + // Call enter: + match self.call_method(obj, "__enter__", vec![]) { + Ok(manager) => { + self.push_block(Block::With { + end: *end, + context_manager: manager.clone(), + }); + self.push_value(manager); + None + } + Err(err) => Some(Err(err)), + } + } + bytecode::Instruction::CleanupWith { end: end1 } => { + let block = self.pop_block().unwrap(); + if let Block::With { + end: end2, + context_manager, + } = &block + { + assert!(end1 == end2); + + // call exit now: + let exc_type = self.ctx.none(); + let exc_val = self.ctx.none(); + let exc_tb = self.ctx.none(); + self.call_method( + context_manager.clone(), + "__exit__", + vec![exc_type, exc_val, exc_tb], + ).unwrap(); + None + } else { + panic!("Block stack is incorrect, expected a with block"); + } } bytecode::Instruction::PopBlock => { self.pop_block(); From 638af166df3777648e21de4b96c135b888664589 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Wed, 12 Sep 2018 19:13:34 +0200 Subject: [PATCH 3/4] Process review comments --- tests/snippets/with.py | 18 +++++++++++++++++- vm/src/vm.rs | 16 +++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/tests/snippets/with.py b/tests/snippets/with.py index c26a1c724..dc65f90e2 100644 --- a/tests/snippets/with.py +++ b/tests/snippets/with.py @@ -15,8 +15,24 @@ class ContextManager: ls.append(3) return "c'est moi!" - with ContextManager() as c: print(c) assert ls == [1, 3, 2] + +ls = [] +class ContextManager2: + def __enter__(self): + print('Entrada') + ls.append(1) + return ls + + def __exit__(self, exc_type, exc_val, exc_tb): + ls.append(2) + print('Wiedersehen') + +with ContextManager2() as c: + print(c) + assert c == [1] + +assert ls == [1, 2] diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 30c6c55b5..2984ec7fa 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -179,10 +179,7 @@ impl VirtualMachine { // Assume top of stack is __exit__ method: // TODO: do we want to put the exit call on the stack? let exit_method = self.pop_value(); - let args = PyFuncArgs { - args: vec![], - kwargs: vec![], - }; + let args = PyFuncArgs::default(); // TODO: what happens when we got an error during handling exception? self.invoke(exit_method, args).unwrap(); } @@ -842,15 +839,15 @@ impl VirtualMachine { None } bytecode::Instruction::SetupWith { end } => { - let obj = self.pop_value(); + let context_manager = self.pop_value(); // Call enter: - match self.call_method(obj, "__enter__", vec![]) { - Ok(manager) => { + match self.call_method(context_manager.clone(), "__enter__", vec![]) { + Ok(obj) => { self.push_block(Block::With { end: *end, - context_manager: manager.clone(), + context_manager: context_manager.clone(), }); - self.push_value(manager); + self.push_value(obj); None } Err(err) => Some(Err(err)), @@ -866,6 +863,7 @@ impl VirtualMachine { assert!(end1 == end2); // call exit now: + // TODO: improve exception handling in context manager. let exc_type = self.ctx.none(); let exc_val = self.ctx.none(); let exc_tb = self.ctx.none(); From 013964549bdee5f87f1034d2af77b6c9c020b692 Mon Sep 17 00:00:00 2001 From: Windel Bouwman Date: Wed, 12 Sep 2018 19:25:21 +0200 Subject: [PATCH 4/4] Add example with two context managers in a single with statement --- tests/snippets/with.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/snippets/with.py b/tests/snippets/with.py index dc65f90e2..f4efa86e7 100644 --- a/tests/snippets/with.py +++ b/tests/snippets/with.py @@ -1,5 +1,4 @@ -ls = [] class ContextManager: def __enter__(self): @@ -15,24 +14,29 @@ class ContextManager: ls.append(3) return "c'est moi!" +ls = [] with ContextManager() as c: print(c) - assert ls == [1, 3, 2] -ls = [] class ContextManager2: def __enter__(self): - print('Entrada') - ls.append(1) + print('Ni hau') + ls.append(4) return ls def __exit__(self, exc_type, exc_val, exc_tb): - ls.append(2) - print('Wiedersehen') + ls.append(5) + print('Ajuus') +ls = [] with ContextManager2() as c: print(c) - assert c == [1] + assert c == [4] +assert ls == [4, 5] -assert ls == [1, 2] +ls = [] +with ContextManager() as c1, ContextManager2() as c2: + print(c1) + assert c2 == [1, 4, 3] +assert ls == [1, 4, 3, 5, 2]