unused_unsafe: stop interpreting unsafe fns as unsafe contexts

[Centril]

In other words, the following will result in a lint being emitted today:

unsafe fn _bar() {}

unsafe fn _foo() {
    unsafe {
        _bar();
    }
}

==>

warning: unnecessary `unsafe` block
 --> src/lib.rs:4:5
  |
3 | unsafe fn _foo() {
  | ---------------- because it's nested under this `unsafe` fn
4 |     unsafe {
  |     ^^^^^^ unnecessary `unsafe` block
  |
  = note: `#[warn(unused_unsafe)]` on by default

Based on the discussion in a recent language team meeting (see summary as outlined by @nikomatsakis in rust-lang/rfcs#2585 (comment)), we would like to stop emitting the lint in this case where we have unsafe operations inside an unsafe { ... } inside an unsafe fn (it's probably easiest to not emit the lint at all as opposed to emitting a different lint name, but this can be determined as part of the implementation).

cc @RalfJung @rust-lang/lang

This issue has been assigned to @LeSeulArtichaut via this comment.

[Centril]

cc @oli-obk @eddyb for mentoring instructions?

[eddyb]

So looking at the code, it seems that the outermost active unsafe block (or unsafe fn) is used:

rust/src/librustc_mir_build/build/block.rs

Lines 212 to 216 in dbef353

	match self.unpushed_unsafe {
	Safety::Safe => {}
	_ => return,
	}
	self.unpushed_unsafe = Safety::ExplicitUnsafe(hir_id);

(NB: very confusing terminology around unpushed_unsafe, AFAICT it means "unsafe unrelated to {Push,Pop}Unsafe", and the latter is a hacky way to do allow_internal_unsafe, so we should rip it out sooner or later - but it shouldn't impact this issue, it's just confusing)

This are the possible values for Safety:

rust/src/librustc/mir/mod.rs

Lines 380 to 388 in dbef353

	pub enum Safety {
	Safe,
	/// Unsafe because of a PushUnsafeBlock
	BuiltinUnsafe,
	/// Unsafe because of an unsafe fn
	FnUnsafe,
	/// Unsafe because of an `unsafe` block
	ExplicitUnsafe(hir::HirId),
	}

So when the "current safety" is Safety::FnUnsafe, it early-returns, instead of replacing it with the inner Safety::ExplicitUnsafe.

By making this change, I think we can get the desired lint effect (while unsafe fn still allowing unsafe operations outside of unsafe blocks):

- Safety::Safe => {}
+ Safety::Safe | Safety::FnUnsafe => {}

[LeSeulArtichaut]

I’m currently trying to tackle this.
@rustbot claim

[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).

[kevinmehall]

This can probably be closed in favor of the RFC tracking issue #71668. Merging the change proposed here alone was rejected in #69245, but an allow-by-default lint was merged in #71862.

[RalfJung]

@kevinmehall agreed, thanks.