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

Publicly expose either macro as for_both #58

Merged
merged 4 commits into from
Jun 23, 2022

Conversation

thomaseizinger
Copy link
Contributor

@thomaseizinger thomaseizinger commented Sep 20, 2021

See commit messages for details.

Resolves #57.

I've added d4f231d at the very end as a suggestion. Happy to drop it if it is undesired. My rationale was that a closure like syntax is

a) more inuitive
b) less error prone

If we allow the user to specify patterns, they might actually run into a situation where they produce a non-exhaustive match. If there is consensus, I can migrate the entire crate to use closure-like syntax but I wanted to get feedback on the idea first.

This is in preparation for the macro being publicly exported.
Crates can be renamed in `Cargo.toml`. To ensure we reference the
`Either` type correctly, we need to use the special-cased `$crate`
substitution.

See https://veykril.github.io/tlborm/decl-macros/minutiae/import-export.html.
@thomaseizinger thomaseizinger changed the title Rename either macro to for_both Publicly expose either macro as for_both Sep 20, 2021
@cuviper
Copy link
Member

cuviper commented Sep 22, 2021

Hmm. Note that Rust functions and closures also allow full patterns for their arguments, not just an identifier.

Also, a lot of the internal uses have ref or ref mut patterns, in part because we still claim support all the way back to Rust 1.12, whereas match ergonomics came in Rust 1.26. That's ancient history, so we probably don't need to support that, but patterns are still useful in general. IMO it would be fine to just document that your pattern has to be irrefutable for both sides, just as your expression has to end up valid both ways as well.

We could still give that closure syntax, if desired, or even let it support patterns both ways.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@thomaseizinger
Copy link
Contributor Author

Friendly ping @bluss !

Co-authored-by: bluss <bluss@users.noreply.github.com>
@cuviper
Copy link
Member

cuviper commented Jun 23, 2022

I committed the syntax suggestion. I'm trying to get things together for a release...

@cuviper cuviper merged commit c5bf4fe into rayon-rs:master Jun 23, 2022
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.

Publicly expose the either! macro?
3 participants