Fix RefUnwindSafe & UnwinsSafe impls for lazy::SyncLazy

[matklad]

I think we should implement those unconditionally with respect to F.

The user code can't observe the closure in any way, and we poison lazy if the closure itself panics.

But I've never fully wrapped my head around UnwindSafe traits, so 🤷‍♂️

[rust-highfive]

r? @kennytm

(rust_highfive has picked a reviewer for you, use r? to override)

[matklad]

cc tracking issue #74465

[kennytm]

r? @KodrAus

[KodrAus]

Hmm, I think we need the closure to be unwind safe, because users can observe panics through borrowed arguments. Right now, this won’t compile:

let not_unwind_safe = std::cell::RefCell::new(42);

let l = std::lazy::SyncLazy::new(|| {
    let mut a = not_unwind_safe.borrow_mut();
    *a = 43;
    
    panic!("lol");
    
    *a
});

std::panic::catch_unwind(|| {
    let a = *l;
});

But if we implement RefUnwindSafe unconditionally then it will, and we can observe the state of not_unwind_safe across the catch_unwind.

They’re pretty niche traits 🙂 I use this blanket impl to try remember how they interact:

impl<T: RefUnwindSafe + ?Sized> UnwindSafe for &T {} // and Rc<T> and Arc<T> etc

fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> { .. }

So UnwindSafe is the main trait for values that will still make sense if passed by value into a recoverable scope, and RefUnwindSafe is used to implement UnwindSafe for smart pointers and other types that are passed by value themselves but are logically borrowed.

Looking at them now, should the impl be:

impl<T: RefUnwindSafe, F: UnwindSafe> RefUnwindSafe for SyncLazy<T, F> {}

because we move out of F when initializing?

It might also be worth moving these impls into the std::panic module alongside the others.

[matklad]

Yup, I agree with your reasoning, I think this is rougthly the same justification we use for Send&Sync impls here. Pushed an update.

[bors]

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

[matklad]

Rebased onto master. I decided not to move stuff over to std::panic for now, so that we ca easier rename types, if we decide to.

[KodrAus]

Looks good to me!

@bors r+

[bors]

📌 Commit ed1439c has been approved by KodrAus