Skip to content

drop some redundant Ord method implementations #8862

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
wants to merge 2 commits into from

Conversation

dcrewi
Copy link
Contributor

@dcrewi dcrewi commented Aug 29, 2013

If this were to break something, I would expect existing tests to catch it, and a make check run passes locally.

@huonw
Copy link
Member

huonw commented Aug 29, 2013

I don't think you can drop the ones for Ratio: it's generic and so may contain a non-total orderable type, e.g. for floating point, 1.0/2.0 > NaN / 2.0 is true under this change, but should probably be false.

@dcrewi
Copy link
Contributor Author

dcrewi commented Aug 29, 2013

Unless I'm reading it wrong, Json is implemented in terms of less-than also. Isn't this exactly what the default implementation of Ord does?

Ratio is generic and so might contain a non-total orderable type. It
should not use the default Ord implementation.
@huonw
Copy link
Member

huonw commented Aug 29, 2013

I guess the current implementation is incorrect then.

@thestinger
Copy link
Contributor

It's possible that JSON shouldn't be allowing NaN at all. It doesn't seem to be included in the specification.

bors added a commit that referenced this pull request Aug 31, 2013
If this were to break something, I would expect existing tests to catch it, and a `make check` run passes locally.
@bors bors closed this Aug 31, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 4, 2022
…xFrednet

`get_last_with_len`: lint `VecDeque` and any deref to slice

changelog: [`get_last_with_len`]: lint `VecDeque` and any deref to slice

Previously only `Vec`s were linted, this will now catch any usages on slices, arrays, etc. It also suggests `.back()` for `VecDeque`s

Also moves the lint into `methods/`
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.

5 participants