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

Rework undocumented_unsafe_blocks #8450

Merged
merged 3 commits into from
Apr 4, 2022
Merged

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Feb 19, 2022

fixes: #8264
fixes: #8449

One thing came up while working on this. Currently comments on the same line are supported like so:

/* SAFETY: reason */ unsafe {}

Is this worth supporting at all? Anything other than a couple of words doesn't really fit well.

edit: zulip topic

changelog: Don't lint undocumented_unsafe_blocks when the unsafe block comes from a proc-macro.
changelog: Don't lint undocumented_unsafe_blocks when the preceding line has a safety comment and the unsafe block is a sub-expression.

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 19, 2022
@Jarcho Jarcho force-pushed the unsafe_blocks_8449 branch 4 times, most recently from 96ae551 to f2d4327 Compare February 20, 2022 00:13
@Jarcho Jarcho force-pushed the unsafe_blocks_8449 branch from f2d4327 to ac04157 Compare March 15, 2022 18:05
@Jarcho Jarcho force-pushed the unsafe_blocks_8449 branch from ac04157 to 65f96e2 Compare March 15, 2022 18:18
@Jarcho Jarcho changed the title WIP Rework undocumented_unsafe_blocks Rework undocumented_unsafe_blocks Mar 15, 2022
@Jarcho
Copy link
Contributor Author

Jarcho commented Mar 16, 2022

This should be good to go now.

Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

Can the comment about style that are no longer supported be added to What it does section on this lint?

@@ -89,11 +93,6 @@ fn block_comment_newlines() {
unsafe {}
}

#[rustfmt::skip]
fn inline_block_comment() {
/* Safety: */unsafe {}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to leave this as it is and add comments not to support this style.

@giraffate
Copy link
Contributor

@bors r+

It looks good, thanks!

@bors
Copy link
Contributor

bors commented Apr 4, 2022

📌 Commit 17c8bee has been approved by giraffate

@bors
Copy link
Contributor

bors commented Apr 4, 2022

⌛ Testing commit 17c8bee with merge 190f0de...

@bors
Copy link
Contributor

bors commented Apr 4, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 190f0de to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
4 participants