Improve Ord violation help

[Voultapher]

Recent experience in #128083 showed that the panic message when an Ord violation is detected by the new sort implementations can be confusing. So this PR aims to improve it, together with minor bug fixes in the doc comments for sort*, sort_unstable* and select_nth_unstable*.

Is it possible to get these changes into the 1.81 release? It doesn't change behavior and would greatly help when users encounter this panic for the first time, which they may after upgrading to 1.81.

Tagging @orlp

[rustbot]

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[tgross35]

Thanks for the followup, this will be great to have.

For all the uses of "total order", could we link somewhere? Either the Wikipedia page, or link somewhere on the Ord page.

Maybe Ord should just get a "total order" section that shows an example incorrect implementation since it's not always easy to compare code to equations.

[workingjubilee]

I already anticipate someone is going to read this note and say "but that can't happen because K is Ord and Ord is a total order". This should be clearer about that case.

[workingjubilee]

Sorry, noticed some typos also.

[Voultapher]

Thanks for the followup, this will be great to have.

For all the uses of "total order", could we link somewhere? Either the Wikipedia page, or link somewhere on the Ord page.

Maybe Ord should just get a "total order" section that shows an example incorrect implementation since it's not always easy to compare code to equations.

Good idea, currently the Ord docs links to wikipedia, we could do the same. That said doing so might seem a little intimidating to users unfamiliar with the matter. I like the idea of having concrete code that demonstrates the ideas at play.

[Voultapher]

Changes since last review:

• Use if the implementation of [Ord] for T
  language
• Link to total order wiki page
• Rework total order help and examples
• Improve language to be more precise and less
  prone to misunderstandings.
• Fix usage of sort_unstable_by in sort_by
  example
• Fix missing author mention
• Use more consistent example input for sort
• Use more idiomatic assert_eq! in examples
• Use more natural "comparison function" language
  instead of "comparator function"
• Hide sort module

[Voultapher]

There are something like 10 places that document sort usage, so chances are I missed something, please carefully review for typos and mistakes.

[Voultapher]

I've also started making changes to the Ord docs to better help people understand how to correctly derive and implement them, but that has grown out of scope and I plan on creating a separate PR for it. The sort docs link people to relevant information and surface some of it where sensible, but it's a tricky balance between too little information and linking to the relevant places and overloading and duplicating information in the sort docs. I hope the current iteration strikes a decent balance between the two.

[workingjubilee]

This looks good. I noticed some remaining nits. They're just typos or non-blocking concerns because the original wording here was signed-off-on by libs already and they don't necessarily relate directly to the concern of the "total order" requirement.

[Voultapher]

@workingjubilee thanks for the detailed review so far.

[workingjubilee]

Okie dokie! I think that everything else is stuff that we can defer to later. Thank you everyone.

@bors r+

[bors]

📌 Commit 613155c has been approved by workingjubilee

It is now in the queue for this repository.

[matthiaskrgr]

@bors r-
guess this failed here: #128739 (comment)

[Voultapher]

While I can't reproduce the issue with ./x.py --stage 0 test linkchecker html-checker I made small tweak based on the error message that should hopefully fix the issue.

[workingjubilee]

@bors r+

[bors]

📌 Commit 1be60b5 has been approved by workingjubilee

It is now in the queue for this repository.

[Voultapher]

I see this PR hasn't made its way into the beta yet, what is needed for that?

[ChrisDenton]

Patience. Merging into beta is a manual process. This PR is labelled as beta-accepted so it'll be merged (possibly along with other beta-accepted PRs) the next time someone does the backports.