Reading Rust You Did Not Write
Every earlier module in this course was about writing Rust. This one is about reading it — specifically, reading Rust that compiles, that looks reasonable at a glance, and that an AI assistant (or a rushed teammate) produced without necessarily thinking hard about the tradeoffs. Rust’s compiler rejects a huge class of bugs other languages let through, which creates a specific trap: code that compiles feels reviewed. It is not. The borrow checker proves your code has no data races and no dangling references. It proves nothing about whether the design is right, whether errors are handled sensibly, or whether a .clone() is covering up a design mistake instead of fixing one.
This lesson is the process. The rest of the module — the bug catalog, the checklist, the tool ladder, the worked reviews — fills in the details.
Read the Types First
Section titled “Read the Types First”Before reading a single line of logic, read the signatures. In Rust, the type signature of a function is a contract that tells you most of what you need to know about ownership before you’ve traced a single value:
fn f(x: Foo)— takes ownership. The caller losesx(unlessFoo: Copy).fn f(x: &Foo)— borrows immutably. The caller keepsxand can still read it.fn f(x: &mut Foo)— borrows exclusively. The caller keepsxbut cannot use it through any other reference while this borrow is alive.fn f() -> Foo— the caller now owns a newFoo.fn f() -> &'a Foo— the caller gets a reference that cannot outlive'a; something else still owns the data.
For every function in the file you’re reviewing, ask: who owns this value, who is only borrowing it, and for how long? A struct field of type Rc<RefCell<T>> or Arc<Mutex<T>> is a loud signal — it means “multiple owners” or “shared across threads” was a real requirement, or it means someone reached for the sledgehammer because the borrow checker complained once and this made the complaint go away. You cannot tell which without reading further, but you now know exactly what question to ask.
struct Order { id: u32, total_cents: u64,}
// Takes ownership -- the caller's `Order` is gone after this call.fn archive(order: Order) -> String { format!("archived order {}", order.id)}
// Borrows -- the caller keeps `order` and can use it again afterward.fn total_dollars(order: &Order) -> f64 { order.total_cents as f64 / 100.0}
fn main() { let order = Order { id: 42, total_cents: 2599 }; println!("{:.2}", total_dollars(&order)); // order still usable here println!("{}", archive(order)); // order is gone now -- using it below would be a compile error}Reading the two signatures tells you the ownership story before you read a single line of the function bodies: total_dollars only looks, archive consumes.
Distrust Every clone(), unwrap(), unsafe, and 'static
Section titled “Distrust Every clone(), unwrap(), unsafe, and 'static”These four things are always legal and often correct — but each one is also the single most common way generated Rust papers over a problem instead of solving it. Treat every occurrence as a question, not a fact:
.clone()— is this clone paying for a genuine second owner, or is it here because the borrow checker complained and cloning was the fastest way to make the red squiggle disappear? A clone that’s dropped almost immediately after being read once is nearly always the second kind..unwrap()/.expect(...)— is this value truly guaranteed to beSome/Okat this point (parsing a compile-time string literal, indexing aconst-sized array), or is it user input, a file read, a network response — something that can and will eventually beNone/Errin production?unsafe— does this block have a// SAFETY:comment explaining why the invariant the compiler can’t check actually holds? No comment is itself a finding, independent of whether the code happens to be sound.'static— is this genuinely “lives for the whole program” data (a string literal, a leaked allocation, aconst), or is'statichere because a lifetime error was annoying and'staticwas the bound that made the compiler stop complaining? The latter usually means the real fix — a borrow, or restructuring who owns what — got skipped.
None of these four are bugs by themselves. All four are exactly where a reviewer’s attention should go first, because the compiler enforces memory safety around them but not good design.
Follow the Error Path
Section titled “Follow the Error Path”Rust makes error handling visible in the type signature (Result<T, E>), which means you can trace it mechanically:
- Find every
?. Each one propagates an error upward — follow the chain to where it’s finally handled (amatch, anif let Err, ormain’s exit code). - Check what
Eactually is.Box<dyn std::error::Error>in amain.rsbinary is a reasonable, idiomatic default. The same type as the public return type of a library function is a smell — callers can’tmatchon the specific failure, only print it. - Look for the drop points: an
unwrap()orexpect()sitting in the middle of an otherwise careful?-chain is a place where “handle the error” quietly became “panic instead.”
Follow the Threads and Futures
Section titled “Follow the Threads and Futures”If the file has thread::spawn, Arc, Mutex, async fn, or .await, ownership questions get a second dimension: not just “who owns this,” but “which thread or task can touch this, and when.” Ask specifically:
- Does a lock (
Mutex/RwLockguard) stay alive across an.awaitpoint? That blocks every other task on that lock while this one is merely waiting, not working. - Is
Arc<Mutex<T>>protecting something that a plainthread::scopeborrow or an atomic integer would have handled with no lock at all? - Is
thread::sleepused inside anasync fn? It blocks the whole worker thread running the executor, not just the current task.
Count the Suppressions
Section titled “Count the Suppressions”Every #[allow(...)], every let _ = fallible_call();, and every _ => catch-all match arm is the code telling the compiler “trust me, ignore this.” Grep for all three before you start reading logic — a file with a handful of suppressions clustered in one function is a much stronger signal than any one of them read in isolation. A _ => arm on an enum is the sharpest of the three: it silently keeps compiling if someone adds a new variant later and forgets to handle it, which is exactly the guarantee match normally gives you for free.
flowchart TD A["Read the types: who owns, who borrows, how long?"] --> B["Distrust clone/unwrap/unsafe/'static"] B --> C["Follow the error path: ? chains and Box<dyn Error>"] C --> D["Follow threads and futures: locks across await?"] D --> E["Count suppressions: #[allow], let _ =, wildcard _"] E --> F[Form a verdict]
What the Compiler Says: A Reference That Does Not Live Long Enough
Section titled “What the Compiler Says: A Reference That Does Not Live Long Enough”“Who owns this, for how long” is not just a reading heuristic — the compiler enforces it directly the moment a reference would outlive the data it points to:
// @expect-error E0515fn borrow_local() -> &'static i32 { let x = 42; &x}
fn main() { println!("{}", borrow_local());}error[E0515]: cannot return reference to local variable `x` --> src/main.rs:3:5 |3 | &x | ^^ returns a reference to data owned by the current function
error: aborting due to 1 previous errorx is owned by borrow_local and dies when the function returns; a &'static i32 promises to be valid for the entire program. No caller-side clone, no unsafe block, no clever lifetime annotation fixes this — the fix is a design change: return i32 by value (it’s Copy, so this is free), or restructure so the caller owns x and only lends it in.
struct Cart { items: Vec<(String, u32)>, // (name, price in cents)}
impl Cart { fn total_cents(&self) -> u32 { self.items.iter().map(|(_, price)| price).sum() }
fn add(&mut self, name: &str, price_cents: u32) { self.items.push((name.to_string(), price_cents)); }}
fn print_receipt(cart: &Cart) { for (name, price) in &cart.items { println!("{name}: {price} cents"); } println!("total: {} cents", cart.total_cents());}
fn main() { let mut cart = Cart { items: Vec::new() }; cart.add("coffee", 350); cart.add("bagel", 275); print_receipt(&cart); // borrows -- cart is still usable after this cart.add("tip", 100); print_receipt(&cart);}Compiling…
Reading this top to bottom: Cart owns a Vec; total_cents and print_receipt only borrow (&self, &Cart), so main can keep using cart between calls; add borrows mutably to push a new item. Nothing here needs a clone, an Rc, or an unwrap — which is itself useful information: when you do see one of those four in real generated code, it’s standing out against a baseline where plain ownership and borrowing were enough.
Mental Model
Section titled “Mental Model”Reading Rust is reading a proof, not a story. The compiler has already checked “does this violate memory safety” and the answer is no by construction — that check is done, permanently, the moment the code compiles. What’s left for a human reviewer is everything the type system doesn’t check: is this the right design, does this error get handled or silently swallowed, is this lock held longer than it needs to be, does this clone exist because two owners were genuinely needed or because it was the path of least resistance. The four escape hatches — clone, unwrap, unsafe, 'static — are exactly the places where a human judgment call replaced a compiler guarantee, which is why they are always the first place to look.
Trade-offs
Section titled “Trade-offs”A strict, checklist-driven read (this lesson’s process) is slower per file than skimming for obviously wrong logic, but it catches an entire class of bugs that “does this look right” skimming misses completely — because compiling, reasonable-looking Rust is precisely the case skimming is worst at. The cost is real: tracing every ? chain and every lock’s lifetime across a large file takes deliberate time. The alternative — trusting that “it compiles and the tests pass” is sufficient review — is cheaper per file and wrong exactly often enough to matter, since neither the compiler nor a shallow test suite checks design-level ownership or error-handling decisions.
Common Misconceptions
Section titled “Common Misconceptions”- “If it compiles and clippy is quiet, it’s fine.” The compiler checks memory safety, not design. Clippy’s default lints catch a subset of style smells. Neither one has an opinion on whether
Rc<RefCell<T>>was the right call, whether an error should have been a panic, or whether a lock is held too long. - “Rust code is safer, so it needs less review than the equivalent Python/JS.” It needs different review. Rust review time shifts away from “did this null-check happen” (the type system already forces that) and toward “is this ownership design right” and “is this error handled or silently discarded” — questions the compiler has no opinion on.
- “A clean
cargo testrun means the logic is correct.” It means the tests that exist pass. A test with noassert!/assert_eq!in its body, or a#[should_panic]with noexpected = "...", both show up as green in that same clean run — see the bug catalog for both.
Reading AI-generated code
Section titled “Reading AI-generated code”Generated Rust often reaches for the heaviest ownership tool available “just in case,” even when the function’s own signature shows a plain borrow would have worked.
🐛 Spot the bug
struct Inventory { items: Vec<String>,}
fn item_count(inventory: Inventory) -> usize { inventory.items.len()}
fn main() { let inventory = Inventory { items: vec!["bolt".to_string(), "nut".to_string()] }; println!("{}", item_count(inventory.clone_from_ref()));}
trait CloneFromRef { fn clone_from_ref(&self) -> Inventory;}
impl CloneFromRef for Inventory { fn clone_from_ref(&self) -> Inventory { Inventory { items: self.items.clone() } }}Show the answer
This compiles and prints 2, and it’s a lot more code than the problem needed. item_count takes Inventory by value for no reason — it only calls .len() on the field, so &Inventory would do — and the call site works around that unnecessary ownership requirement by hand-rolling a whole CloneFromRef trait just to avoid moving inventory away from main. The actual fix deletes the trait entirely: change the signature to fn item_count(inventory: &Inventory) -> usize and call item_count(&inventory). Whenever a caller is going out of its way to clone or copy something before a call, check the callee’s signature first — often the borrow was available the whole time and the function just didn’t ask for it.