Repository: rune-rs/rune (language: rust)

Issue:
&& operator changes the value of a boolean
Given the following code:
```rust
pub fn main() {
    let value = true;
    
    if value && false {
    	println!("not seen")
    }
    
    println!("{value}")
}
```

One would expect `true` to be printed, but instead the `&&` operator seems to change the value of `value` to `false`. If instead the condition is changed to `value && true`, `value` doesn't seem to change.

Playground link [here](https://rune-rs.github.io/play/?c=cHViIGZuIG1haW4oKSB7CiAgICBsZXQgdmFsdWUgPSB0cnVlOwogICAgCiAgICBpZiB2YWx1ZSAmJiBmYWxzZSB7CiAgICAJcHJpbnRsbiEoIm5vdCBzZWVuIikKICAgIH0KICAgIAogICAgcHJpbnRsbiEoInt2YWx1ZX0iKQp9Cg%3D%3D)


Discussion / hints:
Thanks for the report!

This is a bug introduced by #733

We can see it incorrectly reusing the allocated slot for `value` (if you we enable `instructions` on the playground):

```
# instruction
fn main() (0x1c69d5964e831fc1):
  0000 = allocate size=3
  0001 = store value=true, out=keep(0)
  entry:2:17-21: let value = true;
  0002 = jump-if-not cond=0, jump=4 // label:conditional_end_4 (4)
  entry:4:8-13: if value && false {
  0003 = store value=false, out=keep(0)
  entry:4:17-22: if value && false {
```

This is probably caused by an incorrect optimization [here](https://github.com/rune-rs/rune/blob/main/crates/rune/src/compile/v1/assemble.rs#L1525), whereas the needs for the left hand side is re-used by the temporary in the right hand side operator, causing it to be assigned the same slot.

Fix patch:
diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs
index 4f507cba5..bb2929e8a 100644
--- a/crates/rune/src/compile/v1/assemble.rs
+++ b/crates/rune/src/compile/v1/assemble.rs
@@ -640,10 +640,10 @@ fn condition<'a, 'hir>(
     match *hir {
         hir::Condition::Expr(hir) => {
             let scope = cx.scopes.child(hir)?;
-            let mut addr = cx.scopes.defer(hir);
+            let mut addr = cx.scopes.alloc(hir)?;
 
             let asm = if expr(cx, hir, &mut addr)?.converging() {
-                cx.asm.jump_if(addr.addr()?.addr(), then_label, hir)?;
+                cx.asm.jump_if(addr.addr(), then_label, hir)?;
                 Asm::new(hir, (scope, Pattern::Irrefutable))
             } else {
                 cx.scopes.pop(hir, scope)?;
@@ -1502,8 +1502,9 @@ fn compile_conditional_binop<'a, 'hir>(
     span: &'hir dyn Spanned,
     needs: &mut dyn Needs<'a, 'hir>,
 ) -> compile::Result<Asm<'hir>> {
-    let end_label = cx.asm.new_label("conditional_end");
     converge!(expr(cx, lhs, needs)?);
+
+    let end_label = cx.asm.new_label("conditional_end");
     let addr = needs.addr()?;
 
     match bin_op {
@@ -1521,7 +1522,8 @@ fn compile_conditional_binop<'a, 'hir>(
         }
     }
 
-    // rhs needs to be ignored since it might be jumped over.
+    // rhs needs to be ignored since its value isn't used directly, but rather
+    // only affects control flow.
     expr(cx, rhs, needs)?.ignore();
     cx.asm.label(&end_label)?;
     Ok(Asm::new(span, ()))


Test patch:
diff --git a/crates/rune/tests/bug_830.rn b/crates/rune/tests/bug_830.rn
new file mode 100644
index 000000000..44617f00a
--- /dev/null
+++ b/crates/rune/tests/bug_830.rn
@@ -0,0 +1,38 @@
+// This tests an issue where the temporary value inside of the `&&` operator
+// overwrites the left hand side slot.
+// https://github.com/rune-rs/rune/issues/830
+
+#[test]
+fn if_stmt() {
+    let value = true;
+
+    if value && false {
+        panic!("should not be reached");
+    }
+
+    assert!(value);
+}
+
+#[test]
+fn else_if_stmt() {
+    let value = true;
+
+    if false {
+        panic!("should not be reached");
+    } else if value && false {
+        panic!("should not be reached");
+    }
+
+    assert!(value);
+}
+
+#[test]
+fn while_stmt() {
+    let value = true;
+
+    while value && false {
+        panic!("should not be reached");
+    }
+
+    assert!(value);
+}
diff --git a/crates/rune/tests/lazy_and_or.rn b/crates/rune/tests/lazy_and_or.rn
index f3353db11..8114ff921 100644
--- a/crates/rune/tests/lazy_and_or.rn
+++ b/crates/rune/tests/lazy_and_or.rn
@@ -1,5 +1,5 @@
 #[test]
-fn test_lazy_and_or() {
+fn lazy_and_or() {
     let result = (|| true || return false)();
     assert_eq!(result, true);