Fix Ord violations in rustdoc items sorting

Author: GuillaumeGomez

src/librustdoc/html/render/print_item.rs

@@ -316,7 +316,8 @@ trait ItemTemplate<'a, 'cx: 'a>: rinja::Template + fmt::Display {
 fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items: &[clean::Item]) {
     write!(w, "{}", document(cx, item, None, HeadingOffset::H2));
 
-    let mut indices = (0..items.len()).filter(|i| !items[*i].is_stripped()).collect::<Vec<usize>>();
+    let mut not_stripped_items =
+        items.iter().filter(|i| !i.is_stripped()).enumerate().collect::<Vec<_>>();
 
     // the order of item types in the listing
     fn reorder(ty: ItemType) -> u8 {
@@ -338,37 +339,30 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
         }
     }
 
-    fn cmp(
-        i1: &clean::Item,
-        i2: &clean::Item,
-        idx1: usize,
-        idx2: usize,
-        tcx: TyCtxt<'_>,
-    ) -> Ordering {
+    fn cmp(i1: &clean::Item, i2: &clean::Item, tcx: TyCtxt<'_>) -> Ordering {
         let ty1 = i1.type_();
         let ty2 = i2.type_();
-        if item_ty_to_section(ty1) != item_ty_to_section(ty2)
-            || (ty1 != ty2 && (ty1 == ItemType::ExternCrate || ty2 == ItemType::ExternCrate))
-        {
-            return (reorder(ty1), idx1).cmp(&(reorder(ty2), idx2));
-        }
-        let s1 = i1.stability(tcx).as_ref().map(|s| s.level);
-        let s2 = i2.stability(tcx).as_ref().map(|s| s.level);
-        if let (Some(a), Some(b)) = (s1, s2) {
-            match (a.is_stable(), b.is_stable()) {
-                (true, true) | (false, false) => {}
-                (false, true) => return Ordering::Greater,
-                (true, false) => return Ordering::Less,
+        if ty1 != ty2 {
+            return reorder(ty1).cmp(&reorder(ty2));
+        }
+        let is_stable1 = i1.stability(tcx).as_ref().map(|s| s.level.is_stable()).unwrap_or(true);
+        let is_stable2 = i2.stability(tcx).as_ref().map(|s| s.level.is_stable()).unwrap_or(true);
+        if is_stable1 != is_stable2 {
+            if !is_stable1 {
+                return Ordering::Greater;
             }
+            return Ordering::Less;
         }
         let lhs = i1.name.unwrap_or(kw::Empty);
         let rhs = i2.name.unwrap_or(kw::Empty);
         compare_names(lhs.as_str(), rhs.as_str())
     }
 
+    let tcx = cx.tcx();
+
     match cx.shared.module_sorting {
         ModuleSorting::Alphabetical => {
-            indices.sort_by(|&i1, &i2| cmp(&items[i1], &items[i2], i1, i2, cx.tcx()));
+            not_stripped_items.sort_by(|(_, i1), (_, i2)| cmp(i1, i2, tcx));
         }
         ModuleSorting::DeclarationOrder => {}
     }
@@ -391,24 +385,19 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
     // can be identical even if the elements are different (mostly in imports).
     // So in case this is an import, we keep everything by adding a "unique id"
     // (which is the position in the vector).
-    indices.dedup_by_key(|i| {
+    not_stripped_items.dedup_by_key(|(idx, i)| {
         (
-            items[*i].item_id,
-            if items[*i].name.is_some() { Some(full_path(cx, &items[*i])) } else { None },
-            items[*i].type_(),
-            if items[*i].is_import() { *i } else { 0 },
+            i.item_id,
+            if i.name.is_some() { Some(full_path(cx, i)) } else { None },
+            i.type_(),
+            if i.is_import() { *idx } else { 0 },
         )
     });
 
-    debug!("{indices:?}");
+    debug!("{not_stripped_items:?}");
     let mut last_section = None;
 
-    for &idx in &indices {
-        let myitem = &items[idx];
-        if myitem.is_stripped() {
-            continue;
-        }
-
+    for (_, myitem) in &not_stripped_items {
         let my_section = item_ty_to_section(myitem.type_());
         if Some(my_section) != last_section {
             if last_section.is_some() {
@@ -424,7 +413,6 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
             );
         }
 
-        let tcx = cx.tcx();
         match *myitem.kind {
             clean::ExternCrateItem { ref src } => {
                 use crate::html::format::anchor;
@@ -453,7 +441,7 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
                 let stab_tags = if let Some(import_def_id) = import.source.did {
                     // Just need an item with the correct def_id and attrs
                     let import_item =
-                        clean::Item { item_id: import_def_id.into(), ..myitem.clone() };
+                        clean::Item { item_id: import_def_id.into(), ..(*myitem).clone() };
 
                     let stab_tags = Some(extra_info_tags(&import_item, item, tcx).to_string());
                     stab_tags
@@ -2026,19 +2014,28 @@ pub(crate) fn compare_names(mut lhs: &str, mut rhs: &str) -> Ordering {
         let (ra, rb) = take_parts(&mut rhs);
         // First process the non-numeric part.
         match la.cmp(ra) {
-            Ordering::Equal => (),
+            Ordering::Equal => {}
             x => return x,
         }
+        match (lb.is_empty(), rb.is_empty()) {
+            (true, false) => return Ordering::Less,
+            (false, true) => return Ordering::Greater,
+            (true, true) => continue,
+            (false, false) => {}
+        }
         // Then process the numeric part, if both sides have one (and they fit in a u64).
-        if let (Ok(ln), Ok(rn)) = (lb.parse::<u64>(), rb.parse::<u64>()) {
-            match ln.cmp(&rn) {
-                Ordering::Equal => (),
+        match (lb.parse::<u64>(), rb.parse::<u64>()) {
+            (Err(_), Err(_)) => return Ordering::Equal,
+            (Ok(_), Err(_)) => return Ordering::Less,
+            (Err(_), Ok(_)) => return Ordering::Greater,
+            (Ok(nb1), Ok(nb2)) => match nb1.cmp(&nb2) {
+                Ordering::Equal => {}
                 x => return x,
-            }
+            },
         }
-        // Then process the numeric part again, but this time as strings.
+        // If both numbers are equal, then we compare the strings.
         match lb.cmp(rb) {
-            Ordering::Equal => (),
+            Ordering::Equal => {}
             x => return x,
         }
     }

src/librustdoc/html/render/tests.rs

@@ -9,7 +9,7 @@ fn test_compare_names() {
         ("hello", "world"),
         ("", "world"),
         ("123", "hello"),
-        ("123", ""),
+        // ("123", ""),
         ("123test", "123"),
         ("hello", ""),
         ("hello", "hello"),