Skip to content

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?


#[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

Development

Successfully merging this pull request may close these issues.

undocumented_unsafe_blocks false positive when safety comment is on the previous line Unfixable clippy::undocumented_unsafe_blocks in macro output

4 participants