Total sort order for redactions

[mitsuhiko]

Followup to #586 there is another case in the redaction code that is most likely also wrong. It pairs sort_by with unwrap_or.

[max-sixty]

From this code?

#[cfg_attr(docsrs, doc(cfg(feature = "redactions")))]
pub fn sorted_redaction() -> Redaction {
    fn sort(mut value: Content, _path: ContentPath) -> Content {
        match value.resolve_inner_mut() {
            Content::Seq(ref mut val) => {
                val.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal))
            }
            Content::Map(ref mut val) => {
                val.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal))
            }
            Content::Struct(_, ref mut fields)
            | Content::StructVariant(_, _, _, ref mut fields) => {
                fields.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal))
            }
            _ => {}
        }
        value
    }
    dynamic_redaction(sort)
}

I don't feel confident here, but what's the case for this not being a total order? That a.partial_cmp(b) is different from b.partial_cmp(a)? I would have thought it's fine that things are occasionally not comparable and treated as equal...

[mitsuhiko]

It would be for floats containing NaNs in vectors.

[max-sixty]

OK, I had thought those consistently evaluate to Equal given .unwrap_or(std::cmp::Ordering::Equal), but I'm surely missing something