Skip to content

Commit ec454f2

Browse files
committed
update plan to include the lint
1 parent 949590b commit ec454f2

File tree

1 file changed

+72
-51
lines changed

1 file changed

+72
-51
lines changed

text/0000-unsafe-block-in-unsafe-fn.md

Lines changed: 72 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ unsafe blocks, which is somewhat inconsistent with `unsafe fn`.)
6060
# Guide-level explanation
6161
[guide-level-explanation]: #guide-level-explanation
6262

63-
When you perform an unsafe operation, like dereferencing a raw pointer or
64-
calling an `unsafe` function, you must enclose that code in an `unsafe` block.
63+
The `unsafe` keyword in Rust serves two related purposes.
64+
65+
When you perform an "unsafe to call" operation, like dereferencing a raw pointer
66+
or calling an `unsafe fn`, you must enclose that code in an `unsafe {}` block.
6567
The purpose of this is to acknowledge that the operation you are performing here
6668
has *not* been checked by the compiler, you are responsible yourself for
6769
upholding Rust's safety guarantees. Generally, unsafe operations come with
@@ -72,59 +74,72 @@ When you are writing a function that itself has additional conditions to ensure
7274
safety (say, it accesses some data without making some necessary bounds checks,
7375
or it takes some raw pointers as arguments and performs memory operations based
7476
on them), then you should mark this as an `unsafe fn` and it is up to you to
75-
document the conditions that must be met for the arguments.
76-
77-
Your `unsafe fn` will likely perform unsafe operations; these have to be
78-
enclosed by an `unsafe` block as usual. This is the place where you have to
79-
check that the requirements you documented for your own function are sufficient
80-
to satisfy the conditions required to perform this unsafe operation.
77+
document the conditions that must be met for the arguments. This use of the
78+
`unsafe` keyword makes your function itself "unsafe to call".
79+
80+
The same duality can be observed in traits: `unsafe trait` is like `unsafe fn`;
81+
it makes implementing this trait an "unsafe to call" operation and it is up to
82+
whoever defines the trait to precisely document what is unsafe about it.
83+
`unsafe impl` is like `unsafe {}`, it acknowledges that there are extra
84+
requirements here that are not checked by the compiler and that the programmer
85+
is responsible to uphold.
86+
87+
For this reason, "unsafe to call" operations inside an `unsafe fn` must be
88+
contained inside an `unsafe {}` block like everywhere else. The author of these
89+
functions has to ensure that the requirements of the operation are upheld. To
90+
this end, the author may of course assume that the caller of the `unsafe fn` in
91+
turn uphold their own requirements.
92+
93+
For backwards compatibility reasons, this unsafety check inside `unsafe fn` is
94+
controlled by a lint, `unsafe_op_in_unsafe_fn`. By setting
95+
`#[deny(unsafe_op_in_unsafe_fn)]`, the compiler is as strict about unsafe
96+
operations inside `unsafe fn` as it is everywhere else.
97+
98+
This lint is allow-by-default initially, and will be warn-by-default across all
99+
editions eventually. In future editions, it may become deny-by-default, or even
100+
a hard error.
81101

82102
# Reference-level explanation
83103
[reference-level-explanation]: #reference-level-explanation
84104

85-
1. First of all, we no longer warn that an `unsafe` block is unnecessary when it is
86-
nested immediately inside an `unsafe fn`. So, the following compiles without
87-
any warning:
88-
89-
```rust
90-
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
91-
unsafe { x.get_unchecked(i) }
92-
}
93-
```
105+
The new `unsafe_op_in_unsafe_fn` lint triggers when an unsafe operation is used
106+
inside an `unsafe fn` but outside `unsafe {}` blocks. So, the following will
107+
emit a warning:
94108

95-
However, nested `unsafe` blocks are still redundant, so this warns:
96-
97-
```rust
98-
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
99-
unsafe { unsafe { x.get_unchecked(i) } }
100-
}
101-
```
109+
```rust
110+
#[warn(unsafe_op_in_unsafe_fn)]
111+
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
112+
x.get_unchecked(i)
113+
}
114+
```
102115

