Rollup merge of rust-lang#138610 - oli-obk:no-sort-hir-ids, r=compiler-errors

impl !PartialOrd for HirId

revive of rust-lang#92233

Another checkbox of rust-lang#90317, another small step in making incremental less likely to die in horrible ways

Author: matthiaskrgr

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -959,9 +959,9 @@ fn link_natively(
                 }
             }
 
-            let (level, src) = codegen_results.crate_info.lint_levels.linker_messages;
+            let level = codegen_results.crate_info.lint_levels.linker_messages;
             let lint = |msg| {
-                lint_level(sess, LINKER_MESSAGES, level, src, None, |diag| {
+                lint_level(sess, LINKER_MESSAGES, level, None, |diag| {
                     LinkerOutput { inner: msg }.decorate_lint(diag)
                 })
             };

compiler/rustc_codegen_ssa/src/lib.rs

@@ -34,7 +34,7 @@ use rustc_hir::CRATE_HIR_ID;
 use rustc_hir::def_id::CrateNum;
 use rustc_macros::{Decodable, Encodable, HashStable};
 use rustc_middle::dep_graph::WorkProduct;
-use rustc_middle::lint::LintLevelSource;
+use rustc_middle::lint::LevelAndSource;
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
 use rustc_middle::middle::dependency_format::Dependencies;
 use rustc_middle::middle::exported_symbols::SymbolExportKind;
@@ -45,7 +45,6 @@ use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
 use rustc_session::Session;
 use rustc_session::config::{CrateType, OutputFilenames, OutputType, RUST_CGU_EXT};
 use rustc_session::cstore::{self, CrateSource};
-use rustc_session::lint::Level;
 use rustc_session::lint::builtin::LINKER_MESSAGES;
 use rustc_session::utils::NativeLibKind;
 use rustc_span::Symbol;
@@ -341,7 +340,7 @@ impl CodegenResults {
 /// Instead, encode exactly the information we need.
 #[derive(Copy, Clone, Debug, Encodable, Decodable)]
 pub struct CodegenLintLevels {
-    linker_messages: (Level, LintLevelSource),
+    linker_messages: LevelAndSource,
 }
 
 impl CodegenLintLevels {

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -546,7 +546,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                         rustc_session::lint::builtin::LONG_RUNNING_CONST_EVAL,
                         hir_id,
                     )
-                    .0
+                    .level
                     .is_error();
                 let span = ecx.cur_span();
                 ecx.tcx.emit_node_span_lint(

compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

@@ -91,13 +91,13 @@ fn annotation_level_for_level(level: Level) -> annotate_snippets::Level {
         Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => {
             annotate_snippets::Level::Error
         }
-        Level::ForceWarning(_) | Level::Warning => annotate_snippets::Level::Warning,
+        Level::ForceWarning | Level::Warning => annotate_snippets::Level::Warning,
         Level::Note | Level::OnceNote => annotate_snippets::Level::Note,
         Level::Help | Level::OnceHelp => annotate_snippets::Level::Help,
         // FIXME(#59346): Not sure how to map this level
         Level::FailureNote => annotate_snippets::Level::Error,
         Level::Allow => panic!("Should not call with Allow"),
-        Level::Expect(_) => panic!("Should not call with Expect"),
+        Level::Expect => panic!("Should not call with Expect"),
     }
 }
 

compiler/rustc_errors/src/diagnostic.rs

@@ -9,7 +9,7 @@ use std::thread::panicking;
 
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_error_messages::{FluentValue, fluent_value_from_str_list_sep_by_and};
-use rustc_lint_defs::Applicability;
+use rustc_lint_defs::{Applicability, LintExpectationId};
 use rustc_macros::{Decodable, Encodable};
 use rustc_span::source_map::Spanned;
 use rustc_span::{DUMMY_SP, Span, Symbol};
@@ -296,6 +296,7 @@ pub struct DiagInner {
 
     pub messages: Vec<(DiagMessage, Style)>,
     pub code: Option<ErrCode>,
+    pub lint_id: Option<LintExpectationId>,
     pub span: MultiSpan,
     pub children: Vec<Subdiag>,
     pub suggestions: Suggestions,
@@ -324,6 +325,7 @@ impl DiagInner {
     pub fn new_with_messages(level: Level, messages: Vec<(DiagMessage, Style)>) -> Self {
         DiagInner {
             level,
+            lint_id: None,
             messages,
             code: None,
             span: MultiSpan::new(),
@@ -346,15 +348,15 @@ impl DiagInner {
         match self.level {
             Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true,
 
-            Level::ForceWarning(_)
+            Level::ForceWarning
             | Level::Warning
             | Level::Note
             | Level::OnceNote
             | Level::Help
             | Level::OnceHelp
             | Level::FailureNote
             | Level::Allow
-            | Level::Expect(_) => false,
+            | Level::Expect => false,
         }
     }
 
@@ -365,7 +367,7 @@ impl DiagInner {
 
     pub(crate) fn is_force_warn(&self) -> bool {
         match self.level {
-            Level::ForceWarning(_) => {
+            Level::ForceWarning => {
                 assert!(self.is_lint.is_some());
                 true
             }
@@ -1259,6 +1261,17 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
         self
     } }
 
+    with_fn! { with_lint_id,
+    /// Add an argument.
+    #[rustc_lint_diagnostics]
+    pub fn lint_id(
+        &mut self,
+        id: LintExpectationId,
+    ) -> &mut Self {
+        self.lint_id = Some(id);
+        self
+    } }
+
     with_fn! { with_primary_message,
     /// Add a primary message.
     #[rustc_lint_diagnostics]

compiler/rustc_errors/src/json.rs

@@ -144,7 +144,7 @@ impl Emitter for JsonEmitter {
                 //
                 // So to avoid ICEs and confused users we "upgrade" the lint level for
                 // those `FutureBreakageItem` to warn.
-                if matches!(diag.level, crate::Level::Allow | crate::Level::Expect(..)) {
+                if matches!(diag.level, crate::Level::Allow | crate::Level::Expect) {
                     diag.level = crate::Level::Warning;
                 }
                 FutureBreakageItem {

compiler/rustc_errors/src/lib.rs

@@ -905,8 +905,8 @@ impl<'a> DiagCtxtHandle<'a> {
             DelayedBug => {
                 return self.inner.borrow_mut().emit_diagnostic(diag, self.tainted_with_errors);
             }
-            ForceWarning(_) | Warning | Note | OnceNote | Help | OnceHelp | FailureNote | Allow
-            | Expect(_) => None,
+            ForceWarning | Warning | Note | OnceNote | Help | OnceHelp | FailureNote | Allow
+            | Expect => None,
         };
 
         // FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic
@@ -1045,7 +1045,7 @@ impl<'a> DiagCtxtHandle<'a> {
                 // Use `ForceWarning` rather than `Warning` to guarantee emission, e.g. with a
                 // configuration like `--cap-lints allow --force-warn bare_trait_objects`.
                 inner.emit_diagnostic(
-                    DiagInner::new(ForceWarning(None), DiagMessage::Str(warnings)),
+                    DiagInner::new(ForceWarning, DiagMessage::Str(warnings)),
                     None,
                 );
             }
@@ -1450,7 +1450,7 @@ impl<'a> DiagCtxtHandle<'a> {
     #[rustc_lint_diagnostics]
     #[track_caller]
     pub fn struct_expect(self, msg: impl Into<DiagMessage>, id: LintExpectationId) -> Diag<'a, ()> {
-        Diag::new(self, Expect(id), msg)
+        Diag::new(self, Expect, msg).with_lint_id(id)
     }
 }
 
@@ -1510,7 +1510,7 @@ impl DiagCtxtInner {
             // Future breakages aren't emitted if they're `Level::Allow` or
             // `Level::Expect`, but they still need to be constructed and
             // stashed below, so they'll trigger the must_produce_diag check.
-            assert_matches!(diagnostic.level, Error | Warning | Allow | Expect(_));
+            assert_matches!(diagnostic.level, Error | Warning | Allow | Expect);
             self.future_breakage_diagnostics.push(diagnostic.clone());
         }
 
@@ -1558,7 +1558,7 @@ impl DiagCtxtInner {
                     };
                 }
             }
-            ForceWarning(None) => {} // `ForceWarning(Some(...))` is below, with `Expect`
+            ForceWarning if diagnostic.lint_id.is_none() => {} // `ForceWarning(Some(...))` is below, with `Expect`
             Warning => {
                 if !self.flags.can_emit_warnings {
                     // We are not emitting warnings.
@@ -1580,9 +1580,9 @@ impl DiagCtxtInner {
                 }
                 return None;
             }
-            Expect(expect_id) | ForceWarning(Some(expect_id)) => {
-                self.fulfilled_expectations.insert(expect_id);
-                if let Expect(_) = diagnostic.level {
+            Expect | ForceWarning => {
+                self.fulfilled_expectations.insert(diagnostic.lint_id.unwrap());
+                if let Expect = diagnostic.level {
                     // Nothing emitted here for expected lints.
                     TRACK_DIAGNOSTIC(diagnostic, &mut |_| None);
                     self.suppressed_expected_diag = true;
@@ -1631,7 +1631,7 @@ impl DiagCtxtInner {
 
                 if is_error {
                     self.deduplicated_err_count += 1;
-                } else if matches!(diagnostic.level, ForceWarning(_) | Warning) {
+                } else if matches!(diagnostic.level, ForceWarning | Warning) {
                     self.deduplicated_warn_count += 1;
                 }
                 self.has_printed = true;
@@ -1899,9 +1899,9 @@ pub enum Level {
     /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation
     /// from finishing.
     ///
-    /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this
+    /// Requires a [`LintExpectationId`] for expected lint diagnostics. In all other cases this
     /// should be `None`.
-    ForceWarning(Option<LintExpectationId>),
+    ForceWarning,
 
     /// A warning about the code being compiled. Does not prevent compilation from finishing.
     /// Will be skipped if `can_emit_warnings` is false.
@@ -1926,8 +1926,8 @@ pub enum Level {
     /// Only used for lints.
     Allow,
 
-    /// Only used for lints.
-    Expect(LintExpectationId),
+    /// Only used for lints. Requires a [`LintExpectationId`] for silencing the lints.
+    Expect,
 }
 
 impl fmt::Display for Level {
@@ -1943,7 +1943,7 @@ impl Level {
             Bug | Fatal | Error | DelayedBug => {
                 spec.set_fg(Some(Color::Red)).set_intense(true);
             }
-            ForceWarning(_) | Warning => {
+            ForceWarning | Warning => {
                 spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
             }
             Note | OnceNote => {
@@ -1953,7 +1953,7 @@ impl Level {
                 spec.set_fg(Some(Color::Cyan)).set_intense(true);
             }
             FailureNote => {}
-            Allow | Expect(_) => unreachable!(),
+            Allow | Expect => unreachable!(),
         }
         spec
     }
@@ -1962,11 +1962,11 @@ impl Level {
         match self {
             Bug | DelayedBug => "error: internal compiler error",
             Fatal | Error => "error",
-            ForceWarning(_) | Warning => "warning",
+            ForceWarning | Warning => "warning",
             Note | OnceNote => "note",
             Help | OnceHelp => "help",
             FailureNote => "failure-note",
-            Allow | Expect(_) => unreachable!(),
+            Allow | Expect => unreachable!(),
         }
     }
 
@@ -1977,8 +1977,7 @@ impl Level {
     // Can this level be used in a subdiagnostic message?
     fn can_be_subdiag(&self) -> bool {
         match self {
-            Bug | DelayedBug | Fatal | Error | ForceWarning(_) | FailureNote | Allow
-            | Expect(_) => false,
+            Bug | DelayedBug | Fatal | Error | ForceWarning | FailureNote | Allow | Expect => false,
 
             Warning | Note | Help | OnceNote | OnceHelp => true,
         }

compiler/rustc_hir/src/hir_id.rs

@@ -83,6 +83,12 @@ pub struct HirId {
     pub local_id: ItemLocalId,
 }
 
+// To ensure correctness of incremental compilation,
+// `HirId` must not implement `Ord` or `PartialOrd`.
+// See https://github.com/rust-lang/rust/issues/90317.
+impl !Ord for HirId {}
+impl !PartialOrd for HirId {}
+
 impl Debug for HirId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         // Example: HirId(DefId(0:1 ~ aa[7697]::{use#0}).10)
@@ -116,10 +122,6 @@ impl HirId {
     pub fn make_owner(owner: LocalDefId) -> Self {
         Self { owner: OwnerId { def_id: owner }, local_id: ItemLocalId::ZERO }
     }
-
-    pub fn index(self) -> (usize, usize) {
-        (rustc_index::Idx::index(self.owner.def_id), rustc_index::Idx::index(self.local_id))
-    }
 }
 
 impl fmt::Display for HirId {
@@ -128,18 +130,6 @@ impl fmt::Display for HirId {
     }
 }
 
-impl Ord for HirId {
-    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
-        (self.index()).cmp(&(other.index()))
-    }
-}
-
-impl PartialOrd for HirId {
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
 rustc_data_structures::define_stable_id_collections!(HirIdMap, HirIdSet, HirIdMapEntry, HirId);
 rustc_data_structures::define_id_collections!(
     ItemLocalMap,

compiler/rustc_hir/src/lib.rs

@@ -11,6 +11,7 @@
 #![feature(debug_closure_helpers)]
 #![feature(exhaustive_patterns)]
 #![feature(let_chains)]
+#![feature(negative_impls)]
 #![feature(never_type)]
 #![feature(rustc_attrs)]
 #![feature(variant_count)]

compiler/rustc_hir_typeck/src/upvar.rs

@@ -85,7 +85,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 /// Intermediate format to store the hir_id pointing to the use that resulted in the
 /// corresponding place being captured and a String which contains the captured value's
 /// name (i.e: a.b.c)
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 enum UpvarMigrationInfo {
     /// We previously captured all of `x`, but now we capture some sub-path.
     CapturingPrecise { source_expr: Option<HirId>, var_name: String },
@@ -1396,14 +1396,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 FxIndexSet::default()
             };
 
-            // Combine all the captures responsible for needing migrations into one HashSet
+            // Combine all the captures responsible for needing migrations into one IndexSet
             let mut capture_diagnostic = drop_reorder_diagnostic.clone();
             for key in auto_trait_diagnostic.keys() {
                 capture_diagnostic.insert(key.clone());
             }
 
             let mut capture_diagnostic = capture_diagnostic.into_iter().collect::<Vec<_>>();
-            capture_diagnostic.sort();
+            capture_diagnostic.sort_by_cached_key(|info| match info {
+                UpvarMigrationInfo::CapturingPrecise { source_expr: _, var_name } => {
+                    (0, Some(var_name.clone()))
+                }
+                UpvarMigrationInfo::CapturingNothing { use_span: _ } => (1, None),
+            });
             for captures_info in capture_diagnostic {
                 // Get the auto trait reasons of why migration is needed because of that capture, if there are any
                 let capture_trait_reasons =
@@ -2325,8 +2330,9 @@ fn should_do_rust_2021_incompatible_closure_captures_analysis(
         return false;
     }
 
-    let (level, _) =
-        tcx.lint_level_at_node(lint::builtin::RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES, closure_id);
+    let level = tcx
+        .lint_level_at_node(lint::builtin::RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES, closure_id)
+        .level;
 
     !matches!(level, lint::Level::Allow)
 }