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

feat(stdlib)!: Handle printing of reference cycles #1844

Merged
merged 3 commits into from
Jun 17, 2023

Conversation

alex-snezhko
Copy link
Member

Closes #1841

I used a style of reporting cycles similar to Node.js where references are labeled with numbers whenever there is a cycle. For example:

record A {
  val: Number,
  mut next: Option<A>
}

let a = { val: 1, next: None }
let aOpt = Some(a)
a.next = aOpt
print(a)

// output:
<1>: {
  val: 1,
  next: Some(<cycle to <1>>)
}

Copy link
Member

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome.

Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to look into only allocating the vec if needed? If not, it'd probably be worth it to just allocate a vec and reuse it instead of making a new one each time.

Copy link
Member

@peblair peblair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but see @ospencer's comment

@alex-snezhko
Copy link
Member Author

@ospencer I did end up doing something like that; initially the allocated vector box is empty and then the first time that vecPush is called it initializes the vector (though you still have to allocate the vector box each time). Reusing one vector through every call to print sounds like an interesting idea, though (and maybe I'm thinking too far ahead here) would this continue to work properly with async/parallelism gets implemented and you are trying to print in multiple threads?

@ospencer
Copy link
Member

I gotcha. We can decide to do that later as a further optimization. And yeah, it could cause some trouble for parallelism.

@phated phated added this pull request to the merge queue Jun 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 14, 2023
@phated phated added this pull request to the merge queue Jun 16, 2023
Merged via the queue into grain-lang:main with commit 49c854e Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Handle printing of cyclic structures
5 participants