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

Merge *SliceExt traits, use assoc types in SliceExt, Neg and Not #20398

Closed
wants to merge 1 commit into from

Conversation

japaric
Copy link
Member

@japaric japaric commented Jan 1, 2015

The following traits have been removed:

  • ClonedSliceExt
  • PartialEqSliceExt
  • OrdSliceExt
  • BoxedSliceExt

All their methods have been moved into the SliceExt trait using where clauses.

The Neg and Not traits now have an associated type for their return type. This breaks all existing implementations.

[breaking-change]


This requires a snapshot that contains PR #20374

r? @aturon
cc @nikomatsakis

@alexcrichton
Copy link
Member

This looks amazing, thanks!

I think the only part about this we really need to think about is the name of the associated types (now that they're a public-facing API). The question of "what should I name this" will probably come up fairly frequently, so it would be good to have an answer. That being said, though, I'd rather land this now if we don't quite have an answer than take time to sort it out.

@aturon
Copy link
Member

aturon commented Jan 1, 2015

@japaric In case you didn't see on IRC: I just realized that using Result by default here is problematic because of the prelude, especially if we start bringing these into scope for the trait. Output seems like a good choice to avoid this problem.

I'm sorry for not catching this earlier on :(

However, I suspect that it should be easy to run a sed script over the patch itself to fix this. Feel free to squash into a single commit if that would make it easier.

@aturon
Copy link
Member

aturon commented Jan 1, 2015

OK, otherwise I've reviewed this PR as well and it looks fantastic.

@japaric
Copy link
Member Author

japaric commented Jan 1, 2015

Closing in favor of #20410

@japaric japaric closed this Jan 1, 2015
@japaric japaric deleted the at branch January 4, 2015 13:06
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.

3 participants