Skip to content

Document all unsafe usages. #72

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 12, 2020
Merged

Document all unsafe usages. #72

merged 5 commits into from
Oct 12, 2020

Conversation

Zenithsiz
Copy link
Contributor

This PR documents all usages of unsafe within the crate. This was done to ensure no soundness holes exist and to document invariants of functions for when they may be worked in the future.

This PR depends on another PR I opened a bit ago, /pull/71, but if that pull is declined, I can attempt to undo those commits and salvage the new ones.

I should also note I'm relatively new to PRs, if opening this PR while the other one is still open cause issues, I'll attempt to fix them to the best of my abilities.

@Zenithsiz
Copy link
Contributor Author

So 1.33.0 failed due to me using copied, should I instead switch is to something like .map(|&b| b) or would this already enter a 1.1 release and so min version would be higher and I'd be able to use .copied?

@Zenithsiz
Copy link
Contributor Author

I believe I've also fixed this one, now it should be based off master.

tomprogrammer
tomprogrammer reviewed Oct 11, 2020
View reviewed changes
Copy link
Owner

@tomprogrammer tomprogrammer left a comment

Choose a reason for hiding this comment

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

For this PR I have the same question left in #73 (comment):

I thought that unsafe blocks should not only contain the unsafe operation itself but also the code, that proves this code to be safe. How is your opinion on this? Is there something in the Nomicon or some other consensus I don't know about? I guess you know more about this than me, as you also future-proofed the code for a RFC.

@tomprogrammer
Copy link
Owner

I would update the MSRV to Rust 1.41.1 as Debian shipped this version recently. Therefore you can continue using Iterator::copied.

@Zenithsiz
Added global allow lint for unused_unsafe in preparation for featur…
89ee040 
…e `unsafe_block_in_unsafe_fn`.

Documented all unsafe uses in `ascii_char` module.
@Zenithsiz
Document all unsafe usages in ascii_str module.
e888608 
Redid implementation of `<str as AsMutAsciiStr>::slice_ascii_mut` to include less unsafe.
@Zenithsiz
Document all unsafe usages in ascii_string module.
d5f206d
@Zenithsiz
Documented all unsafe usages in free_functions module.
9848243
@Zenithsiz
Copy link
Contributor Author

Thanks! I've rebased on the current master to apply these changes to the branch

@Zenithsiz
Copy link
Contributor Author

Thanks for merging! Since clippy's still failing, should I add a commit to this branch to fix it?

@tomprogrammer tomprogrammer merged commit 92512d6 into tomprogrammer:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomprogrammer tomprogrammer tomprogrammer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Zenithsiz @tomprogrammer