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

Add ucontext_t for aarch64-unknown-linux-musl #1863

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

ifreund
Copy link
Contributor

@ifreund ifreund commented Aug 16, 2020

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@ifreund ifreund force-pushed the aarch64-musl-ucontext_t branch from 1ca768f to eb618e2 Compare August 16, 2020 18:30
@JohnTitor
Copy link
Member

Use no_extra_traits due to array longer than 32

Did you test with this workaround? I guess we could auto-derive it here as well...

// nested arrays to get the right size/length while being able to
// auto-derive traits like Debug
__reserved: [[u64; 32]; 16],

@ifreund
Copy link
Contributor Author

ifreund commented Aug 19, 2020

Did you test with this workaround? I guess we could auto-derive it here as well...

I tried it but the math doesn't work out in this case. The prime factorization of 36 + 512 is 2 * 2 * 137. Guess we could use multiple members though if that would be preferable.

@JohnTitor
Copy link
Member

Ah, makes sense. I'd still prefer to auto-derive it as this will make the difference between arch/env and users will be confused.

@JohnTitor
Copy link
Member

(Note that CI will fail anyway until #1865 gets merged.)

@JohnTitor
Copy link
Member

Looks good except for spurious failures. Could you rebase and squash commits into one?

@ifreund ifreund force-pushed the aarch64-musl-ucontext_t branch from e385d17 to 2259b0b Compare August 19, 2020 08:18
@ifreund
Copy link
Contributor Author

ifreund commented Aug 19, 2020

@JohnTitor squashed!

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@JohnTitor JohnTitor merged commit 50318f7 into rust-lang:master Aug 19, 2020
@ifreund ifreund deleted the aarch64-musl-ucontext_t branch August 19, 2020 20:42
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