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

Add async traits, implementations and fuzzers #9

Merged
merged 16 commits into from
Jun 11, 2024
Merged

Add async traits, implementations and fuzzers #9

merged 16 commits into from
Jun 11, 2024

Conversation

mycognosist
Copy link
Collaborator

@mycognosist mycognosist commented Jun 4, 2024

  • Add async traits
    • nb : Send
    • local_nb : !Send
  • Add SyncToLocalNbConsumer and SyncToLocalNbProducer wrapper structs
    • These are used for the implementations listed below
  • Add local_nb consumer implementations
    • Cursor
    • IntoVec
    • IntoVecFallible
    • Invariant
    • InvariantNoop
    • Scramble
  • Add local_nb producer implementations
    • Cursor
    • Invariant
    • InvariantNoop
    • Scramble
  • Add local_nb fuzzers
    • cursors
    • invariant
    • invariant_noop
    • scramble_consumer
    • scramble_producer

@mycognosist mycognosist added the enhancement New feature or request label Jun 4, 2024
@AljoschaMeyer
Copy link
Contributor

AljoschaMeyer commented Jun 4, 2024

You can simply write a wrapper around a sync::Consumer that implements local_nb::Consumer. And then implement most consumers by wrapping the sync versions in that wrapper. Much fewer lines of code, and a nice utility for all users of the crate as well.

Analogous for producers as well, of course.

SyncToLocalNbConsumer and SyncToLocalNbProducer are not the greatest names, but not the worst either.

@AljoschaMeyer
Copy link
Contributor

(There's little else for me to review, those wrappers would get around all that copypasted code that I'm now skipping over.)

src/sync.rs Outdated Show resolved Hide resolved
@AljoschaMeyer
Copy link
Contributor

The pipe functions don't need separate error types for sync, nb, and local, nb. Those should all use the same type, unless I'm missing something.

///
/// Must not be called after any function of this trait has returned an error,
/// nor after `close` was called.
fn consumer_slots(
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, why doesn't this require the explicit lifetime bound that local_nb::Consumer::consumer_slots needs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Something to do with the additional trait bounds maybe?

src/lib.rs Outdated
pub mod sync;
pub mod sync_to_local_nb;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm open to changing the location of this module. Could also be incorporated into the sync module.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd place it in the local_nb module. I'd prefer keeping those three top-level mods and then putting any details inside them.

@AljoschaMeyer
Copy link
Contributor

All looking good so far.

@mycognosist
Copy link
Collaborator Author

@AljoschaMeyer Any changes or additions you'd like to see here before merging?

I can do the consume_all() helpers in a separate PR.

@AljoschaMeyer
Copy link
Contributor

Looks good.

@AljoschaMeyer AljoschaMeyer merged commit 338a298 into main Jun 11, 2024
1 check failed
@mycognosist mycognosist deleted the async branch June 11, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants