Skip to content

Remove Sendable from AsyncBufferedByteIterator #220

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

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

FranzBusch
Copy link
Member

Motivation

I raise on the pitch of AsyncBufferedByteIterator on the forums that I think the iterator must not be Sendable. The reasoning for this is twofold. First, an iterator is the connection of a consumer to the AsyncSequence; therefore, iterators should not be shared since it breaks that assumption. Secondly, the implementation of the AsyncBufferedByteIterator can be more straight forward since we don't have to check for unique ownership of the storage.

Modification

Remove the Sendable conformances from AsyncBufferedByteIterator.

@FranzBusch FranzBusch requested a review from phausler October 20, 2022 12:52
Copy link
Member

@phausler phausler left a comment

Choose a reason for hiding this comment

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

given the two issues raised I think this will be fine.

@FranzBusch FranzBusch force-pushed the fb-remove-sendable-buffered-byte branch from bc8ef67 to 1016fdc Compare October 20, 2022 16:54
# Motivation
I raise on the pitch of `AsyncBufferedByteIterator` on the forums that I think the iterator must not be `Sendable`. The reasoning for this is twofold. First, an iterator is the connection of a consumer to the `AsyncSequence`; therefore, iterators should not be shared since it breaks that assumption. Secondly, the implementation of the `AsyncBufferedByteIterator` can be more straight forward since we don't have to check for unique ownership of the storage.

# Modification
Remove the `Sendable` conformances from  `AsyncBufferedByteIterator`.
@FranzBusch FranzBusch force-pushed the fb-remove-sendable-buffered-byte branch from 1016fdc to ebc4aca Compare October 20, 2022 16:55
@FranzBusch FranzBusch requested a review from phausler October 20, 2022 16:55
Copy link
Member

@phausler phausler left a comment

Choose a reason for hiding this comment

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

Looks great!

@phausler phausler merged commit ed0b086 into apple:main Oct 20, 2022
@FranzBusch FranzBusch deleted the fb-remove-sendable-buffered-byte branch October 26, 2022 20:29
tcldr pushed a commit to tcldr/swift-async-algorithms that referenced this pull request Nov 3, 2022
# Motivation
I raise on the pitch of `AsyncBufferedByteIterator` on the forums that I think the iterator must not be `Sendable`. The reasoning for this is twofold. First, an iterator is the connection of a consumer to the `AsyncSequence`; therefore, iterators should not be shared since it breaks that assumption. Secondly, the implementation of the `AsyncBufferedByteIterator` can be more straight forward since we don't have to check for unique ownership of the storage.

# Modification
Remove the `Sendable` conformances from  `AsyncBufferedByteIterator`.
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.

2 participants