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 new protocol and native support for Ordering #591

Merged
merged 2 commits into from
Jul 30, 2023
Merged

Conversation

udoprog
Copy link
Collaborator

@udoprog udoprog commented Jul 30, 2023

I noticed while looking over https://github.com/khvzak/script-bench-rs/blob/main/benches/rune.rs that the comparison operations were implemented using associated functions, which is not very idiomatic.

This introduces the CMP protocol, which matches Ord::cmp in Rust, allowing you to implement a single handler for all total comparison operators.

At some point down the line we might want to look into supporting PARTIAL_CMP as well.

The second commit refines testing, and fixes support for tuple index assign operations.

@udoprog udoprog added the enhancement New feature or request label Jul 30, 2023
@ModProg
Copy link
Contributor

ModProg commented Jul 30, 2023

I mean, we could also just allow CMP to return both Option or Ordering.

Not sure if it makes sense to separate these explicitly in an untyped language.

@udoprog
Copy link
Collaborator Author

udoprog commented Jul 30, 2023

The goal is eventually to introduce gradual typing, at which point retrofitting support for partial ordering might become harder if we don't make the distinction right now. To that end I'm OK with having to look up the existence of two protocol methods to emulate Ord: PartialOrd if the type is unknown. Once the type is known by the compiler, it can immediately pick which one to call.

Ultimately the overriding decision making here is "whatever Rust does" as long as there's no obvious error so that we don't have to think too much about it 😄.

@udoprog udoprog merged commit ceb177f into main Jul 30, 2023
@udoprog udoprog deleted the cmp-ordering branch July 30, 2023 16:19
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.

2 participants