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

Improve compatibility with nimskull #167

Merged
merged 6 commits into from
Nov 5, 2023

Conversation

alaviss
Copy link
Contributor

@alaviss alaviss commented Nov 3, 2023

Major changes

  • Fix memory corruption on ARC/ORC due to use-after-free
  • Switch most usage of multimap to single map. More testing needed.
  • Support for nimskull config parser API.

TODO before merge

Notes

Nimble fork to use: https://github.com/alaviss/nimble/tree/nimskull

`defer` in Nim works at scope level and not function level. By calling
`defer` in an `if` like this, the pointer was freed immediately, causing
an use-after-free bug, breaking nimph on ARC/ORC.

This commit fixes the `defer` declaration, allowing nimph to work on ARC/ORC.
@alaviss alaviss force-pushed the towards-nimskull branch 2 times, most recently from bda572d to 45f6d41 Compare November 4, 2023 17:39
The multimap API has been deprecated for quite awhile and nimskull
already removed it from tables.

This commit switch to Table with seq value as an alternative.
Overloading iterator and proc of the same name is currently buggy on NimSkull.

Rename symbolicMatch iterator to symbolicMatches to avoid this issue.
NimSkull has removed the multimap API, and given that Group was not
very clear on whether it's a multimap or singular map centric, let's
temporary use singular map and convert back as needed.
This release contains fixes for nimskull
@alaviss
Copy link
Contributor Author

alaviss commented Nov 4, 2023

Should now work with nimskull >= 0.1.0-dev.21077

Will need some CI to test this

@alaviss alaviss marked this pull request as ready for review November 4, 2023 20:04
@disruptek disruptek merged commit 2c8d484 into disruptek:master Nov 5, 2023
3 checks passed
@alaviss alaviss deleted the towards-nimskull branch November 5, 2023 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants