Tracking issue for `cell_extras` stabilization

[aturon]

The Ref type connected to RefCell supports some methods for changing the type of the reference, such as map and filter_map. While map is clearly fully general, filter_map is somewhat of a special case hooked into Option. To make it fully general would likely require HKT. We need an overall plan before stabilization.

[aturon]

cc @SimonSapin

[SimonSapin]

This is the kind of thing (unlike say ref_slice #27774) that is impossible to do reasonably outside of std. (Where “unreasonable” is guessing struct layout to access private fields.)

[SimonSapin]

(Ok, thinking a bit more about it, this may not be true. Something like https://github.com/Kimundi/owning-ref-rs/ could, using (Ref<'a, T>, *const U) and unsafe code to dereference the raw pointer, have equivalent functionality with a different API (separate type) and 3 words instead of 2 per reference.)

[mbrt]

What about making RefCell::map stable? It has good use cases and it is clearly usable as-is.

[SimonSapin]

I’d like to nominate at least Ref::map and RefMut::map for stabilization.

filter_map’s functionality can be safely built on top of it with a performance cost (testing the presence of the mapped thing twice). For example:

fn borrow_get<'a>(hashmap: &'a RefCell<HashMap<String, String>>, key: &str) 
                  -> Option<Ref<'a String>> {
    let hashmap: = hashmap.borrow();
    if hashmap:.contains_key(key) {  // Duplicated hash table lookup.
        Some(Ref::map(|hashmap| {
            hashmap[key]  // panic!() for missing key unlikely to be optimized away
        }))
    } else {
        None
    }
}

It’s probably possible to do it unsafely without the run-time cost with raw pointers. (But still less risky than guessing the memory layout of Ref or RefMut.)

[SimonSapin]

For future reference, here is fully generic filter_map built on top of map using raw pointers. I’m fairly confident that it’s safe.

use std::cell::{Ref, RefMut};

pub fn ref_filter_map<
    T: ?Sized,
    U: ?Sized,
    F: FnOnce(&T) -> Option<&U>
>(orig: Ref<T>, f: F) -> Option<Ref<U>> {
    f(&orig)
        .map(|new| new as *const U)
        .map(|raw| Ref::map(orig, |_| unsafe { &*raw }))
}

pub fn ref_mut_filter_map<
    T: ?Sized,
    U: ?Sized,
    F: FnOnce(&mut T) -> Option<&mut U>
>(mut orig: RefMut<T>, f: F) -> Option<RefMut<U>> {
    f(&mut orig)
        .map(|new| new as *mut U)
        .map(|raw| RefMut::map(orig, |_| unsafe { &mut *raw }))
}

[alexcrichton]

🔔 This issue is now entering its cycle final comment period to be handled in 1.8 🔔

We're specifically considering stabilizing map and deprecating filter_map. We're also willing to stabilize map on the lock-related guards as well so long as they land soon.

[reem]

As just noted in #30834:

I've discovered a safety hole in MutexGuard::map - it is not safe in combination with Condvar, since the "inner" borrow can be invalidated by another thread taking the lock during a Condvar::wait see: http://is.gd/RM038X.

This is not a problem with the RwLock guard methods, since the RwLock guards cannot be used with a Condvar.

It would be possible to provide a similar, but safe, API by defining a separate type for "mapped" mutex guards that can't be used with a Condvar.