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

Check the size of piccolo's Thread, not std's. #99

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

ahmedcharles
Copy link
Contributor

No description provided.

@ginnyTheCat
Copy link

The only reason they fail under miri is that the sizes changed in the newest nightly version with which miri is being run. The only appropriate fix would be to adjust or remove the test all together.

@ahmedcharles
Copy link
Contributor Author

Do you know why the size changed with the nightly? Do you have a link to that RFC?

@ginnyTheCat
Copy link

ginnyTheCat commented Nov 18, 2024

Jup. I noticed it when the CI build for my PR failed as well and it's caused by this PR rust-lang/rust#123550.

What I don't quite understand is what the reason is piccolo checks whether these sizes are what they are, as from the little code I read piccolo doesn't seem to use std::thread::Thread anywhere else. What I'm assuming is that it happened to be a miss-import and actually the size of piccolo::Thread should've been tested instead (which is a pointer wide as well).

The fact that at the bottom of the test it references the Value enum as well which has all these types in the same order makes me think that's the intention of the test.

@ahmedcharles
Copy link
Contributor Author

Yes, it's supposed to check piccolo::Thread. I assume this was due to an IDE importing the wrong Thread.

@ahmedcharles ahmedcharles changed the title Update sizes.rs to be ignored under miri. Check the size of piccolo's Thread, not std's. Nov 21, 2024
@kyren kyren merged commit 28d9c92 into kyren:master Dec 2, 2024
1 check passed
@kyren
Copy link
Owner

kyren commented Dec 2, 2024

Thank you for the PR!

Yes, that is definitely supposed to check the size of piccolo's Thread, it's supposed to check to make sure it's a thin pointer.

@ahmedcharles ahmedcharles deleted the patch-2 branch December 3, 2024 06:27
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.

3 participants