This function is supposed to look up a name's ID in a cache, mint a fresh ID on first sight, and reuse the cached value on subsequent lookups. The returned IDs are correct in every test — but production keeps burning through the ID space three times faster than it should.

use std::collections::HashMap;
use std::sync::atomic::{AtomicU64, Ordering};

static NEXT_ID: AtomicU64 = AtomicU64::new(1);

fn allocate_id() -> u64 {
    NEXT_ID.fetch_add(1, Ordering::SeqCst)
}

// Look up a name's ID. Mint a fresh one on first sight,
// otherwise return the cached value.
fn resolve(cache: &mut HashMap<String, u64>, name: &str) -> u64 {
    let id = cache.get(name).copied().unwrap_or(allocate_id());
    cache.entry(name.to_string()).or_insert(id);
    id
}

fn main() {
    let mut cache = HashMap::new();
    println!("{}", resolve(&mut cache, "alice")); // 1
    println!("{}", resolve(&mut cache, "bob"));   // 2
    println!("{}", resolve(&mut cache, "alice")); // 1
    println!("counter: {}", NEXT_ID.load(Ordering::SeqCst)); // 4
}

The Bug

unwrap_or is a function, not a control-flow construct. Rust evaluates its argument at the call site, before unwrap_or decides what to return. Every call to resolve — including cache hits — calls allocate_id(), burns an ID, and throws the result away.

Trace the third call, resolve(cache, "alice"):

• cache.get("alice") returns Some(&1). Good.
• Before unwrap_or executes, its argument evaluates: allocate_id() increments NEXT_ID from 3 to 4 and returns 3.
• unwrap_or(3) looks at Some(1), ignores the 3, and returns 1.

The returned ID is correct — but the counter has silently advanced. Any test that asserts on the return value passes. The bug only surfaces as ID gaps, exhausted quotas, extra database writes, or duplicate analytics events, depending on what allocate_id actually does.

It's especially misleading because Rust's closure-based combinators — .map, .and_then, .or_else — are lazy. That teaches the wrong intuition: "combinators on Option only run when relevant." unwrap_or takes a value, not a closure, so the "when relevant" part doesn't apply.

The Fix

Use unwrap_or_else, which takes a closure that runs only on None:

let id = cache.get(name).copied().unwrap_or_else(allocate_id);

Same trap lurks in unwrap_or(vec![]) (allocates every call), unwrap_or(config.default.clone()) (clones every call), and Result::unwrap_or(load_from_disk()) (does I/O on the happy path). If the borrow checker pushes you toward a .clone() to satisfy unwrap_or, that's a signal to switch: move the clone into an unwrap_or_else closure and the clone only fires when needed.

The naming convention is the tell. Across std, the _else and _with suffixes — or_insert_with, get_or_insert_with, ok_or_else, map_or_else — all mean "closure, evaluated lazily." Reach for those any time the fallback is a computation rather than a literal. For the specific case of "fallback is T::default()," unwrap_or_default is lazy too.