Skip to content

#![deny(unsafe_op_in_unsafe_fn)] in sys/hermit - #74979

Merged
bors merged 4 commits into
rust-lang:masterfrom
maekawatoshiki:fix
Nov 6, 2020
Merged

#![deny(unsafe_op_in_unsafe_fn)] in sys/hermit#74979
bors merged 4 commits into
rust-lang:masterfrom
maekawatoshiki:fix

Conversation

@maekawatoshiki

@maekawatoshikimaekawatoshiki commented Jul 31, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Partial fix of #73904.

This encloses unsafe operations in unsafe fn in sys/hermit.
Some unsafe blocks are not well documented because some system-based functions lack documents.

@rust-highfive

rust-highfive commented Jul 31, 2020

Copy link
Copy Markdown
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (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.

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2020
@LeSeulArtichautLeSeulArtichaut mentioned this pull request Aug 2, 2020
Closed
22 tasks
@Dylan-DPC-zz

Dylan-DPC-zz commented Aug 19, 2020•
edited
Loading

Copy link
Copy Markdown

@joshtriplett this is ready for review reassigning

@Dylan-DPC-zz

Dylan-DPC-zz commented Aug 20, 2020

Copy link
Copy Markdown

maybe

r? @Mark-Simulacrum

@Mark-Simulacrum

Mark-Simulacrum commented Aug 20, 2020

Copy link
Copy Markdown
Member

I think it's probably best to just leave this module as-is (perhaps adding a top-level allow at most). The hermit folks are the ones who can review safety comments in it well and for now it's not necessary.

I can accept this with the changes reverted and an allow added instead of denys.

I would guess that having the current diff available though would be helpful, so posting a link to a gist or so with the diff seems good.

Thanks!

@maekawatoshiki

maekawatoshiki commented Aug 21, 2020

Copy link
Copy Markdown
ContributorAuthor

This is a link to a gist with diff: https://gist.github.com/maekawatoshiki/ccd255d2e59288917ebee1fb42ba776c

Mark-Simulacrum
Mark-Simulacrum reviewed Aug 29, 2020
View reviewed changes
Comment threadlibrary/std/src/sys/hermit/thread_local_dtor.rs Outdated
@@ -1,5 +1,6 @@
#![cfg(target_thread_local)]
#![unstable(feature = "thread_local_internals", issue = "none")]
#![allow(unsafe_op_in_unsafe_fn)]

@Mark-SimulacrumMark-SimulacrumAug 29, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we put these in hermit/mod.rs instead?

@Mark-SimulacrumMark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2020
@crlf0710

crlf0710 commented Sep 18, 2020

Copy link
Copy Markdown
Member

@maekawatoshiki Ping from triage! Any updates on this?

@maekawatoshiki

maekawatoshiki commented Sep 18, 2020

Copy link
Copy Markdown
ContributorAuthor

I'm a little confused about what to do next...

@crlf0710

crlf0710 commented Oct 8, 2020

Copy link
Copy Markdown
Member

Triage: @maekawatoshiki I think leaving one single #![allow(unsafe_op_in_unsafe_fn)] at the hermit root is enough. No need to add the others.

@maekawatoshiki

maekawatoshiki commented Oct 8, 2020

Copy link
Copy Markdown
ContributorAuthor

I removed #![allow(unsafe_op_in_unsafe_fn)]s expect for mod.rs.

@Dylan-DPC-zzDylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 28, 2020
@Dylan-DPC-zz

Dylan-DPC-zz commented Oct 28, 2020

Copy link
Copy Markdown

@Mark-Simulacrum this is ready for review

@Mark-Simulacrum

Mark-Simulacrum commented Nov 5, 2020

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Nov 5, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 14158f5 has been approved by Mark-Simulacrum

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 6, 2020
@Dylan-DPC
Rollup merge ofrust-lang#74979- maekawatoshiki:fix, r=Mark-Simulacrum
4f4ce1c
`#![deny(unsafe_op_in_unsafe_fn)]` in sys/hermit
Partial fix of rust-lang#73904.
This encloses ``unsafe`` operations in ``unsafe fn`` in ``sys/hermit``.
Some unsafe blocks are not well documented because some system-based functions lack documents.
@Dylan-DPC-zzDylan-DPC-zz mentioned this pull request Nov 6, 2020
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Nov 6, 2020
@JohnTitor
Rollup merge ofrust-lang#74979- maekawatoshiki:fix, r=Mark-Simulacrum
69fe16a
`#![deny(unsafe_op_in_unsafe_fn)]` in sys/hermit
Partial fix of rust-lang#73904.
This encloses ``unsafe`` operations in ``unsafe fn`` in ``sys/hermit``.
Some unsafe blocks are not well documented because some system-based functions lack documents.
@JohnTitorJohnTitor mentioned this pull request Nov 6, 2020
Closed
@JohnTitorJohnTitor mentioned this pull request Nov 6, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 6, 2020
@bors
Auto merge ofrust-lang#78810- JohnTitor:rollup-8fhtvxu, r=JohnTitor
7e9a36f
Rollup of 15 pull requests
Successful merges:
- rust-lang#74979 (`#![deny(unsafe_op_in_unsafe_fn)]` in sys/hermit)
- rust-lang#78006 (Use Intra-doc links for std::io::buffered)
- rust-lang#78167 (Fix unreachable sub-branch detection in or-patterns)
- rust-lang#78514 (Allow using 1/2/3/4 for `x.py setup` options)
- rust-lang#78538 (BTreeMap: document a curious assumption in test cases)
- rust-lang#78559 (Add LLVM upgrades from 7 to 10 to RELEASES.md)
- rust-lang#78666 (Fix shellcheck error)
- rust-lang#78705 (Print a summary of which test suite failed)
- rust-lang#78726 (Add link to rust website)
- rust-lang#78730 (Expand explanation of reverse_bits)
- rust-lang#78760 (`deny(invalid_codeblock_attributes)` for rustc_error_codes)
- rust-lang#78771 (inliner: Copy unevaluated constants only after successful inlining)
- rust-lang#78794 (rustc_expand: use collect_bang helper instead of manual reimplementation)
- rust-lang#78795 (The renumber pass is long gone)
- rust-lang#78798 (Fixing Spelling Typos)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 4136ed2 into rust-lang:masterNov 6, 2020
@rustbotrustbot added this to the 1.49.0 milestone Nov 6, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mark-SimulacrumMark-SimulacrumMark-Simulacrum left review comments

Assignees

@Mark-SimulacrumMark-Simulacrum

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.49.0

Development

Successfully merging this pull request may close these issues.

8 participants

@maekawatoshiki@rust-highfive@Dylan-DPC-zz@Mark-Simulacrum@crlf0710@bors@joshtriplett@rustbot