From de0dde21f0396f2e5ceb3962e1f18ec74896ffbd Mon Sep 17 00:00:00 2001
From: Adam Kelly <adam@cthulahoops.org>
Date: Sun, 26 Aug 2018 08:36:13 +0100
Subject: [PATCH] Don't create extra JumpIfOrPop instructions.

As discussed:

https://github.com/RustPython/RustPython/pull/82#discussion_r212809238
---
 vm/src/bytecode.rs |  6 ------
 vm/src/compile.rs  |  8 ++++++--
 vm/src/vm.rs       | 50 +++++++++++++++++-----------------------------
 3 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/vm/src/bytecode.rs b/vm/src/bytecode.rs
index dccf7f6a1..2f7b6853d 100644
--- a/vm/src/bytecode.rs
+++ b/vm/src/bytecode.rs
@@ -83,15 +83,9 @@ pub enum Instruction {
     JumpIf {
         target: Label,
     },
-    JumpIfOrPop {
-        target: Label,
-    },
     JumpIfFalse {
         target: Label,
     },
-    JumpIfFalseOrPop {
-        target: Label,
-    },
     MakeFunction,
     CallFunction {
         count: usize,
diff --git a/vm/src/compile.rs b/vm/src/compile.rs
index d4a1848d8..a95a8d9bd 100644
--- a/vm/src/compile.rs
+++ b/vm/src/compile.rs
@@ -545,16 +545,20 @@ impl Compiler {
                 ast::BooleanOperator::And => {
                     let false_label = self.new_label();
                     self.compile_expression(a);
-                    self.emit(Instruction::JumpIfFalseOrPop {
+                    self.emit(Instruction::Duplicate);
+                    self.emit(Instruction::JumpIfFalse {
                         target: false_label,
                     });
+                    self.emit(Instruction::Pop);
                     self.compile_expression(b);
                     self.set_label(false_label);
                 }
                 ast::BooleanOperator::Or => {
                     let true_label = self.new_label();
                     self.compile_expression(a);
-                    self.emit(Instruction::JumpIfOrPop { target: true_label });
+                    self.emit(Instruction::Duplicate);
+                    self.emit(Instruction::JumpIf { target: true_label });
+                    self.emit(Instruction::Pop);
                     self.compile_expression(b);
                     self.set_label(true_label);
                 }
diff --git a/vm/src/vm.rs b/vm/src/vm.rs
index 8a99f6cd9..22e62fd0e 100644
--- a/vm/src/vm.rs
+++ b/vm/src/vm.rs
@@ -773,22 +773,28 @@ impl VirtualMachine {
             }
             bytecode::Instruction::JumpIf { target } => {
                 let obj = self.pop_value();
-                self.jump_if(target, obj, true, false)
+                match objbool::boolval(self, obj) {
+                    Ok(value) => {
+                        if value {
+                            self.jump(target);
+                        }
+                        None
+                    }
+                    Err(value) => Some(Err(value)),
+                }
             }
 
             bytecode::Instruction::JumpIfFalse { target } => {
                 let obj = self.pop_value();
-                self.jump_if(target, obj, false, false)
-            }
-
-            bytecode::Instruction::JumpIfOrPop { target } => {
-                let obj = self.last_value();
-                self.jump_if(target, obj, true, true)
-            }
-
-            bytecode::Instruction::JumpIfFalseOrPop { target } => {
-                let obj = self.last_value();
-                self.jump_if(target, obj, false, true)
+                match objbool::boolval(self, obj) {
+                    Ok(value) => {
+                        if !value {
+                            self.jump(target);
+                        }
+                        None
+                    }
+                    Err(value) => Some(Err(value)),
+                }
             }
 
             bytecode::Instruction::Raise { argc } => {
@@ -873,26 +879,6 @@ impl VirtualMachine {
         }
     }
 
-    fn jump_if(
-        &mut self,
-        label: &bytecode::Label,
-        obj: PyObjectRef,
-        jump_on: bool,
-        or_pop: bool,
-    ) -> Option<PyResult> {
-        match objbool::boolval(self, obj) {
-            Ok(value) => {
-                if value == jump_on {
-                    self.jump(label);
-                } else if or_pop {
-                    self.pop_value();
-                }
-                None
-            }
-            Err(value) => Some(Err(value)),
-        }
-    }
-
     fn jump(&mut self, label: &bytecode::Label) {
         let current_frame = self.current_frame();
         let target_pc = current_frame.code.label_map[label];