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

Lint request: Safety comment on safe code #6880

Closed
Tracked by #79
alice-i-cecile opened this issue Mar 10, 2021 · 5 comments · Fixed by #9822
Closed
Tracked by #79

Lint request: Safety comment on safe code #6880

alice-i-cecile opened this issue Mar 10, 2021 · 5 comments · Fixed by #9822
Labels
A-lint Area: New lints

Comments

@alice-i-cecile
Copy link

alice-i-cecile commented Mar 10, 2021

What it does

Warns when a SAFETY comment is left on a safe function, trait etc.

What is the advantage of the recommended code over the original code

Avoids users accidentally using unsafe code in an inappropriate fashion because it's marked safe.

Drawbacks

Won't work on normal comments, see #4828.

Example

/// Safety: This function must be used very carefully 
pub fn my_fn(){
}

Should be written as:

/// Safety: This function must be used very carefully 
pub unsafe fn my_fn(){
}

Here's an example of where this slipped through into real code.

@alice-i-cecile alice-i-cecile added the A-lint Area: New lints label Mar 10, 2021
@alice-i-cecile alice-i-cecile changed the title Lint request: Safety comment on safe function Lint request: Safety comment on safe code Mar 10, 2021
@giraffate
Copy link
Contributor

Do you mean the missing_safety_doc lint?

@alice-i-cecile
Copy link
Author

No: the reverse. A safety doc was left on code that was not marked unsafe, implying that there were UB implications to using it incorrectly.

This is dangerous because safe code should not cause UB, even indirectly by violating unsafe code's assumptions in complex ways.

@ojeda
Copy link
Contributor

ojeda commented Nov 9, 2021

This is a good idea; in fact, possibly for both missing_safety_doc and undocumented_unsafe_blocks.

The description is unclear which one it is referring to, though (it says "SAFETY comment" and "unsafe code", but the examples seem to imply safety contracts for unsafe functions).

@alice-i-cecile
Copy link
Author

The description is unclear which one it is referring to, though (it says "SAFETY comment" and "unsafe code", but the examples seem to imply safety contracts for unsafe functions).

My apologies. I meant the latter, but I suspect both would be useful.

@ojeda
Copy link
Contributor

ojeda commented Nov 9, 2021

My apologies. I meant the latter, but I suspect both would be useful.

No need to apologize! And thanks for this idea and opening this issue. I have opened another one for the former, then: #7954.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants