Better handling of copied captured lambda vars

Author: pachanga

src/vm/symbol/func.cs

@@ -694,6 +694,7 @@ VariableSymbol AddUpValue(VariableSymbol src)
       //NOTE: if there's no capture info we consider a default value
       UpvalMode mode = UpvalMode.STRONG;
       captures.TryGetValue(src, out mode);
+      local._upvalue_mode = mode;
 
       var upval = new AST_UpVal(local, src, mode,
         //TODO: should be the line of its usage

src/vm/symbol/variable.cs

@@ -21,12 +21,13 @@ public int scope_idx
   //referenced upvalue, keep in mind it can be a 'local' variable from the
   //outer lambda wrapping the current one
   internal VariableSymbol _upvalue;
+  internal UpvalMode _upvalue_mode;
   internal bool _is_ref =>
     //let's make sure 'this' symbol is not a ref (since it doesn't make sense)
     !(_scope_idx == 0 && name == "this") &&
       (_is_ref_decl ||
       (this is FuncArgSymbol fs && fs.is_ref) ||
-      (_upvalue?._is_ref ?? false))
+      ((_upvalue?._is_ref ?? false) && _upvalue_mode == UpvalMode.STRONG))
     ;
 #endif
 

src/vm/vm.exec.cs

@@ -2523,8 +2523,20 @@ unsafe static void OpcodeSetUpval(VM vm, ExecState exec, ref Region region, ref
           upval.frame_local_idx = func_ptr_local_idx;
 
           ref var val = ref frame.locals.vals[frame.locals_offset + frame_local_idx];
-          val._refc?.Retain();
-          upval.val = val;
+
+          if(val._refc != null)
+          {
+            //TODO: it's quite a rare case so we use reflection here
+            if(mode == UpvalMode.COPY && val._refc is ValRef vr)
+              upval.val = vr.val.Clone();
+            else
+            {
+              val._refc.Retain();
+              upval.val = val;
+            }
+          }
+          else
+            upval.val = val;
         }
 #if BHL_USE_OPCODE_SWITCH
           break;

tests/test_lambda.cs

@@ -1398,7 +1398,7 @@ func test()
   }
 
   [Fact]
-  public void TestCaptureMixCopyAndStrong()
+  public void TestCaptureMixCopyAndReference()
   {
     string bhl = @"
     func test()
@@ -1423,6 +1423,74 @@ func test()
     CommonChecks(vm);
   }
 
+  [Fact]
+  public void TestCapturedScalarReferencePassedAsCopy()
+  {
+    string bhl = @"
+    func test()
+    {
+      int v = 10
+
+      start(coro func() [v] {
+          yield()
+          yield()
+          yield()
+          trace("";1:"" + (string)v)
+        })
+      v = 100
+      start(coro func() {
+          yield()
+          yield()
+          trace("";2:"" + (string)v)
+        })
+    }
+    ";
+
+    var log = new StringBuilder();
+    var ts_fn = new Action<Types>((ts) => { BindTrace(ts, log); });
+
+    var vm = MakeVM(bhl, ts_fn);
+    Execute(vm, "test");
+    Assert.Equal(";2:100;1:10", log.ToString());
+    CommonChecks(vm);
+  }
+
+  [Fact]
+  public void TestCapturedObjReferencePassedAsCopyButNotACopy()
+  {
+    string bhl = @"
+    class Foo {
+      int f
+    }
+
+    func test()
+    {
+      Foo foo = {f: 42}
+
+      start(coro func() [foo] {
+          yield()
+          yield()
+          yield()
+          trace("";1:"" + (string)foo.f)
+        })
+      foo.f = 100
+      start(coro func() {
+          yield()
+          yield()
+          trace("";2:"" + (string)foo.f)
+        })
+    }
+    ";
+
+    var log = new StringBuilder();
+    var ts_fn = new Action<Types>((ts) => { BindTrace(ts, log); });
+
+    var vm = MakeVM(bhl, ts_fn);
+    Execute(vm, "test");
+    Assert.Equal(";2:100;1:100", log.ToString());
+    CommonChecks(vm);
+  }
+
   [Fact]
   public void TestClosure()
   {