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

More fixes of insufficient atomics, and make most of the test suite runnable under miri #31

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

JoJoDeveloping
Copy link
Contributor

Hi there, here are some more bugs similar to these of #30.

Additionally, I added some cfg(miri)s to make the test suite run under miri in a reasonable time.

The reason I am doing all of this is that I noticed that this crate is incompatible with Tree Borrows (TB), a new aliasing model for Rust and a potential replacement of Stacked Borrows. While TB was designed to be more lenient than SB, it also gets rid of some dirty hacks, and these unfortunately were relied on this crate (along with a few others which I am doing my best to fix now).

Or, to be more precise: They break rarena-allocator, which is a dependency of this crate, but the breakage only shows up in the tests in this crate. Fortunately, that crate is another crate of yours.

Unfortunately, while I have fixed the TB errors there, the last published version of rarena-allocator is quite different from the development HEAD (and the version depended on by skl), so I am not sure how to best contribute them. Advice on how to proceed is appreciated, if you have any questions about Tree Borrows or why these changes are necessary, feel free to ask.

@al8n
Copy link
Owner

al8n commented Aug 28, 2024

Hi, thank you so much!

Regarding the rarena-allocator, the upcoming skl version 0.14.0 (#29) will be based on the changes introduced in the rarena-allocator's pull request (al8n/rarena#8). Therefore, I would like to suggest fix any miri bugs based on al8n/rarena#8.

@JoJoDeveloping
Copy link
Contributor Author

Wow, that's a lot of change. I'll aim my changes on top of these branches.

@al8n al8n mentioned this pull request Aug 29, 2024
@al8n
Copy link
Owner

al8n commented Aug 29, 2024

I just cleaned up TB errors in rarena-allocator and publish the new version of rarena-allocator. I also ddded CI jobs to test TB in #29, and found more TB errors in this project which not related to rarena-allocator, https://github.com/al8n/skl/actions/runs/10621035558/job/29442139071

@JoJoDeveloping
Copy link
Contributor Author

Nice. I think the error looks familiar, but of course it might just be a different thing at a similar place. I'll have a look at it after the weekend tho. Thanks for getting things published :)

@al8n
Copy link
Owner

al8n commented Aug 30, 2024

Nice. I think the error looks familiar, but of course it might just be a different thing at a similar place. I'll have a look at it after the weekend tho. Thanks for getting things published :)

Thanks!

@al8n
Copy link
Owner

al8n commented Sep 5, 2024

I am merging this PR and creating a new issue, #34 for tree borrow stuff.

@al8n al8n merged commit c4e6493 into al8n:main Sep 5, 2024
22 of 25 checks passed
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