Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Probable reference getting dropped twice in match statement #580

Closed
dusty-phillips opened this issue Jun 18, 2023 · 2 comments
Closed

Probable reference getting dropped twice in match statement #580

dusty-phillips opened this issue Jun 18, 2023 · 2 comments
Labels
bug Defects, unintended behaviour, etc compiler Changes related to the compiler
Milestone

Comments

@dusty-phillips
Copy link
Contributor

Please describe the bug

Thread here: https://discord.com/channels/1044083486509781122/1044083486509781125/1119329987489714346

Basically, I'm getting some kind of underflow on reference count when matching in a recursive situation. My original code had a lot of references within the matched state, but I actually managed to create a repro that doesn't keep any references in the data structure.

❯ inko run
Stack trace (the most recent call comes last):
  /Users/dustyphillips/Desktop/Code/inko/repro/src/main.inko:39 in main::Main.main
  /Users/dustyphillips/Desktop/Code/inko/repro/src/main.inko:1 in main::State.$dropper
  /Users/dustyphillips/Desktop/Code/inko/repro/src/main.inko:24 in main::Transition.$dropper
  /Users/dustyphillips/Desktop/Code/inko/repro/src/main.inko:24 in main::Transition.$dropper
  /Users/dustyphillips/Desktop/Code/inko/repro/src/main.inko:1 in main::State.$dropper
Process 'Main' (0x600002d40100) panicked: can't drop a value of type 'State' as it still has 4294967295 reference(s)

Please list the exact steps necessary to reproduce the bug

This gist is the smallest repro I was able to come up with: https://gist.github.com/dusty-phillips/a8608bcfc8fb2250134d6c776cc82300

Operating system

macos Ventura 13.4

Inko version

main

Rust version

1.69.0

@dusty-phillips dusty-phillips added the bug Defects, unintended behaviour, etc label Jun 18, 2023
@yorickpeterse yorickpeterse added the compiler Changes related to the compiler label Jun 18, 2023
@yorickpeterse yorickpeterse added this to the 0.13.0 milestone Jun 18, 2023
@yorickpeterse
Copy link
Collaborator

The example can be reduced to the following:

class pub State {
  let @transition: Transition

  fn static new(transition: Transition) -> State {
    State { @transition = transition }
  }

  fn foo {
    match @transition {
      case Split(Empty(state1)) -> {}
      case _ -> {}
    }
  }
}

class pub enum Transition {
  case End
  case Empty(State)
  case Split(Transition)
}

class async Main {
  fn async main() {
    State
      .new(Transition.Split(Transition.Empty(State.new(Transition.End))))
      .foo
  }
}

The MIR for this is:

graph

The problem here is that we forget to increment state1. This happens because the type of the register we're checking to determine if a move or increment is needed, is owned and not a reference.

The problem seems to be that as part of generating the pattern matching decision tree and variables, we don't take into account that if the root type is a ref T/mut T, any sub values matched against must also be a ref T/mut T (as we don't allow moving values out of references). The following code is probably not working correctly:

TypeResolver::new(&mut self.state.db, &args, &self.bounds)
.resolve(raw_type)
.cast_according_to(source_variable_type, self.db());

@yorickpeterse
Copy link
Collaborator

yorickpeterse commented Jun 20, 2023

Actually I was wrong, it's these lines:

if !instance.instance_of().is_generic(self.db()) {
return types.into_iter().map(|t| self.new_variable(t)).collect();
}

While we don't need to use TypeResolver here, we also forgot to include a cast_according_to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defects, unintended behaviour, etc compiler Changes related to the compiler
Projects
None yet
Development

No branches or pull requests

2 participants