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

Array as map key #657

Closed
jinyus opened this issue Nov 16, 2023 · 5 comments
Closed

Array as map key #657

jinyus opened this issue Nov 16, 2023 · 5 comments
Labels
bug Defects, unintended behaviour, etc

Comments

@jinyus
Copy link
Contributor

jinyus commented Nov 16, 2023

Please describe the bug

Is this possible with a mutable array? somehow it's not detecting that the new array shouldn't be dropped.

Sample repo: https://github.com/jinyus/inko_demo

Code:

  let mut tag_map: Map[String, mut Array[Int]] = Map.with_capacity(128)

  posts.iter.each_with_index fn (idx,p){

    p.tags.iter.each fn (tag){
      # rust equivalent: post_tags_map.entry(tag).or_default().push(post_idx);

      let tag_posts = tag_map.opt_mut(tag).unwrap_or(mut Array.with_capacity(128))

      
      if tag_posts.size == 0 {
        tag_map.set(tag, tag_posts)
      }

      tag_posts.push(idx)
    }

  }

Error:

Stack trace (the most recent call comes last):
  /home/dev/projects/inko/demo/inko_demo/demo.inko:29 in main.Main.main
  /usr/lib/inko/std/std/iter.inko:69 in std.iter.Stream.each_with_index
  /usr/lib/inko/std/std/iter.inko:50 in std.iter.Stream.each
  /usr/lib/inko/std/std/iter.inko:71 in std.iter.Closure75.call
 /projects/inko/demo/inko_demo/demo.inko:31 in main.Closure53.call
  /usr/lib/inko/std/std/iter.inko:50 in std.iter.Stream.each
  /projects/inko/demo/inko_demo/demo.inko:34 in main.Closure54.call
  /usr/lib/inko/std/std/array.inko:104 in std.array.Array.$dropper
Process 'Main' (0x55d19d1878b0) panicked: can't drop a value of type 'Array' as it still has 1 reference(s)

ps: I'm trying to add inko to my benchmark: https://github.com/jinyus/related_post_gen

Please list the exact steps necessary to reproduce the bug

git clone https://github.com/jinyus/inko_demo.git

cd inko_demo

inko run demo.inko

Operating system

Linux (6.6.1-arch1-1)

Inko version

0.13.1

Rust version

1.73.0 (cc66ad468 2023-10-03)

@jinyus jinyus added the bug Defects, unintended behaviour, etc label Nov 16, 2023
@yorickpeterse
Copy link
Collaborator

The problem is this line:

let tag_posts = tag_map.opt_mut(tag).unwrap_or(mut Array.with_capacity(128))

Within a method call chain, temporary values that aren't moved are dropped at the end of the chain. In this case the mut Array is moved, leaving behind the owned value. This value is then dropped, but the mut Array is still stored in tag_posts.

To better understand this, the line in question is essentially the same as this:

let tag_posts = {
  let array = Array.with_capacity(128)
  let ret = tag_map.opt_mut(tag).unwrap_or(mut array)

  drop(array)
  ret
}

What you want to do instead here is something like this:

match tag_map.opt_mut(tag) {
  case Some(v) -> v
  case _ -> {
    let array = Array.with_capacity(128)
    let ret = mut array

    tag_map.set(tag, array)
    array
  }
}

However, in doing so you'll most likely run into the bug outlined in #636. You can however work around that by adding an explicit type annotation to tag_map.

Looking at the documentation, I realized I didn't document how drops work for method call chains. In fact, the only documentation on method calls is the syntax guide, which isn't great. I'll create an issue for that.

@yorickpeterse
Copy link
Collaborator

The issue in question is #658.

@yorickpeterse
Copy link
Collaborator

Since this isn't an actual bug but rather the result of lacking documentation, I'll close this in favour of #658

@jinyus
Copy link
Contributor Author

jinyus commented Nov 16, 2023

Thanks for the guidance. I guess this can be closed since it's expected behavior..

@jinyus
Copy link
Contributor Author

jinyus commented Nov 17, 2023

@yorickpeterse take a look if you have the time: jinyus/related_post_gen#440

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

No branches or pull requests

2 participants