Skip to content

Conversation

@jamesmunns
Copy link
Collaborator

Fixes #535

This change fixes the two things noted in the #535: RawIter now holds an &mut to the List, and also now implements Send.

I think we could also implement Send for Iter/IterMut, but I'm less confident about that.

I also am not certain whether the unsafe Send impl should be gated on T: Send, I opted not to because we never take ownership of a node (e.g. a pop) using the RawIter interface, so we're only ever "visiting" a pinned node, so requiring T: Send didn't seem necessary. I'm open to more confident logic around that.

I also added a util helper to assert that RawIter remains Send.

@jamesmunns jamesmunns requested a review from hawkw June 2, 2025 12:41
@jamesmunns
Copy link
Collaborator Author

Fun note I just realized: if we are iterating in an async context, the items yielded by the iterator (NonNull<T>) are also !Send, which means you need an annoying adapter still.

jamesmunns added a commit to tweedegolf/cfg-noodle that referenced this pull request Jun 4, 2025
…r types

Unfortunately as we are holding NonNulls across await points, we'll still need some
kind of adapter to mark this as sound. This means that
hawkw/mycelium#536 is not in and of itself sufficient.

Comments have been updated to reflect this.
@jamesmunns
Copy link
Collaborator Author

(I still think we should merge this, in case some users don't need to hold the yielded ptrs across await point, and to fix the &mut soundness hole).

Copy link
Owner

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay yeah we should probably just merge this, thanks and sorry i forgot about it for, uh, 7 months... 😅

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

Successfully merging this pull request may close these issues.

cordyceps: Should list::RawIter impl Send?

2 participants