Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Explain &**node #17

Closed
ericye16 opened this issue Aug 10, 2015 · 10 comments
Closed

Explain &**node #17

ericye16 opened this issue Aug 10, 2015 · 10 comments

Comments

@ericye16
Copy link
Contributor

From https://www.reddit.com/r/rust/comments/3geb5q/learning_rust_with_entirely_too_many_linked_lists/ctxgwmz, maybe explain &**node to make it more obvious what is being dereferenced. Coming from C, this was a little confusing.

@Gankra
Copy link
Collaborator

Gankra commented Aug 10, 2015

Thanks for logging the issue 👍

@mackwic
Copy link

mackwic commented Nov 15, 2015

Just stumbled upon myself on this, I didn't even caught the new ** in 3.5 iter, at line 369 (or after “ (ノಥ益ಥ)ノ ┻━┻ ”) and was very surprised to see it in the final code.

I also didn't fully understood was it was so obvious that we needed as_ref(). Expanding a little on that would do no harm. :)

@hwchen
Copy link

hwchen commented Dec 27, 2015

I was looking through this section, should the &**node on line 259 be &*node? I may have missed something, but I didn't see a reason for it to change from the previous section.

https://github.com/Gankro/too-many-lists/blame/master/text/second-iter.md#L259

Similar issue in the error at line 123123.

https://github.com/Gankro/too-many-lists/blame/master/text/second-iter.md#L123

@louy2
Copy link
Contributor

louy2 commented Aug 8, 2016

Remember what we want at Iter::next is of type Option<&Node<T>>, and & is reference while * is dereference just as in C.

self.head is of type Option<Box<Node<T>>> so without as_ref(), inside map() node is of type Box<Node<T>>. One dereference, *, to get rid of Box<> and take out Node<T>. We then refer to it with &.

With as_ref() returning Option<&Box<Node<T>>>, though, inside map() node becomes type &Box<Node<T>>. One dereference for & and another for Box<>, two dereferences are needed to take out Node<T>. It is then referred to with &.

@mackwic
IMHO, the theme is we cannot simply move the result out of an Option<>. Either we take it with take() or map(), or view it with as_ref(). If we take it with map() but only returns a reference, the node itself goes out of scope as soon as the reference is returned. So we have to first view it as_ref(), then map() the reference out to dereference the node.

@neverfox
Copy link

neverfox commented Feb 28, 2017

In keeping with the theme of the book, it probably would be good for the section to have another case of the compiler screaming at us for the mismatched type after adding as_ref and then fixing it and explaining why. Also, there are a few parts where the text gives a spoiler and shows ** before that change was necessary.

@jhenninger
Copy link

jhenninger commented Mar 5, 2018

I'm stil working through the book and always try to implement the current feature (e.g. .iter()) on my own (using only the Rust documentation) before looking at the "official" solution. My version of iter() ended up looking a little bit different:

pub struct List<T> {
    head: Link<T>,
}

type Link<T> = Option<Box<Node<T>>>;

struct Node<T> {
    elem: T,
    next: Link<T>,
}

impl<T> List<T> {
    // [...]

    pub fn iter(&self) -> Iter<T> {
        Iter(&self.head)
    }
}

pub struct Iter<'a, T: 'a>(&'a Link<T>);

impl<'a, T> Iterator for Iter<'a, T> {
    type Item = &'a T;

    fn next(&mut self) -> Option<Self::Item> {
        self.0.as_ref().map(|node| {
            self.0 = &node.next;
            &node.elem
        })
    }
}

Not sure if this is any better or worse, but it gets rid of both the &** as well as the .map() call in List::iter(), which felt like a duplication of the code in Iter::next() to me.

Edit: Unfortunately I'm struggling to use the same approach for iter_mut() :(

@m-ou-se
Copy link
Contributor

m-ou-se commented Mar 24, 2019

It might be clearer to use .map(Box::deref) instead of .map(|node| &**node).

@Gankra
Copy link
Collaborator

Gankra commented Mar 24, 2019

I just published a major update and fixing this was one of the changes.

(using Box::deref is interesting though)

@Gankra Gankra closed this as completed Mar 24, 2019
@louy2
Copy link
Contributor

louy2 commented Mar 25, 2019

I tried @m-ou-se 's suggestion in playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=7156f8fdd1d3c883379b5de868f230be

Curious, I went to read the source code of Box::deref, and voila, there it is:

impl<T: ?Sized> Deref for Box<T> {
    type Target = T;

    fn deref(&self) -> &T {
        &**self
    }
}

I believe this can be introduced in the same vein as the pattern of take() and map().

Btw, the more general Deref::deref also works.

@yhpark
Copy link

yhpark commented Mar 31, 2019

Hi, I was following the tutorial today and stumbled upon this chapter.. and my solution was the following:

Iter { next: self.head.as_ref().map(|boxed_node| boxed_node.as_ref()) }

I don't know whether people usually deref & manually via * but adding another * for that purpose seemed somewhat unnatural, and above all, the resulting code doesn't look very intelligible to me.

Actually I also just went ahead to read the source code of Box.as_ref() and voila,

#[stable(since = "1.5.0", feature = "smart_ptr_as_ref")]
impl<T: ?Sized> AsRef<T> for Box<T> {
    fn as_ref(&self) -> &T {
        &**self
    }
}

I think either of Box::deref and Box.as_ref should be preferred to the current solution in the tutorial

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants