Recheck refs after processing fields

When dropping a class, we recheck the reference count to ensure a
destructor doesn't introduce new references to a value that we're
dropping. This check must be performed _after_ processing all fields and
not before it, otherwise we won't be able to drop cyclic types (e.g.
"Foo" stores an owned "Bar", which contains a reference back to "Foo").

Thanks to Dusty for reporting this in the Discord channel.

Changelog: fixed

compiler/src/mir/passes.rs

@@ -457,10 +457,6 @@ impl<'a> GenerateDropper<'a> {
             lower.reduce_call(typ, loc);
         }
 
-        // We check _after_ the destructor to guard against new references to
-        // "self" being created.
-        lower.current_block_mut().check_refs(self_reg, loc);
-
         let variants = class.variants(lower.db());
         let mut blocks = Vec::new();
         let before_block = lower.current_block;
@@ -499,6 +495,11 @@ impl<'a> GenerateDropper<'a> {
 
         lower.current_block = after_block;
 
+        // Destructors may introduce new references, so we have to check again.
+        // We do this _after_ processing fields so we can correctly drop cyclic
+        // types.
+        lower.current_block_mut().check_refs(self_reg, loc);
+
         lower.drop_register(tag_reg, loc);
         lower.current_block_mut().free(self_reg, loc);
 
@@ -541,10 +542,6 @@ impl<'a> GenerateDropper<'a> {
             lower.reduce_call(typ, loc);
         }
 
-        // We check _after_ the destructor to guard against new references to
-        // "self" being created.
-        lower.current_block_mut().check_refs(self_reg, loc);
-
         for field in class.fields(lower.db()) {
             let typ = field.value_type(lower.db());
 
@@ -560,6 +557,11 @@ impl<'a> GenerateDropper<'a> {
             lower.drop_register(reg, loc);
         }
 
+        // Destructors may introduce new references, so we have to check again.
+        // We do this _after_ processing fields so we can correctly drop cyclic
+        // types.
+        lower.current_block_mut().check_refs(self_reg, loc);
+
         if free_self {
             lower.current_block_mut().free(self_reg, loc);
         }

std/test/compiler/test_drop.inko

@@ -31,6 +31,18 @@ class Box {
   }
 }
 
+class CyclicClass {
+  let @backref: Backref[CyclicClass]
+}
+
+class Backref[T] {
+  let @target: Option[ref T]
+}
+
+class enum CyclicEnum {
+  case Backref(Backref[CyclicEnum])
+}
+
 fn pub tests(t: mut Tests) {
   t.test('Assigning a variable drops the old value') fn (t) {
     let val = Value.new
@@ -63,4 +75,20 @@ fn pub tests(t: mut Tests) {
     fn move { vref = Option.None }.call
     t.equal(val.dropped, 1)
   }
+
+  t.no_panic('Dropping a class with a cyclic reference') fn {
+    let val_ref = Backref { @target = Option.None }
+    let val = CyclicClass { @backref = val_ref }
+
+    val.backref.target = Option.Some(ref val)
+  }
+
+  t.no_panic('Dropping an enum with a cyclic reference') fn {
+    let val_ref = Backref { @target = Option.None }
+    let val = CyclicEnum.Backref(val_ref)
+
+    match mut val {
+      case Backref(r) -> r.target = Option.Some(ref val)
+    }
+  }
 }