103-
2. Optionally, we could add a clippy "correctness" lint to warn about unsafe
104-
operations inside an `unsafe fn`, but outside an `unsafe` block. So, this
105-
would trigger the lint:
116+
Moreover, if and only if the `unsafe_op_in_unsafe_fn` lint is not `allow`ed, we
117+
no longer warn that an `unsafe` block is unnecessary when it is nested
118+
immediately inside an `unsafe fn`. So, the following compiles without any
119+
warning:
106120

107-
```rust
108-
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
109-
x.get_unchecked(i)
110-
}
111-
```
121+
```rust
122+
#[warn(unsafe_op_in_unsafe_fn)]
123+
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
124+
unsafe { x.get_unchecked(i) }
125+
}
126+
```
112127

113-
3. In a next step, we move this lint to rustc proper, make it warn-by-default.
114-
This gets us into a state where programmers are much less likely to
115-
accidentally perform undesired unsafe operations inside `unsafe fn`.
128+
However, nested `unsafe` blocks are still redundant, so this warns:
116129

117-
4. Even later (in the 2021 edition?), it might be desirable to turn this
118-
warning into an error.
130+
```rust
131+
#[warn(unsafe_op_in_unsafe_fn)]
132+
unsafe fn get_unchecked<T>(x: &[T], i: usize) -> &T {
133+
unsafe { unsafe { x.get_unchecked(i) } }
134+
}
135+
```
119136

120137
# Drawbacks
121138
[drawbacks]: #drawbacks
122139

123-
This new warning will likely fire for the vast majority of `unsafe fn` out there.
124-
125-
Many `unsafe fn` are actually rather short (no more than 3 lines) and will
126-
likely end up just being one large `unsafe` block. This change would make such
127-
functions less ergonomic to write, they would likely become
140+
Many `unsafe fn` are actually rather short (no more than 3 lines) and will end
141+
up just being one large `unsafe` block. This change would make such functions
142+
less ergonomic to write, they would likely become
128143

129144
```rust
130145
unsafe fn foo(...) -> ... { unsafe {
@@ -165,14 +180,20 @@ culture of thinking about this in terms of proof obligations.
165180
# Unresolved questions
166181
[unresolved-questions]: #unresolved-questions
167182

168-
Should this lint be in clippy first before in becomes warn-by-default in rustc,
169-
to avoid a huge flood of warnings showing up at once? Should the lint ever
170-
become a hard error (on newer editions), or remain a warning indefinitely?
183+
What is the timeline for adding the lint, and cranking up its default level?
184+
Should the default level depend on the edition?
185+
186+
Should we ever make this deny-by-default or even a hard error, in a future
187+
edition?
171188

172189
Should we require `cargo fix` to be able to do *something* about this warning
173-
before making it warn-by-default? `cargo fix` could, for example, wrap the body
174-
of every `unsafe fn` in one big `unsafe` block. That would not improve the
175-
amount of care that is taken for unsafety in the fixed code, but it would
176-
provide a way to the incrementally improve the big functions, and new functions
177-
written later would have the appropriate amount of care applied to them from the
178-
start.
190+
before making it even warn-by-default? (We certainly need to do something
191+
before making it deny-by-default or a hard error in a future edition.) `cargo
192+
fix` could add big `unsafe {}` blocks around the entire body of every `unsafe
193+
fn`. That would not improve the amount of care that is taken for unsafety in
194+
the fixed code, but it would provide a way to the incrementally improve the big
195+
functions, and new functions written later would have the appropriate amount of
196+
care applied to them from the start. Potentially, `rustfmt` could be taught to
197+
format `unsafe` blocks that wrap the entire function body in a way that avoids
198+
double-indent. "function bodies as expressions" would enable a format like
199+
`unsafe fn foo() = unsafe { body }`.

0 commit comments

Comments
 (0)