We can't find the internet
Attempting to reconnect
Something went wrong!
Attempting to reconnect
← The Expert Edge: Idiom, Review and Capstones step 11 of 14
Unsound API smells: the patterns that are always wrong
Two functions in this Registry are unsound. Restructure them so the
compiler and clippy accept the design, without changing what any input
produces.
pub fn fix_the_api(ops: Vec<String>) -> Vec<i64>
The starter compiles as a program and passes every test. It fails the lint gate on two deny-by-default correctness lints — the small set clippy treats as bugs rather than opinions — and neither can be satisfied by any unsound shape. That is what makes this gradeable: you cannot pass by tweaking a message, only by changing the design.
Smell 1: fn get(&self) -> &mut T
pub fn slot(&self, i: usize) -> &mut i64
clippy: mutable borrow from immutable input(s) (clippy::mut_from_ref,
deny).
::: question A shared reference and an exclusive one to the same i64. What
actually goes wrong?
Everything, and not only in the obvious way.
&T means shared and immutable, and rustc does not merely trust that —
it tells LLVM, by putting noalias and readonly on the parameter. The
optimiser is then entitled to cache the value in a register across a call, to
reorder reads around writes, and to assume two &i64s that came from
different places cannot be the same place. Hand out a &mut derived from a
& and every one of those assumptions is false, so the miscompilation is not
a bug in the optimiser: you lied.
And it is trivially reachable from safe code:
let a = reg.slot(0);
let b = reg.slot(0); // two &mut to the same i64, no unsafe in sight
The fix is a guard type, and it is what every interior-mutability type in
std already does. RefCell::borrow_mut returns a RefMut, not a &mut T.
Mutex::lock returns a MutexGuard. Vec::get_mut takes &mut self. Here,
return Option<Slot<'_>> from &mut self: the borrow checker then guarantees
the index was valid and that only one slot borrow is alive, and both facts
are checked by the compiler rather than asserted in a comment.
Note what the guard buys you beyond soundness — the Option makes the
out-of-range case a value rather than a precondition, so callers stop needing
to know the length before they can call.
:::
Smell 2: a safe pub fn that dereferences a raw-pointer argument
pub fn raw_sum(p: *const i64, n: usize) -> i64
clippy: this public function might dereference a raw pointer but is not marked unsafe (clippy::not_unsafe_ptr_arg_deref, deny).
A pub safe function is a promise that no input at all can cause
undefined behaviour. raw_sum(0x1 as *const i64, usize::MAX) is a call any
safe caller can write. The function cannot check the pointer — validity,
alignment and provenance are not observable at runtime — so the obligation
has to move to the caller, which is precisely what unsafe fn means.
Mark it pub unsafe fn, write the # Safety section saying what the caller
must guarantee (non-null, aligned, valid for n initialised reads, not
mutated during the call), and then call it from total() inside an unsafe
block with a // SAFETY: comment explaining why those particular arguments
satisfy it. Two comments, both load-bearing: one states the contract, the
other discharges it.
The catalogue
Eight shapes that are always worth stopping on in review. The first two are above; the rest are here so you recognise them.
-
fn get(&self) -> &mut T— deny (mut_from_ref). -
A safe
pub fndereferencing a raw-pointer argument — deny (not_unsafe_ptr_arg_deref). -
Writing through a pointer derived from a shared reference — rustc’s own
invalid_reference_casting, deny. -
transmute::<&T, &mut T>— rustc’smutable_transmutes, deny. Same bug as (1), spelled so it does not even look like a function signature. -
A
pub unsafe fnwith no# Safetysection —missing_safety_doc, warn. You imposed an obligation and did not say what it was. -
A safe function whose soundness depends on its destructor running.
mem::forgetis safe,Rccycles leak, and a panic during unwinding can skip a drop. No lint. You have to know this one. -
A
pubfield, or a#[derive(Clone)], on a type whoseunsafeinternals depend on it. DerivingCloneon a handle that owns a raw pointer gives you a double free from safe code; apub lenfield lets a caller lie about how much is initialised. No lint exists for this, and that is worth saying out loud — it is the most common way a correct-looking unsafe abstraction becomes unsound six months later, in a commit that only touched a derive. -
Unsafe hidden behind a macro.
clippy::macro_metavars_in_unsafecatches the exported-macro case — amacro_rules!that expands a caller-supplied expression inside anunsafeblock, letting callers write unsafe code without typing the word.clippy::unsafe_removed_from_namecatches the import-site version:use std::cell::UnsafeCell as Cell;makes the danger invisible at every use.
The rule about #[allow]
The original of this file carried #[allow(clippy::mut_from_ref)] and
#[allow(clippy::not_unsafe_ptr_arg_deref)]. They have been removed for you,
and putting them back is not a fix.
A #[allow] on a correctness lint is a claim that clippy is wrong about this
case. For the style and pedantic groups that claim is often true and an
#[allow] with a one-line justification is a normal professional outcome. For
the correctness group it is almost never true — these lints exist because the
shape they match is unsound in general, and “but my callers are careful” is
not a property the compiler, the optimiser, or the next maintainer can rely
on.
Nothing in this harness can see whether you added an #[allow]. A reviewer
can, in one grep, and that is the audience this item is training you for.
Constraint
Every test asserts the existing output, sentinels and all: -1001 for an
index or number that will not parse, -1000 for an unknown command. This is a
refactor with a proof attached, not a rewrite.
Stuck?
Rust reference solution
Sign in to attempt this problem and reveal the reference solution.