Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Document unsafety in core::{panicking, alloc::layout, hint, iter::adapters::zip} #71492

Merged
merged 1 commit into from
Apr 24, 2020

Conversation

LeSeulArtichaut
Copy link
Contributor

Helps with #66219.
r? @Mark-Simulacrum do you want to continue reading safety comments? :D

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 23, 2020
@@ -81,10 +81,13 @@ pub fn spin_loop() {
{
#[cfg(target_arch = "aarch64")]
{
// SAFETY: the `cfg` attr ensures that we only execute this on aarch64 targets.
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have this setup so strangely? Can we "unwrap" these inner cfg's?

AFAICT we should also be able to do the same for the x86/x86_64 cfg's, just gating on target_feature = "sse2" in the wrapper (or inlining that).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should probably be done in a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, just leaving feedback in case you're up for making that PR :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like a low-effort low-hanging fruit. Yum!

@LeSeulArtichaut
Copy link
Contributor Author

@Mark-Simulacrum Resolved your review comments!

@Mark-Simulacrum
Copy link
Member

r=me with commits squashed

@LeSeulArtichaut
Copy link
Contributor Author

@Mark-Simulacrum Rebased! (I can't r+ myself I believe)

@LeSeulArtichaut
Copy link
Contributor Author

I'll try anyway :D
@bors r=Mark-Simulacrum rollup

@bors
Copy link
Contributor

bors commented Apr 23, 2020

@LeSeulArtichaut: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Contributor

bors commented Apr 23, 2020

@LeSeulArtichaut: 🔑 Insufficient privileges: not in try users

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 23, 2020

📌 Commit d515168 has been approved by Mark-Simulacrum

@bors bors 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 Apr 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 24, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#70845 (Make the `structural_match` error diagnostic for const generics clearer)
 - rust-lang#71063 (Document unsafety in core::{option, hash})
 - rust-lang#71068 (Stabilize UNICODE_VERSION (feature unicode_version))
 - rust-lang#71426 (fix error code in E0751.md)
 - rust-lang#71459 (Add leading 0x to offset in Debug fmt of Pointer)
 - rust-lang#71492 (Document unsafety in core::{panicking, alloc::layout, hint, iter::adapters::zip})

Failed merges:

r? @ghost
@bors bors merged commit 8a0e88e into rust-lang:master Apr 24, 2020
@LeSeulArtichaut LeSeulArtichaut deleted the document-unsafe-2 branch April 24, 2020 08:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants