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

clippy::incorrect_partial_ord_impl_on_ord_type tripping up #115

Open
rooooooooob opened this issue Oct 18, 2023 · 2 comments
Open

clippy::incorrect_partial_ord_impl_on_ord_type tripping up #115

rooooooooob opened this issue Oct 18, 2023 · 2 comments
Labels
bug The crate does not work as expected

Comments

@rooooooooob
Copy link

This was introduced in rust 1.73. I'm not sure if this is the right spot to report this. I figure it was better here since this isn't a part of standard rust so a clippy issue didn't seem correct. Feel free to close the issue if this isn't the correct spot to handle it.

Having both a Ord and PartialOrd implementation that isn't just delegating to the Ord wrapping in Some will cause this. I assume it's based on how derivative's proc macros implement the traits cause rustc to look at it as if it were hand implemented.

As a user I would assume that using derivative to derive traits should generally function as close to directly using derive() as possible, e.g. to not have this reported when both traits are derived via derivative to match derive()'s behavior here.

As an end user I also couldn't find a place to put [allow(clippy::incorrect_partial_ord_impl_on_ord_type)] locally on the relevant types to suppress it, I imagine due to how derivative + the lint work.

Minimal repro example with rust 1.73 calling cargo clippy

#[derive(Derivative)]
#[derivative(Eq, PartialEq, Ord, PartialOrd)]
pub struct Foo {
    x: i64,
}
@rooooooooob rooooooooob added the bug The crate does not work as expected label Oct 18, 2023
rooooooooob added a commit to dcSpark/cardano-multiplatform-lib that referenced this issue Oct 18, 2023
`clippy::incorrect_partial_ord_impl_on_ord_type` was failing.

In a few cases this was fixed here, but others were coming from our use
of the `derivative` crate causing clippy to think both were
non-canonical implementations.

Issue here for derivative: mcarton/rust-derivative#115

This was blanket ignored in cml-chain+cml-core to work around those.
gostkin pushed a commit to dcSpark/cardano-multiplatform-lib that referenced this issue Oct 19, 2023
`clippy::incorrect_partial_ord_impl_on_ord_type` was failing.

In a few cases this was fixed here, but others were coming from our use
of the `derivative` crate causing clippy to think both were
non-canonical implementations.

Issue here for derivative: mcarton/rust-derivative#115

This was blanket ignored in cml-chain+cml-core to work around those.
@AndrewKvalheim
Copy link

This lint has been renamed to clippy::non_canonical_partial_ord_impl.

AndrewKvalheim added a commit to AndrewKvalheim/little-a-map that referenced this issue Dec 30, 2023
@AndrewKvalheim
Copy link

Likely the same issue as #112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The crate does not work as expected
Projects
None yet
Development

No branches or pull requests

2 participants