Remove "unnecessary unsafe block in unsafe fn" lint

[LeSeulArtichaut]

Closes #69173.

r? @eddyb
cc @Centril

[Centril]

cc also @arielb1

[ogoffart]

Consider this function:

unsafe fn bar() {
    something_unsafe();
    unsafe { foo(); } 
}

Now, when reading this snippet, it seems that foo is the only unsafe part.
In my opinion, we should still show the warning if there is any unsafe code outside of the unsafe block.

[Centril]

@ogoffart I don't think we should complicate the linting logic here and make it more conditional. What I think we should do, at a later point, when people have had a change to adapt (so that we don't do a complete 180 over night), is to introduce a lint in the other direction: you get a warning if you put unsafe operations directly in the unsafe function. This achieves the desired net effect.

---

Seems like @eddyb is too busy atm, so let's try r? @pnkfelix

[Mark-Simulacrum]

Seems like we should implement that lint as soon as we can, but as allow by default, to allow experimentation (e.g. liballoc's btree code could really use it).

[LeSeulArtichaut]

@Mark-Simulacrum Do you want me to implement that lint in this PR, or it could be added as a future PR? And if you choose the latter, should I amend this PR description to not close #69173?

[Centril]

@LeSeulArtichaut I'm writing up a separate issue for this, which is orthogonal to the PR at hand.

[Centril]

I've filed #69270 for the other lint.

[Mark-Simulacrum]

I am personally uncertain that we should be changing the existing lint, or at least, I would want to gather wider feedback before making that kind of change -- at least FCP. I personally think we should have a migration path in place before making changes here, and that migration path (AFAIK) is the new lint, even if it is allow by default. But of course this is just my opinion and if lang feels otherwise...

I would not, at least for now, change this PR while we have this discussion.

[Centril]

Let's kick of FCP to land this lint and the rough plan discussed in the language team (which everyone attending in the meeting agreed with) as outlined in rust-lang/rfcs#2585 (comment).

(To clarify, what is proposed:

• Accept this PR.
• Add an allow-by-default lint in the style of [lint: unsafe_in_unsafe_fn] A lint which triggers on unsafe operations in unsafe fn #69270.
• Eventually transition the new lint in [lint: unsafe_in_unsafe_fn] A lint which triggers on unsafe operations in unsafe fn #69270 to warn-by-default.

)

@rfcbot merge

[rfcbot]

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Centril
• @cramertj
• @eddyb
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @withoutboats

Concerns:

• should-warn-on-unsafe-code-outside-of-previously-flagged-unsafe-block (Remove "unnecessary unsafe block in unsafe fn" lint #69245 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[ogoffart]

I do not think this is orthogonal.

The lint was there for a reason, i suppose: un-needed unsafe blocks make it look like the code outside of them is now safe. So removing it for every case would be bad.

I'm suggesting this:

unsafe fn bar1() {
    something_safe();
    unsafe { // OK: remove the lint as we want to allow this in the future
       something_unsafe();
    }
}

unsafe fn bar2() {
    something_unsafe(); // No lint there yet
    unsafe { // LINT: Keep the `unused_unsafe` if there is something unsafe outside 
       something_unsafe();
    }
}

unsafe fn bar3() {
    something_unsafe(); // No lint there yet, this would be the future `unsafe_in_unsafe_fn`
}

That is, I think we should still keep the warning in the unused_unsafe bar2 function.

I guess when #69270 (unsafe_in_unsafe_fn lint) will be implemented, and if it is enabled, we might want to avoid the double warnings in bar2 (although even the two lints wouldn't hurt)

@ogoffart I don't think we should complicate the linting logic here and make it more conditional. What I think we should do, at a later point, when people have had a change to adapt (so that we don't do a complete 180 over night),

Fair enough, I haven't checked how complicated is the logic of what I am suggesting is.
I understand the "don't do a 180 over night" argument, but removing the lint completely allow the 90° route (writing function like bar2 which is strictly worse than both bar1 and bar3)
(i.e: I completely agree there should be a transition period during which both bar1 and bar3 are allowed. But bar2 should probably never be allowed, even during this transition period).

[Mark-Simulacrum]

To clarify my position a bit -- having reread the summary from the RFC thread:

I am unclear why making this allow-by-default (or removing the lint entirely, as is done in this PR) would help ease the transition to a new lint which makes these unsafe blocks required. Presumably, current code does not have these unsafe blocks (or at least, if it does, then it's already allowing the lint, so there's no point in making it allow-by-default). Code written after this lands is also unlikely to have unsafe blocks -- I at least don't expect most people to think this is an unsafe operation, let's add an unsafe block for function calls, pointer deref, etc. when inside an unsafe function. Maybe that expectation is flawed.

I feel like removing the lint / making it allow by default without an immediate migration step of "here's how to get all the unsafe blocks you want" makes for an odd position, where while we're not actively pushing towards either direction, I would expect the common practice to still be "omit all double-wrapping" unsafe blocks, just because the code would not be any easier to audit with them, you still need to check every single operation AFAICT.

If, however, we had the new lint in place, even if for now that lint is allow by default, then the migration plan is clear to me:

• add #![deny(missing_inner_unsafe)] as appropriate to crate roots, etc.
  • this has the effect of this PR, as well as linting against the old expectation of "one unsafe"

This new lint approach is even backwards compatible, as on stable today you'd just get "unknown lint" warning, no errors (well, I guess you'd likely want #![allow(unnecessary_unsafe)] or something, though perhaps not).

[Centril]

I think we are pushing clearly in one direction (making #69270 warn by default, eventually). Doing it incrementally is about implementation and ecosystem staging. That is, the missing_inner_unsafe you reference is the same as the aforementioned and linked issue (under a slightly different name), but we don't have to do it in the same PR. It's fine if we do it next week, for example. (Notice also that the FCP is for the rough plan to add the aforementioned other lint, in addition to this PR's change.)

The idea is that we start with removing the active nudge away from inner unsafe { ... }s in unsafe fns. This starts to shift the mental model away from the current one, which many (e.g. as evidenced by discussion in the RFC) have found to be counter-intuitive (so there is some evidence that people would add unsafe { ... } blocks though the language doesn't require it, if not for the current lint). Roughly at the same time, we start introducing the lint in #69270 as allow-by-default, allowing e.g. the standard library to change its policy. Eventually, we crank this lint up to warn-by-default. At that point, we may consider making that lint deny-by-default and possibly an error in a future edition (though we have not established language team consensus for this.)

[Mark-Simulacrum]

Notice also that the FCP is for the rough plan to add the aforementioned other lint, in addition to this PR's change.

This was not clear to me. Could you edit the FCP initiation comment to specifically note this? (I would do it, but I don't feel comfortable doing that with another team's FCPs :)

---

I disagree that there are benefits to this PR without an accompanying missing_inner_unsafe lint. I guess, in some sense, I can't come up with concrete examples of old code that would change (apparent, if not semantic) meaning. But, if I'm copying code around, then I can easily introduce a confusing situation.

unsafe fn parent() {
    ...
    unsafe { foo(); } // copied from some other function
    bar(); // pre-existing
    ...
}

In the situation described above, I copy in some code from some other function that's calling foo, an unsafe function. Now, the reviewer (or some future reader) comes along, and thinks that bar() is safe as the unsafe block above indicates to them that they are currently in a safe scope (otherwise the lint would fire). Edit: note that such a reader/reviewer may not be aware of this PR! In the post-missing_inner_unsafe world, they would also be fine, as we'd expect to have unsafe blocks around both the foo and bar calls..

Now, I will acknowledge that this example is somewhat contrived and perhaps would not occur in practice -- especially if the new lint is landed and promoted to at least warning quite quickly -- but it does seem plausible.

[nikomatsakis]

I agree this step makes no sense unless there is general agreement that we wish to encourage folks (and perhaps eventually require them) to use unsafe blocks inside of unsafe functions. I personally do wish to encourage this -- I think not doing so before was a mistake (for reasons I can enumerate, but won't in this comment). In that case, I definitely think it makes sense to disable lints that actively move people to remove unsafe blocks, given that we want them to eventually add them.

I guess @Mark-Simulacrum you are arguing that it would be better to swap the lint "atomically", i.e., go in one step from linting against unsafe blocks towards linting to have people add them?

[nikomatsakis]

@rfcbot reviewed

[cramertj]

I guess @Mark-Simulacrum you are arguing that it would be better to swap the lint "atomically", i.e., go in one step from linting against unsafe blocks towards linting to have people add them?

It does seem like it's unfortunate to have an intermediate state in which folks have some unsafe blocks in their unsafe functions, but those blocks do not surround all unsafe calls. Perhaps we should immediately start linting on unsafe calls outside of unsafe blocks if you have formerly-unnecessary unsafe blocks. This shouldn't cause any warnings on code that didn't have warnings before (swapping one warning for another), and would in some cases unlock the desired end behavior on a speedier timeline.

[joshtriplett]

On Mon, Feb 24, 2020 at 10:15:36AM -0800, Taylor Cramer wrote: > I guess @Mark-Simulacrum you are arguing that it would be better to swap the lint "atomically", i.e., go in one step from linting against unsafe blocks towards linting to have people add them? It does seem like it's unfortunate to have an intermediate state in which folks have some unsafe blocks in their unsafe functions, but those blocks do not surround all unsafe calls. Perhaps we should immediately start linting on `unsafe` calls outside of `unsafe` blocks if you have formerly-unnecessary unsafe blocks. This shouldn't cause any warnings on code that didn't have warnings before (swapping one warning for another), and will in some cases unlock the desired end behavior on a speedier timeline.

I like this plan. If you have *no* unsafe blocks in your unsafe function, provide a grace period where we don't lint. If you have *any* unsafe blocks in your unsafe function, we can immediately start ensuring that you have the *right* unsafe blocks.

[Mark-Simulacrum]

@cramertj's proposal in #69245 (comment) seems okay to me. It's a slightly more granular proposal than I had in mind, but it also satisfies the "atomic transition" plan but on a per-function basis and in an automated fashion. I think this might be worse.

In particular, I am still concerned that folks will have trouble identifying whether a function is "new" or "old" -- i.e., if I see no unsafe blocks in my unsafe function, is that function body entirely unsafe (with lots of unsafe calls) or entirely safe code? That's something that @cramertj's proposal, AFAICT, does not solve, in the sense that when transitioning you'd almost want to add a let () = unsafe { () }; to every unsafe function or something like that just to opt-in to the new behavior. With that in mind, I still lean towards a lint that enables the new functionality (and most crates would likely want to opt-in at the root).

[cramertj]

if I see no unsafe blocks in my unsafe function, is that function body entirely unsafe (with lots of unsafe calls) or entirely safe code? That's something that @cramertj's proposal, AFAICT, does not solve, in the sense that when transitioning you'd almost want to add a let () = unsafe { () }; to every unsafe function or something like that just to opt-in to the new behavior.

Presumably you'd set #![deny(the_new_at_first_allow_by_default_warning)] instead.

[nikomatsakis]

I'm ok with that plan, although the "add one thing and then you're in a new mode" is a pattern we generally try to avoid and I wouldn't want to land there long term (I think it could well be confusing to folks even in the short term). I mean maybe we should just adopt this RFC and just start the warnings immediately. =) I would personally be in favor of that, though I think we would also want to do a nice blog post on why we are changing the behavior (and emphasize that this will only be new warnings).

[pnkfelix]

@rfcbot concern should-warn-on-unsafe-code-outside-of-previously-flagged-unsafe-block

I just want to formalize the concern, initially denoted bar2 in @ogoffart 's comment above, and the proposed plan to address it as described in @cramertj 's comment

(At least, I think what is being discussed is blocking a merge of this PR until that step is taken. Which I approve of, which is why I'm willing to have my name on the formal concern.)

[scottmcm]

@rfcbot reviewed

I'm not certain that it's always the best choice to require an unsafe block to call unsafe functions from an unsafe function, but the plan is still linting, so I'm good with the proposed plan.

(I do also think the swap-the-warnings plan sounds interesting, but felix's concern will drive that conversation so I'm not going to wait on my box for that to finish.)

[Mark-Simulacrum]

We discussed this in the lang team meeting.

There were a couple of options considered as to the exact transition plan we should follow, though everyone present seemed to agree that the end state we want is some version of the new lint which requires unsafe blocks within unsafe functions for unsafe operations.

There are three possible plans:

1. Remove unnecessary_unsafe linting now, and at some point start phasing in the new lint.
2. Add a allow-by-default lint requiring unsafe blocks in unsafe functions.
   • We would expect code to do something like #![allow(unnecessary_unsafe)] #![deny(unsafe_operations_in_unsafe_functions)] to opt-in to the new behavior
3. Opt-in to the new behavior automatically if an unsafe block is added to an unsafe function.

There was relatively broad agreement that (3) is not really a good idea, primarily based on past experience. This was most prominently raised by Niko. The reasoning is that it's hard to figure out if you've opted in or not, especially in longer unsafe functions, and doing so on function-level granularity is not great. It also means that there's not a good way to opt-in and then run something like cargo fix.

(1) faced strong opposition from Mark, at least, though based on this thread it seems like @pnkfelix also agrees with this point. This is mostly due to the in-between point of not having either lint may lead to confusion, in particular see #69245 (comment) for an example of a confusing situation.

I believe there was broad agreement that the proposal in (2) is the way forward. To elaborate a bit, we have the following plan:

• Implement the new functionality as a lint, allow-by-default.
  • Ideally, though plausibly not in the initial implementation, provide machine-applicable suggestions to add unsafe blocks at the finest-grained level possible to functions.
• Announce the new functionality on inside rust, possibly in the release post when it lands in stable
  • Suggest that users try it out
• Switch the old lint off and the new lint on as deny-by-default at the edition boundary

Of course, the above plan is likely to be changed around a bit -- for one, the edition is quite far away -- and we may want to start phasing in the new lint to code somehow over time (though I am unclear on how we would do so other than to just set it to deny by default earlier than the edition).

I believe that if the above plan is a correct summary, the appropriate thing to do is to close this PR. It's a bit unclear to me if the existing unused_unsafe lint is broader than exactly what this PR implements; if so, we would likely want to split out the specific "I have an unsafe block in an unsafe function" into a separate lint under the unused_unsafe group(?).

Then, either @LeSeulArtichaut or someone else, should implement the new lint at the allow-by-default level in a separate PR.

---

We did not decide how we should approve the new plan. It is likely sufficiently different from the original proposal that I would lean towards canceling FCP here, filing an issue with the above summary (though perhaps stripped down) and proposing FCP again there with this concrete proposal in mind, specifically approving step 1 (the new lint). OTOH, it seems like an allow by default lint is not something that we necessarily need full consensus on :)

[RalfJung]

I generally agree with the plan, except:

#![allow(unnecessary_unsafe)] #![deny(unsafe_operations_in_unsafe_functions)]

One problem with this is that it disables unnecessary_unsafe everywhere, while the lint could otherwise still be useful outside of unsafe fn.

[Mark-Simulacrum]

Hm, yes, that's a good point. I think we'd then want to split the functionality up into a sublint, or, if possible, selectively disable bits of the unnecessary_unsafe lint if unsafe_operations is in a warn or higher state.

[withoutboats]

I wasn't in any of the meetings that discussed this, so sorry to burst in with somewhat heterodox thoughts.

Basically, I think we should ultimately support both styles of annotating unsafe code - the one we have today, and one in which a user annotates every unsafe operation with its own marker (presumably commented with a description of how the invariant is upheld). I think there are some users for whom requiring the second will just be frustrating without benefit, leading them to just put their entire body in an unsafe block - strictly worse than today. And I think we should trust these users that decide that they don't need this extra due dilligence for this use case.

So I'd like to consider a way the "transition" option could be the final option - either you annotate everything or nothing, but both are somehow allowed. EDIT: I guess if this is just a lint, users who are satisfied with the current annotation system can just allow the new lint. That's maybe sufficient to support both use cases.

Additionally, I think if we move forward with this we should reconsider the decision on allowing unsafe as a modifier to any expression, instead of always a block, to avoid syntactic noise once you have many small unsafe annotations and encourage extremely narrow unsafe scopes with specific descriptions of their annotation (the best way to use this feature, IMO).

[nikomatsakis]

So I'd like to consider a way the "transition" option could be the final option - either you annotate everything or nothing, but both are somehow allowed. EDIT: I guess if this is just a lint, users who are satisfied with the current annotation system can just allow the new lint. That's maybe sufficient to support both use cases.

I remain somewhat skeptical of this choice, for two reasons.

First, I think it is confusing: you add one unsafe block in a function, and suddenly you're getting warnings from unrelated lines of code. In the (distant) past we had more options like this (e.g., all things were pub until you labeled one thing pub, etc) and I remember it being quite confusing.

But secondly, I don't think it will serve those users that wish to have finer-grained unsafe blocks very well. In particular, it would be very easy to overlook that an unsafe function is being invoked or that an unsafe block is required, and since the compiler won't be giving you any warnings, you will never know that you were missing unsafe blocks. When I'm writing unsafe code, at least, a big part of my style revolves around labeling unsafe blocks with comments and things to document my reasoning, so I think that having the compiler fail to notify me when an unsafe block is missing is kind of a big deal.

In our most recent meeting, we were saying that perhaps a lint-based solution is actually just the best solution here. In particular, if we have a lint for "unsafe block operation in unsafe fn" that is warn-by-default, then people can make it allow-by-default if they prefer (for example, around a module with a lot of external FFI calls). I'm curious what you think of that approach, @withoutboats?

My main hesitation here is that I've observed that people often feel bad silencing lints in their code, which seems somewhat unfortunate, as indeed the reason things are lints is often that there are some good scenarios where it makes sense to silence the warning (and others where it does not).

[Ixrec]

My main hesitation here is that I've observed that people often feel bad silencing lints in their code, which seems somewhat unfortunate, as indeed the reason things are lints is often that there are some good scenarios where it makes sense to silence the warning (and others where it does not).

I feel like if this is a concern then we can just add wording to the lint message that says we expect you to just silence it if you're in the kind of code where this advice is not applicable.

Plus, from what I hear, FFI code already tends to involve more lint silencing than typical Rust code, e.g. #[allow(nonstandard_style)] because the foreign function names aren't snake case or whatever.

[bors]

☔ The latest upstream changes (presumably #70118) made this pull request unmergeable. Please resolve the merge conflicts.

[rust-highfive]

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2020-03-19T14:53:32.7688666Z ========================== Starting Command Output ===========================
2020-03-19T14:53:32.7692966Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/f243df80-ffed-4df2-b238-58ac69c268c2.sh
2020-03-19T14:53:32.7693458Z 
2020-03-19T14:53:32.7697971Z ##[section]Finishing: Disable git automatic line ending conversion
2020-03-19T14:53:32.7719786Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69245/merge to s
2020-03-19T14:53:32.7723499Z Task         : Get sources
2020-03-19T14:53:32.7724296Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-03-19T14:53:32.7724652Z Version      : 1.0.0
2020-03-19T14:53:32.7724892Z Author       : Microsoft
---
2020-03-19T14:53:34.1022142Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-03-19T14:53:34.1027990Z ##[command]git config gc.auto 0
2020-03-19T14:53:34.1030782Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-03-19T14:53:34.1033253Z ##[command]git config --get-all http.proxy
2020-03-19T14:53:34.1040049Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/69245/merge:refs/remotes/pull/69245/merge
---
2020-03-19T15:00:56.3329266Z 
2020-03-19T15:00:56.3501324Z error: could not compile `rustc_mir`.
2020-03-19T15:00:56.3501564Z 
2020-03-19T15:00:56.3501915Z To learn more, run the command again with --verbose.
2020-03-19T15:00:56.3520474Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-03-19T15:00:56.3534240Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2020-03-19T15:00:56.3534532Z Build completed unsuccessfully in 0:03:39
2020-03-19T15:00:56.3578696Z == clock drift check ==
2020-03-19T15:00:56.3592570Z   local time: Thu Mar 19 15:00:56 UTC 2020
2020-03-19T15:00:56.3592570Z   local time: Thu Mar 19 15:00:56 UTC 2020
2020-03-19T15:00:56.4611606Z   network time: Thu, 19 Mar 2020 15:00:56 GMT
2020-03-19T15:00:57.7248693Z == end clock drift check ==
2020-03-19T15:00:57.7257560Z 
2020-03-19T15:00:57.7293528Z ##[error]Bash exited with code '1'.
2020-03-19T15:00:57.7308068Z ##[section]Finishing: Run build
2020-03-19T15:00:57.7374557Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69245/merge to s
2020-03-19T15:00:57.7378963Z Task         : Get sources
2020-03-19T15:00:57.7379267Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-03-19T15:00:57.7379533Z Version      : 1.0.0
2020-03-19T15:00:57.7379726Z Author       : Microsoft
2020-03-19T15:00:57.7379726Z Author       : Microsoft
2020-03-19T15:00:57.7380034Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-03-19T15:00:57.7380374Z ==============================================================================
2020-03-19T15:00:58.0287361Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-03-19T15:00:58.0329007Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/69245/merge to s
2020-03-19T15:00:58.0406951Z Cleaning up task key
2020-03-19T15:00:58.0408190Z Start cleaning up orphan processes.
2020-03-19T15:00:58.0557268Z Terminate orphan process: pid (3543) (python)
2020-03-19T15:00:58.0898524Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

[Centril]

@rfcbot cancel

[rfcbot]

@Centril proposal cancelled.

[Mark-Simulacrum]

We discussed this in the language team meeting today, and (like before, I think) felt that the current state of this PR is not something that the language team wants to accept. The FCP also started with a different -- and somewhat unclear -- goal in mind, so we wanted to restart it if we do decide to proceed here.

Specifically, we identified the following next steps (for this area):

• Implement the allow-by-default lint and appropriate toggles for the current lint which would permit us to transition into the (potential) future of unsafe code with very granular annotations.
• At some point in the future, likely in the next edition, we want to discuss whether to change the defaults here (and how to do so). @nikomatsakis in particular felt that perhaps we're moving too slowly. I brought up (and I think @pnkfelix agreed) that seeing how the lint works inside std/the compiler would be worthwhile, and would help us decide how exactly to move forward by providing valuable data (on both the transition and how helpful the lint is).

With those steps in mind, and given that the discussion on this PR is quite long (and complex), I'm going to close it. We would love to see @LeSeulArtichaut (or someone else) write an implementation for the proposed allow-by-default lint; I think @Centril has put some thought into how to structure it.

I also felt that given the new plan, the lint itself may not need language team FCP, especially if we land it initially unstable or so. Regardless, we should get an implementation before trying to FCP it I think :)

Thanks everyone! I believe there's a tracking issue already so we don't need to open a new one with the closing of this PR.

[RalfJung]

I updated the RFC to discuss the new proposed lint and the interaction with the "unnecessary unsafe" lint. See rust-lang/rfcs#2585 (comment).