Skip to content

RFC: Replace built-in compare operations with an iface #2007

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

Closed
marijnh opened this issue Mar 16, 2012 · 2 comments
Closed

RFC: Replace built-in compare operations with an iface #2007

marijnh opened this issue Mar 16, 2012 · 2 comments

Comments

@marijnh
Copy link
Contributor

marijnh commented Mar 16, 2012

(I don't think this is a big priority, but feel it'd be an improvement on the long term.)

Our comparison operators currently use shape information to walk the compared values using a hard-coded algorithm. I feel it would be cleaner to use an interface for this

  • It'd allow people to write custom comparison code (scoped, so you can implement several styles of comparing for a type, and pick the one you want to use using imports)
  • It'd probably be faster, since these would be completely statically resolved, optionally inlined, rather than interpreting shape strings (or using a walker, which still carries an overhead).
  • It'd unify concepts. Comparing in a generic function is very much like other operations on generic types. Using a parameter bound for that seems sensible, and removes a special case in the monomorphization code, which currently has to treat generics that use comparison operators specially.

A potential problem is that, while we can write sensible generic impls for things like pointer, vec, and tuple types, records would have to be implemented per-case. Since we already have #[auto_serialize], it probably makes sense to also provide an #[auto_cmp] attribute for type items, which would generate a default comparision impl for the type.

@ghost ghost assigned marijnh Mar 16, 2012
@nikomatsakis
Copy link
Contributor

I am tentatively in favor of this but there are two considerations, both related to the consistency problem:

  1. When we auto-generate comparison operators, we have to consider which impls to use for the nested operations. So long as the user has to declare the generation of the operators, as you proposed, this is relatively simple: you use the impls that are in scope at the declaration.
  2. I think we need to resolve the hashtable problem first (that is, track the impl along with the value of a type parameter for bounded type parameters) or else we will certainly encounter problems.

@graydon
Copy link
Contributor

graydon commented Sep 13, 2012

We came to consensus a while ago to do this and are mid-way-through picking the bugs out of the implementation (which is, I believe, now on by default). Closing this.

@graydon graydon closed this as completed Sep 13, 2012
@marijnh marijnh removed their assignment Jun 16, 2014
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

No branches or pull requests

3 participants