[Pre-RFC] Rustc and Handling Unsafe Code

1

Hi all, I wanted to reach out before putting big reams of words together for some feedback around previous Unsafe usage discussions, in case this idea has been floated before.

My idea is to propose the following in an RFC:

Unsafe Code Comes with a Warning

The #![warn(unsafe_code)] compiler lint is switched on by default and becomes becomes an opt-out warning message (instead of the current opt-in), this warning can then be turned into an error using the deny attribute.

This warning should apply to all crates in the project, thus allowing a user to see these warnings and be drawn to the use of unsafe code in any of the crates that the users included in Cargo.toml.

Unsafe Code Can be signed off

As many of us know, unsafe is not always problematic, sometimes it is really necessary, which leads to the second part of the proposal, an annotation with which the unsafe code warning lint can be suppressed. The same mechanism that suppresses other warning can be re-used or a new annotation can be added, perhaps something like: #[unsafe-signoff: "I'm just that good, this can't possibly be UB"], #[unsafe-signoff: "Unsafe use mandated for FFI"], #[unsafe-signoff: "See comment above for motivation"], etc.

This can then be applied to the various bits and pieces of the std library, for example, serving as a sign of quality and a bit of an education.

Signoff Messages can be disabled/ignored

Lastly a compiler flag will be proposed that will disable the annotation/suppression, something along the line of: --warn-all-unsafe or --ignore-unsafe-signoff. This will allow developers to view all use of unsafe in their projects.

Has a strategy like this been considered before, are these ideas worth turning into an RFC? The idea here is to nudge developers on the importance, to the community, of protecting the rust ecosystem as a whole.

Any comments welcome.

2

This sounds very similar to the existing unsafe_code lint which can be turned into either a warning or error via #![warn(unsafe_code)] or #![deny(unsafe_code)] example

4 Likes
3

Oh yeah, there's been tons of discussion on this sort of thing. Here's just one example:

My impression based on all of the past chatter I've seen is that unsafe annotations are just not where the big potential wins are right now. What we really need is to keep pushing forward on all the stuff the UCG group is doing, such as pin down an official spec for what is and isn't UB/what unsafe code is "sound" in Rust, but also to what extent soundness/UB-ness can be programmatically checked.

4 Likes
4

Ah, wasn’t aware of this thanks, updating proposal.

5

This is something that could be done as a proc-macro-attribute plus an external tool. Create an attribute that just adds a no-op #[unsafe_signoff(reason = "")] and validates that syntax, then use syn or rust-analyzer to crawl the directory and analyze unsafe blocks with/without the attribute.

Likely, though, this would require the tool to also recognize the attribute for scopes (i.e. #![unsafe_signoff]) to be useful, as unsafe used to build a safe abstraction is all over the module (and often doesn’t need unsafe when it’s about field invariants!).

Also, attributes on expressions aren’t stable yet, so some contortion of code for unsafe at the statement level rather than tighter on the expression would be required.

I’m all for tools to make auditing of unsafe easier. But I don’t think forcing one convention is necessarily the best path. Rather, make and mature more APIs that remove the need to use unsafe anywhere other than implementation of these helper libraries, FFI, and research data structure development.

6

I agree with you, thanks for also adding additional detail.

The proposal is in light of the actix-web debacle that ended in the main developer quitting, it started with this blog post: https://64.github.io/actix/ and ended with https://twitter.com/fafhrd91/status/1151628328266285057

Nudging the ecosystem in the right direction, without “unsafe shaming” :wink: on important pieces of infrastructure seems like a better long term strategy, a lot of developers take proud in publishing crates without warnings and I almost see my proposal as being in the same spirit as Add must_use annotations to Result::is_ok and is_err #59648

These signals do add up over time, that’s why the example I just referenced was such a good idea.

2 Likes
7

That blog post was rather sensationalistic and in somewhat poor taste, IMO. It also failed to take into account more subtle details like the fact that (AFAIK) the actix-web developer is not a native English speaker, and thus both might interpret things others say in ways they weren't meant, and also possibly respond in ways that seem more abrasive than intended.

I was extremely disappointed that it was so heavily upvoted on Reddit. People would have (and will still, presumably) happily kept using actix-web forever for all the clear reasons there are to do so without a peep, were that blog post never uploaded.

Nobody found an actual problem with an actual negative effect on the end user independently, they just read an opinion piece that implied a problem and took it at face value.

Also, consider the general fact (which is a fact that perhaps people might not even be widely aware of) that the actix-web author is a senior engineer at Microsoft, and that Microsoft is known to use actix-web internally in parts of the Azure infrastructure (something I can't imagine they'd do if it was Actually That Bad.) The guy isn't just some random amateur Rust programmer.

5 Likes
8

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.