Conversation
Codecov Report
|
jonhoo
left a comment
jonhoo
left a comment
There was a problem hiding this comment.
Thanks for taking this on! Some mostly smaller comments
| TreeNode::get_tree_node(p_parent) | ||
| .left | ||
| .store(Shared::null(), Ordering::Relaxed); | ||
| p_parent_deref.left.store(Shared::null(), Ordering::Relaxed); |
There was a problem hiding this comment.
I wonder why were previously re-computed get_tree_node here 🤔 There are a couple of other places too where we re-do TreeNode:;get_tree_node. But maybe those were just unnecessary.
| let guard = unsafe { Guard::unprotected() }; | ||
| let p = self.first.swap(Shared::null(), Ordering::Relaxed, &guard); | ||
| Self::drop_tree_nodes(p, drop_values, &guard); | ||
| unsafe { Self::drop_tree_nodes(p, drop_values, &guard) } |
There was a problem hiding this comment.
This is missing a safety comment.
| /// | ||
| /// # Safety | ||
| /// | ||
| /// This method may be called only if the pointer is valid. |
There was a problem hiding this comment.
| /// This method may be called only if the pointer is valid. | |
| /// This method may be called only if the currently-stored pointer is valid. |
There was a problem hiding this comment.
Should this also require that the pointer stored in self hasn't otherwise been shared elsewhere?
There was a problem hiding this comment.
Also, it's not entirely clear what "valid" means here. I think it has to say "valid as an &mut T".
| /// | ||
| /// # Safety | ||
| /// | ||
| /// This method may be called only if the pointer is valid |
There was a problem hiding this comment.
Here, too, I think we need to say exactly what validity requirement we're after.
2 participants
Closes #104.
This change is