Remove redundant Ord method impls. #8357
Conversation
|
The default methods for Ord aren't marked inline (I added them, so sorry!). We should mark them inline, but does it even take effect? |
|
@blake2-ppc: yeah, I think it works - this is going to be a performance regression at the moment |
|
Yes, the only concern in this patch is whether I should add |
|
OK, I'll fix them. Thank you. |
|
I opened #8360 about the existing issues with container |
|
@thestinger Great. Should I leave containers as it was until #8360 get a consensus? |
|
Well it doesn't really need a consensus - it's wrong and should be fixed. It's just not entirely clear to me how to fix it. You don't have to fix it as part of this PR though, we just shouldn't break more. |
|
@thestinger Sure. |
Basically, generic containers should not use the default methods since a type of elements may not guarantees total order. str could use them since u8's Ord guarantees total order. Floating point numbers are also broken with the default methods because of NaN. Thanks for @thestinger. Timespec also guarantees total order AIUI. I'm unsure whether extra::semver::Identifier does so I left it alone. Proof needed. Signed-off-by: OGINO Masanori <masanori.ogino@gmail.com>
|
Rebased to current master. r? |
I feel it's time to eliminate them (and add some testcases.)
I feel it's time to eliminate them (and add some testcases.)