lang (syntax | unsafe | typesystem | effects)
unsafe_block_in_unsafe_fnNo longer treat the body of an unsafe fn as being an unsafe block. To avoid
a breaking change, this is a warning now and may become an error in a future
edition.
Marking a function as unsafe is one of Rust's key protections against
undefined behavior: Even if the programmer does not read the documentation,
calling an unsafe function (or performing another unsafe operation) outside an
unsafe block will lead to a compile error, hopefully followed by reading the
documentation.
However, we currently entirely lose this protection when writing an unsafe fn:
If I, say, accidentally call offset instead of wrapping_offset, or if I
dereference a raw pointer thinking it is a reference, this happens without any
further notice when I am writing an unsafe fn because the body of an unsafe fn is treated as an unsafe block.
For example, notice how
this PR significantly
increased the amount of code in the thread spawning function that is considered
to be inside an unsafe block.
The original justification for this behavior (according to my understanding) was
that calling this function is anyway unsafe, so there is no harm done in
allowing it to perform unsafe operations. And indeed the current situation
does provide the guarantee that a program without unsafe cannot be UB.
However, this neglects the other aspect of unsafe that I described above: To
make the programmer aware that they are treading dangerous ground even when they
may not realize they are doing so.
In fact, this double role of unsafe in unsafe fn (making it both unsafe to
call and enabling it to call other unsafe operations) conflates the two dual
roles that unsafe plays in Rust. On the one hand, there are places that
define a proof obligation, these make things "unsafe to call/do" (e.g., the
language definition says that dereferencing a raw pointer requires it not to be
dangling). On the other hand, there are places that discharge the proof
obligation, these are "unsafe blocks of code" (e.g., unsafe code that
dereferences a raw pointer has to locally argue why it cannot be dangling).
unsafe {} blocks are about discharging obligations, but unsafe fn are
about defining obligations. The fact that the body of an unsafe fn is also
implicitly treated like a block has made it hard to realize this duality
even for experienced Rust developers. (Completing the picture,
unsafe Trait also defines an obligation, that is discharged by unsafe impl.
Curiously, unsafe trait does not implicitly make all bodies of default
functions defined inside this trait unsafe blocks, which is somewhat
inconsistent with unsafe fn when viewed through this lens.)
The unsafe keyword in Rust serves two related purposes.
When you perform an "unsafe to call" operation, like dereferencing a raw pointer
or calling an unsafe fn, you must enclose that code in an unsafe {} block.
The purpose of this is to acknowledge that the operation you are performing here
has not been checked by the compiler, you are responsible yourself for
upholding Rust's safety guarantees. Generally, unsafe operations come with
detailed documentation for the conditions that must be met when this operation
is executed; it is up to you to check that all these conditions are indeed met.
When you are writing a function that itself has additional conditions to ensure
safety (say, it accesses some data without making some necessary bounds checks,
or it takes some raw pointers as arguments and performs memory operations based
on them), then you should mark this as an unsafe fn and it is up to you to
document the conditions that must be met for the arguments. This use of the
unsafe keyword makes your function itself "unsafe to call".
The same duality can be observed in traits: unsafe trait is like unsafe fn;
it makes implementing this trait an "unsafe to call" operation and it is up to
whoever defines the trait to precisely document what is unsafe about it.
unsafe impl is like unsafe {}, it acknowledges that there are extra
requirements here that are not checked by the compiler and that the programmer
is responsible to uphold.
For this reason, "unsafe to call" operations inside an unsafe fn must be
contained inside an unsafe {} block like everywhere else. The author of these
functions has to ensure that the requirements of the operation are upheld. To
this end, the author may of course assume that the caller of the unsafe fn in
turn uphold their own requirements.
For backwards compatibility reasons, this unsafety check inside unsafe fn is
controlled by a lint, unsafe_op_in_unsafe_fn. By setting
#[deny(unsafe_op_in_unsafe_fn)], the compiler is as strict about unsafe
operations inside unsafe fn as it is everywhere else.
This lint is allow-by-default initially, and will be warn-by-default across all editions eventually. In future editions, it may become deny-by-default, or even a hard error.
The new unsafe_op_in_unsafe_fn lint triggers when an unsafe operation is used
inside an unsafe fn but outside unsafe {} blocks. So, the following will
emit a warning:
#[warn(unsafe_op_in_unsafe_fn)]
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
x.get_unchecked(i)
}
Moreover, if and only if the unsafe_op_in_unsafe_fn lint is not allowed, we
no longer warn that an unsafe block is unnecessary when it is nested
immediately inside an unsafe fn. So, the following compiles without any
warning:
#[warn(unsafe_op_in_unsafe_fn)]
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
unsafe { x.get_unchecked(i) }
}
However, nested unsafe blocks are still redundant, so this warns:
#[warn(unsafe_op_in_unsafe_fn)]
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
unsafe { unsafe { x.get_unchecked(i) } }
}
Many unsafe fn are actually rather short (no more than 3 lines) and will end
up just being one large unsafe block. This change would make such functions
less ergonomic to write, they would likely become
unsafe fn foo(...) -> ... { unsafe {
// Code goes here
} }
To achieve the goals laid out in the motivation section, the proposed approach is least invasive in the sense that it avoids introducing new keywords, and instead relies on the existing lint mechanism to perform the transition.
One alternative always is to not do anything, and live with the current situation.
We could avoid using unsafe for dual purpose, and instead have unsafe_to_call fn for functions that are "unsafe to call" but do not implicitly have an
unsafe {} block in their body. For consistency, we might want unsafe_to_impl trait for traits, though the behavior would be the same as unsafe trait.
We could introduce named proof obligations (proposed by @Centril) such that the
compiler can be be told (to some extend) if the assumptions made by the unsafe fn are sufficient to discharge the requirements of the unsafe operations.
We could restrict this requirement to use unsafe blocks in unsafe fn to
those unsafe fn that contain at least one unsafe block, meaning short
unsafe fn would keep compiling like they do now.
And of course, the lint name is subject to bikeshedding.
The only other language that I am aware of that has a notion of unsafe blocks
and unsafe functions is C#. It
looks like
there, unsafe operations can be freely used inside an unsafe function even
without a further unsafe block. However, based on @Ixrec's experience,
unsafe plays hardly any role in the C# ecosystem and they do not have a
culture of thinking about this in terms of proof obligations.
What is the timeline for adding the lint, and cranking up its default level? Should the default level depend on the edition?
Should we ever make this deny-by-default or even a hard error, in a future edition?
Should we require cargo fix to be able to do something about this warning
before making it even warn-by-default? (We certainly need to do something
before making it deny-by-default or a hard error in a future edition.) cargo fix could add big unsafe {} blocks around the entire body of every unsafe fn. That would not improve the amount of care that is taken for unsafety in
the fixed code, but it would provide a way to the incrementally improve the big
functions, and new functions written later would have the appropriate amount of
care applied to them from the start. Potentially, rustfmt could be taught to
format unsafe blocks that wrap the entire function body in a way that avoids
double-indent. "function bodies as expressions" would enable a format like
unsafe fn foo() = unsafe { body }.
It is not entirely clear if having the behavior of one lint depend on another
will work (though most likely, it will). If it does not, we should try to find
some other mechanism to opt-in to the new treatment of unsafe fn bodies.