Remove track_errors.

`track_errors` returns `Err` if any errors were issued during the
execution of the closure. It uses a global count to determine this and
`span_delayed_debug` to provide the `ErrorGuaranteed`, which is a hacky
way to do it.

There are four calls to it.
- Three of them are to simple checking functions where it's easy to just
  return `Result<(), ErrorGuaranteed>`, which this commit does.
- The fourth is much harder to fix, but it's the only remaining call, so
  this commit inlines and removes `track_errors`.

Author: nnethercote

compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs

@@ -1,4 +1,4 @@
-use rustc_errors::StashKey;
+use rustc_errors::{ErrorGuaranteed, StashKey};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID};
 use rustc_hir::intravisit::{self, Visitor};
@@ -9,16 +9,19 @@ use rustc_span::{sym, DUMMY_SP};
 
 use crate::errors::{TaitForwardCompat, TypeOf, UnconstrainedOpaqueType};
 
-pub fn test_opaque_hidden_types(tcx: TyCtxt<'_>) {
+pub fn test_opaque_hidden_types(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
+    let mut result = Ok(());
     if tcx.has_attr(CRATE_DEF_ID, sym::rustc_hidden_type_of_opaques) {
         for id in tcx.hir().items() {
             if matches!(tcx.def_kind(id.owner_id), DefKind::OpaqueTy) {
                 let type_of = tcx.type_of(id.owner_id).instantiate_identity();
 
-                tcx.dcx().emit_err(TypeOf { span: tcx.def_span(id.owner_id), type_of });
+                result =
+                    Err(tcx.dcx().emit_err(TypeOf { span: tcx.def_span(id.owner_id), type_of }));
             }
         }
     }
+    result
 }
 
 /// Checks "defining uses" of opaque `impl Trait` types to ensure that they meet the restrictions

compiler/rustc_hir_analysis/src/lib.rs

@@ -169,38 +169,40 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
     // FIXME(matthewjasper) We shouldn't need to use `track_errors` anywhere in this function
     // or the compiler in general.
     if tcx.features().rustc_attrs {
-        tcx.sess.track_errors(|| {
-            tcx.sess.time("outlives_testing", || outlives::test::test_inferred_outlives(tcx));
-        })?;
+        tcx.sess.time("outlives_testing", || outlives::test::test_inferred_outlives(tcx))?;
     }
 
-    tcx.sess.track_errors(|| {
-        tcx.sess.time("coherence_checking", || {
-            // Check impls constrain their parameters
-            tcx.hir().for_each_module(|module| tcx.ensure().check_mod_impl_wf(module));
-
-            for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
-                tcx.ensure().coherent_trait(trait_def_id);
-            }
+    // FIXME: it would be better to bubble up `ErrorGuaranteed` results for
+    // evidence of errors, rather than using the global count, but that's not
+    // easy because there are many checking functions that get called within
+    // this block.
+    let old_err_count = tcx.dcx().err_count();
+    tcx.sess.time("coherence_checking", || {
+        // Check impls constrain their parameters
+        tcx.hir().for_each_module(|module| tcx.ensure().check_mod_impl_wf(module));
+
+        for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
+            tcx.ensure().coherent_trait(trait_def_id);
+        }
 
-            // these queries are executed for side-effects (error reporting):
-            tcx.ensure().crate_inherent_impls(());
-            tcx.ensure().crate_inherent_impls_overlap_check(());
-        });
-    })?;
+        // these queries are executed for side-effects (error reporting):
+        tcx.ensure().crate_inherent_impls(());
+        tcx.ensure().crate_inherent_impls_overlap_check(());
+    });
+    if tcx.dcx().err_count() != old_err_count {
+        return Err(tcx.dcx().delayed_bug("`check_crate` counted an error but no error emitted"));
+    }
 
     if tcx.features().rustc_attrs {
-        tcx.sess.track_errors(|| {
-            tcx.sess.time("variance_testing", || variance::test::test_variance(tcx));
-        })?;
+        tcx.sess.time("variance_testing", || variance::test::test_variance(tcx))?;
     }
 
     tcx.sess.time("wf_checking", || {
         tcx.hir().try_par_for_each_module(|module| tcx.ensure().check_mod_type_wf(module))
     })?;
 
     if tcx.features().rustc_attrs {
-        tcx.sess.track_errors(|| collect::test_opaque_hidden_types(tcx))?;
+        collect::test_opaque_hidden_types(tcx)?;
     }
 
     // Freeze definitions as we don't add new ones at this point. This improves performance by

compiler/rustc_hir_analysis/src/outlives/test.rs

@@ -1,7 +1,8 @@
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::symbol::sym;
 
-pub fn test_inferred_outlives(tcx: TyCtxt<'_>) {
+pub fn test_inferred_outlives(tcx: TyCtxt<'_>) -> Result<(), rustc_errors::ErrorGuaranteed> {
+    let mut result = Ok(());
     for id in tcx.hir().items() {
         // For unit testing: check for a special "rustc_outlives"
         // attribute and report an error with various results if found.
@@ -22,7 +23,8 @@ pub fn test_inferred_outlives(tcx: TyCtxt<'_>) {
             for p in pred {
                 err.note(p);
             }
-            err.emit();
+            result = Err(err.emit());
         }
     }
+    result
 }

compiler/rustc_hir_analysis/src/variance/test.rs

@@ -5,16 +5,17 @@ use rustc_span::symbol::sym;
 
 use crate::errors;
 
-pub fn test_variance(tcx: TyCtxt<'_>) {
+pub fn test_variance(tcx: TyCtxt<'_>) -> Result<(), rustc_errors::ErrorGuaranteed> {
+    let mut result = Ok(());
     if tcx.has_attr(CRATE_DEF_ID, sym::rustc_variance_of_opaques) {
         for id in tcx.hir().items() {
             if matches!(tcx.def_kind(id.owner_id), DefKind::OpaqueTy) {
                 let variances_of = tcx.variances_of(id.owner_id);
 
-                tcx.dcx().emit_err(errors::VariancesOf {
+                result = Err(tcx.dcx().emit_err(errors::VariancesOf {
                     span: tcx.def_span(id.owner_id),
                     variances_of: format!("{variances_of:?}"),
-                });
+                }));
             }
         }
     }
@@ -25,10 +26,11 @@ pub fn test_variance(tcx: TyCtxt<'_>) {
         if tcx.has_attr(id.owner_id, sym::rustc_variance) {
             let variances_of = tcx.variances_of(id.owner_id);
 
-            tcx.dcx().emit_err(errors::VariancesOf {
+            result = Err(tcx.dcx().emit_err(errors::VariancesOf {
                 span: tcx.def_span(id.owner_id),
                 variances_of: format!("{variances_of:?}"),
-            });
+            }));
         }
     }
+    result
 }

compiler/rustc_session/src/session.rs

@@ -331,20 +331,6 @@ impl Session {
         }
     }
 
-    // FIXME(matthewjasper) Remove this method, it should never be needed.
-    pub fn track_errors<F, T>(&self, f: F) -> Result<T, ErrorGuaranteed>
-    where
-        F: FnOnce() -> T,
-    {
-        let old_count = self.dcx().err_count();
-        let result = f();
-        if self.dcx().err_count() == old_count {
-            Ok(result)
-        } else {
-            Err(self.dcx().delayed_bug("`self.err_count()` changed but an error was not emitted"))
-        }
-    }
-
     /// Used for code paths of expensive computations that should only take place when
     /// warnings or errors are emitted. If no messages are emitted ("good path"), then
     /// it's likely a bug.