Check tail node in check_links #46269
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @KodrAus (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| @@ -1314,6 +1316,11 @@ mod tests { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| // verify that the tail node points to the last node. | |||
| let tail = list.tail.as_ref().expect("some tail node").as_ref(); | |||
There was a problem hiding this comment.
Do we need the extra as_ref at the end here?
There was a problem hiding this comment.
list.tail is an Option<Shared<Node<T>>>. The first as_ref avoids creating a local copy (which would have to be in scope). The second converts the Shared ptr into a reference (through Shared::as_ref), which can then be casted.
So yes unless I'm missing something?
There was a problem hiding this comment.
Oh I see, sorry I hadn't noticed the extra level of indirection in there. Looks like the way you're doing this is about as good as it gets, since Shared::as_ptr returns a *mut T, so you'd have to cast that anyways.
|
Hi @udoprog, thanks for improving these tests! This looks good to me, just one tiny question about a maybe redundant |
|
@bors r+ rollup |
|
@KodrAus: 🔑 Insufficient privileges: and not in try users |
|
@KodrAus: 🔑 Insufficient privileges: Not in reviewers |
|
Hmm @kennytm should I be able to |
|
📌 Commit 4e4f8ab has been approved by |
|
Thank you! |
Check tail node in check_links
No description provided.