Skip to content

Move mutable key access to a trait #7

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
bluss opened this issue Nov 2, 2016 · 8 comments
Closed

Move mutable key access to a trait #7

bluss opened this issue Nov 2, 2016 · 8 comments

Comments

@bluss
Copy link
Member

bluss commented Nov 2, 2016

We should put this feature a bit on the side, in a trait.

Something like:

impl Ordermap {
    fn get_pair_mut(&mut self, key: &Q) -> (&K, &mut V) { .. }
}

impl MutableKeys for OrderMap {
    fn good_method_name(&mut self, key: &Q) -> (&mut K, &mut V) {  .. }
}
@Popog
Copy link

Popog commented Dec 7, 2016

Should this be done via specialization?

Something like:

impl <K, V, S> Ordermap<K, V, S> {
    default fn get_pair_mut(&mut self, key: &Q) -> (&K, &mut V) { .. }
}
impl <K: MutableHashMarker, V, S> Ordermap<K, V, S> {
    fn get_pair_mut(&mut self, key: &Q) -> (&mut K, &mut V) { .. }
}

I suspect inherent impl specialization is a long way off, but ultimately I much prefer only having one method name.

@bluss
Copy link
Member Author

bluss commented Dec 7, 2016

The point of a trait would be to provide the feature, but have it a bit out of the way, so reduce misuses of it.

@bluss
Copy link
Member Author

bluss commented Dec 7, 2016

Oh I see the role of that marker trait now. I don't think that's a viable thing to do for technical reasons: get_pair_mut would then vary its signature in a way that's not compatible with Rust's usual trait rules (it would be similar to a C++ templaty thing, that expands to something different for different input types).

@Popog
Copy link

Popog commented Dec 7, 2016

There are several ways to accomplish something close to it.

You could use associated types:

fn get_pair_mut(&mut self, key: &Q) -> (KeyReference::Type, &'a mut V)
where K: KeyReference

impl<'a, K> KeyReference<'a> for K { default type Type = &'a K; }
impl<'a, K:MutableHashMarker> KeyReference<'a> for K { type Type = &'a mut K; }

Also just realized you could do it without specialization if you use a wrapper:

struct KeyWrapper<'a, K: 'a>(&'a mut K);

trait MutableHashMarker{}

impl<'a, K> Deref for KeyWrapper<'a, K> {
    type Target = K;
    fn deref(&self) -> &K { self.0 }
}
impl<'a, K: MutableHashMarker> DerefMut for KeyWrapper<'a, K> {
    fn deref_mut(&mut self) -> &mut K { self.0 }
}

impl<'a, K> Into<&'a K> for KeyWrapper<'a, K> {
    fn into(self) -> &'a K { self.0 }
}
impl<'a, K: MutableHashMarker> Into<&'a mut K> for KeyWrapper<'a, K> {
    fn into(self) -> &'a mut K { self.0 }
}

fn get_pair_mut(&mut self, key: &Q) -> (KeyWrapper<K>, &mut V)

@bluss
Copy link
Member Author

bluss commented Dec 7, 2016

I've already implemented KeyWrapper once. (I promise). I didn't commit to the idea.

@bluss
Copy link
Member Author

bluss commented Dec 7, 2016

In general I think each advanced use of the type system has a cost and it has to have a value that offsets that :) Sometimes the simple is the best.

@Popog
Copy link

Popog commented Dec 7, 2016

I think it'll be rather difficult to come up with good_method_name. get_pair_mut_key_mut doesn't really roll of the proverbial tongue.

Even with a good name, having two functions still seems less intuitive to me.

@Popog
Copy link

Popog commented Dec 7, 2016

It is possible to avoid using a wrapper structure, and achieve almost the same thing using impl Trait

Here's a small demo:

#![feature(conservative_impl_trait)]
use std::ops::Deref;

trait MutableHashMarker{}

trait KeyRef<'a, K>: Deref<Target=K> {
    fn as_mut(self) -> &'a mut K
    where K: MutableHashMarker;
}

impl<'a, K> KeyRef<'a, K> for &'a mut K {
    fn as_mut(self) -> &'a mut K
    where K: MutableHashMarker { self }
}



struct OrderMap<K, V, S>(K, V, S);

impl <K, V, S> OrderMap<K, V, S> {
    fn get_pair_mut<'a, Q>(&'a mut self, _key: &Q) -> (impl KeyRef<'a, K>, &'a mut V) {
        (&mut self.0, &mut self.1)
    }
}


struct MutKey(i32);
impl MutableHashMarker for MutKey{}

fn main() {
    {
        let mut v = OrderMap(MutKey(1i32), (), ());
        let (k, _) = v.get_pair_mut(&0);
        k.as_mut();
    }
    {
        let mut v = OrderMap(1i32, (), ());
        let (k, _) = v.get_pair_mut(&0);
        let i: i32 = *k;
        // k.as_mut(); // Error: the trait `MutableHashMarker` is not implemented for `i32`
    }
}

@bluss bluss mentioned this issue Sep 2, 2017
8 tasks
@bluss bluss closed this as completed in #39 Sep 24, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants