Auto merge of #153383 - nnethercote:overhaul-ensure_ok, r=Zalathar

Overhaul `ensure_ok`

The interaction of `ensure_ok` and the `return_result_from_ensure_ok` query modifier is weird and hacky. This PR cleans it up. Details in the individual commits.

r? @Zalathar

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -294,7 +294,7 @@ fn has_ambiguous_copy(&mut self, ty: Ty<'tcx>) -> bool {
 
         // Avoid bogus move errors because of an incoherent `Copy` impl.
         self.infcx.type_implements_trait(copy_def_id, [ty], self.infcx.param_env).may_apply()
-            && self.infcx.tcx.coherent_trait(copy_def_id).is_err()
+            && self.infcx.tcx.ensure_result().coherent_trait(copy_def_id).is_err()
     }
 
     fn report_cannot_move_from_static(&mut self, place: Place<'tcx>, span: Span) -> Diag<'infcx> {

compiler/rustc_borrowck/src/lib.rs

@@ -121,7 +121,7 @@ fn mir_borrowck(
     // We should eagerly check stalled coroutine obligations from HIR typeck.
     // Not doing so leads to silent normalization failures later, which will
     // fail to register opaque types in the next solver.
-    tcx.check_coroutine_obligations(def)?;
+    tcx.ensure_result().check_coroutine_obligations(def)?;
 
     let input_body: &Body<'_> = &input_body.borrow();
     if let Some(guar) = input_body.tainted_by_errors {

compiler/rustc_const_eval/src/check_consts/check.rs

@@ -334,7 +334,7 @@ fn check_static(&mut self, def_id: DefId, span: Span) {
             self.tcx.dcx().span_bug(span, "tls access is checked in `Rvalue::ThreadLocalRef`");
         }
         if let Some(def_id) = def_id.as_local()
-            && let Err(guar) = self.tcx.ensure_ok().check_well_formed(hir::OwnerId { def_id })
+            && let Err(guar) = self.tcx.ensure_result().check_well_formed(hir::OwnerId { def_id })
         {
             self.error_emitted = Some(guar);
         }

compiler/rustc_hir_analysis/src/check/always_applicable.rs

@@ -52,7 +52,7 @@ pub(crate) fn check_drop_impl(
         }
     }
 
-    tcx.ensure_ok().orphan_check_impl(drop_impl_did)?;
+    tcx.ensure_result().orphan_check_impl(drop_impl_did)?;
 
     let self_ty = tcx.type_of(drop_impl_did).instantiate_identity();
 
@@ -96,7 +96,7 @@ pub(crate) fn check_negative_auto_trait_impl<'tcx>(
         tcx.dcx().span_delayed_bug(tcx.def_span(impl_def_id), "default impl cannot be negative");
     }
 
-    tcx.ensure_ok().orphan_check_impl(impl_def_id)?;
+    tcx.ensure_result().orphan_check_impl(impl_def_id)?;
 
     match impl_trait_ref.self_ty().kind() {
         ty::Adt(adt_def, adt_to_impl_args) => {

compiler/rustc_hir_analysis/src/check/check.rs

@@ -825,7 +825,7 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(),
             if of_trait {
                 let impl_trait_header = tcx.impl_trait_header(def_id);
                 res = res.and(
-                    tcx.ensure_ok()
+                    tcx.ensure_result()
                         .coherent_trait(impl_trait_header.trait_ref.instantiate_identity().def_id),
                 );
 
@@ -1256,8 +1256,7 @@ fn check_impl_items_against_trait<'tcx>(
             Err(ErrorGuaranteed { .. }) => continue,
         };
 
-        let res = tcx.ensure_ok().compare_impl_item(impl_item.expect_local());
-
+        let res = tcx.ensure_result().compare_impl_item(impl_item.expect_local());
         if res.is_ok() {
             match ty_impl_item.kind {
                 ty::AssocKind::Fn { .. } => {

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -2458,7 +2458,7 @@ pub(super) fn check_type_bounds<'tcx>(
 ) -> Result<(), ErrorGuaranteed> {
     // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case
     // other `Foo` impls are incoherent.
-    tcx.ensure_ok().coherent_trait(impl_trait_ref.def_id)?;
+    tcx.ensure_result().coherent_trait(impl_trait_ref.def_id)?;
 
     let param_env = tcx.param_env(impl_ty.def_id);
     debug!(?param_env);

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -936,7 +936,7 @@ pub(crate) fn check_associated_item(
 
         // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case
         // other `Foo` impls are incoherent.
-        tcx.ensure_ok().coherent_trait(tcx.parent(item.trait_item_or_self()?))?;
+        tcx.ensure_result().coherent_trait(tcx.parent(item.trait_item_or_self()?))?;
 
         let self_ty = match item.container {
             ty::AssocContainer::Trait => tcx.types.self_param,
@@ -1328,9 +1328,9 @@ fn check_impl<'tcx>(
                 // therefore don't need to be WF (the trait's `Self: Trait` predicate
                 // won't hold).
                 let trait_ref = tcx.impl_trait_ref(item.owner_id).instantiate_identity();
-                // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case
-                // other `Foo` impls are incoherent.
-                tcx.ensure_ok().coherent_trait(trait_ref.def_id)?;
+                // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in
+                // case other `Foo` impls are incoherent.
+                tcx.ensure_result().coherent_trait(trait_ref.def_id)?;
                 let trait_span = of_trait.trait_ref.path.span;
                 let trait_ref = wfcx.deeply_normalize(
                     trait_span,
@@ -2334,15 +2334,22 @@ fn check_false_global_bounds(&mut self) {
 
 pub(super) fn check_type_wf(tcx: TyCtxt<'_>, (): ()) -> Result<(), ErrorGuaranteed> {
     let items = tcx.hir_crate_items(());
-    let res = items
-        .par_items(|item| tcx.ensure_ok().check_well_formed(item.owner_id.def_id))
-        .and(items.par_impl_items(|item| tcx.ensure_ok().check_well_formed(item.owner_id.def_id)))
-        .and(items.par_trait_items(|item| tcx.ensure_ok().check_well_formed(item.owner_id.def_id)))
-        .and(
-            items.par_foreign_items(|item| tcx.ensure_ok().check_well_formed(item.owner_id.def_id)),
-        )
-        .and(items.par_nested_bodies(|item| tcx.ensure_ok().check_well_formed(item)))
-        .and(items.par_opaques(|item| tcx.ensure_ok().check_well_formed(item)));
+    let res =
+        items
+            .par_items(|item| tcx.ensure_result().check_well_formed(item.owner_id.def_id))
+            .and(
+                items.par_impl_items(|item| {
+                    tcx.ensure_result().check_well_formed(item.owner_id.def_id)
+                }),
+            )
+            .and(items.par_trait_items(|item| {
+                tcx.ensure_result().check_well_formed(item.owner_id.def_id)
+            }))
+            .and(items.par_foreign_items(|item| {
+                tcx.ensure_result().check_well_formed(item.owner_id.def_id)
+            }))
+            .and(items.par_nested_bodies(|item| tcx.ensure_result().check_well_formed(item)))
+            .and(items.par_opaques(|item| tcx.ensure_result().check_well_formed(item)));
     super::entry::check_for_entry_fn(tcx);
 
     res

compiler/rustc_hir_analysis/src/coherence/builtin.rs

@@ -226,7 +226,7 @@ fn visit_implementation_of_coerce_unsized(checker: &Checker<'_>) -> Result<(), E
     // Just compute this for the side-effects, in particular reporting
     // errors; other parts of the code may demand it for the info of
     // course.
-    tcx.ensure_ok().coerce_unsized_info(impl_did)
+    tcx.ensure_result().coerce_unsized_info(impl_did)
 }
 
 fn is_from_coerce_pointee_derive(tcx: TyCtxt<'_>, span: Span) -> bool {
@@ -259,7 +259,7 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
         tcx.require_lang_item(LangItem::CoerceUnsized, span),
         source,
         |impl_def_id| {
-            res = res.and(tcx.ensure_ok().coerce_unsized_info(impl_def_id));
+            res = res.and(tcx.ensure_result().coerce_unsized_info(impl_def_id));
         },
     );
     res?;

compiler/rustc_hir_analysis/src/coherence/mod.rs

@@ -160,7 +160,7 @@ fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) -> Result<(), ErrorGuaranteed>
     }
     // Trigger building the specialization graph for the trait. This will detect and report any
     // overlap errors.
-    let mut res = tcx.ensure_ok().specialization_graph_of(def_id);
+    let mut res = tcx.ensure_result().specialization_graph_of(def_id);
 
     for &impl_def_id in impls {
         let impl_header = tcx.impl_trait_header(impl_def_id);
@@ -171,7 +171,7 @@ fn coherent_trait(tcx: TyCtxt<'_>, def_id: DefId) -> Result<(), ErrorGuaranteed>
             .and(check_impl(tcx, impl_def_id, trait_ref, trait_def, impl_header.polarity))
             .and(check_object_overlap(tcx, impl_def_id, trait_ref))
             .and(unsafety::check_item(tcx, impl_def_id, impl_header, trait_def))
-            .and(tcx.ensure_ok().orphan_check_impl(impl_def_id))
+            .and(tcx.ensure_result().orphan_check_impl(impl_def_id))
             .and(builtin::check_trait(tcx, def_id, impl_def_id, impl_header));
     }
 

compiler/rustc_hir_analysis/src/impl_wf_check.rs

@@ -63,7 +63,7 @@ pub(crate) fn check_impl_wf(
     // Check that the args are constrained. We queryfied the check for ty/const params
     // since unconstrained type/const params cause ICEs in projection, so we want to
     // detect those specifically and project those to `TyKind::Error`.
-    let mut res = tcx.ensure_ok().enforce_impl_non_lifetime_params_are_constrained(impl_def_id);
+    let mut res = tcx.ensure_result().enforce_impl_non_lifetime_params_are_constrained(impl_def_id);
     res = res.and(enforce_impl_lifetime_params_are_constrained(tcx, impl_def_id, of_trait));
 
     if of_trait && tcx.features().min_specialization() {

compiler/rustc_hir_analysis/src/lib.rs

@@ -172,14 +172,14 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
         // what we are intending to discard, to help future type-based refactoring.
         type R = Result<(), ErrorGuaranteed>;
 
-        let _: R = tcx.ensure_ok().check_type_wf(());
+        let _: R = tcx.ensure_result().check_type_wf(());
 
         for &trait_def_id in tcx.all_local_trait_impls(()).keys() {
-            let _: R = tcx.ensure_ok().coherent_trait(trait_def_id);
+            let _: R = tcx.ensure_result().coherent_trait(trait_def_id);
         }
         // these queries are executed for side-effects (error reporting):
-        let _: R = tcx.ensure_ok().crate_inherent_impls_validity_check(());
-        let _: R = tcx.ensure_ok().crate_inherent_impls_overlap_check(());
+        let _: R = tcx.ensure_result().crate_inherent_impls_validity_check(());
+        let _: R = tcx.ensure_result().crate_inherent_impls_overlap_check(());
     });
 
     tcx.sess.time("emit_ast_lowering_delayed_lints", || {

compiler/rustc_hir_typeck/src/callee.rs

@@ -50,7 +50,7 @@ pub(crate) fn check_legal_trait_for_method_call(
         };
         return Err(tcx.dcx().emit_err(errors::ExplicitDestructorCall { span, sugg }));
     }
-    tcx.ensure_ok().coherent_trait(trait_id)
+    tcx.ensure_result().coherent_trait(trait_id)
 }
 
 #[derive(Debug)]

compiler/rustc_hir_typeck/src/coercion.rs

@@ -778,7 +778,7 @@ fn coerce_unsized_old_solver(
                     // for local impls, since upstream impls should be valid.
                     if impl_source.impl_def_id.is_local()
                         && let Err(guar) =
-                            self.tcx.ensure_ok().coerce_unsized_info(impl_source.impl_def_id)
+                            self.tcx.ensure_result().coerce_unsized_info(impl_source.impl_def_id)
                     {
                         self.fcx.set_tainted_by_errors(guar);
                     }

compiler/rustc_macros/src/query.rs

@@ -150,7 +150,6 @@ struct QueryModifiers {
     eval_always: Option<Ident>,
     feedable: Option<Ident>,
     no_hash: Option<Ident>,
-    return_result_from_ensure_ok: Option<Ident>,
     separate_provide_extern: Option<Ident>,
     // tidy-alphabetical-end
 }
@@ -167,7 +166,6 @@ fn parse_query_modifiers(input: ParseStream<'_>) -> Result<QueryModifiers> {
     let mut depth_limit = None;
     let mut separate_provide_extern = None;
     let mut feedable = None;
-    let mut return_result_from_ensure_ok = None;
 
     while !input.is_empty() {
         let modifier: Ident = input.parse()?;
@@ -211,8 +209,6 @@ macro_rules! try_insert {
             try_insert!(separate_provide_extern = modifier);
         } else if modifier == "feedable" {
             try_insert!(feedable = modifier);
-        } else if modifier == "return_result_from_ensure_ok" {
-            try_insert!(return_result_from_ensure_ok = modifier);
         } else {
             return Err(Error::new(modifier.span(), "unknown query modifier"));
         }
@@ -232,11 +228,29 @@ macro_rules! try_insert {
         depth_limit,
         separate_provide_extern,
         feedable,
-        return_result_from_ensure_ok,
     })
 }
 
-fn make_modifiers_stream(query: &Query, modifiers: &QueryModifiers) -> proc_macro2::TokenStream {
+// Does `ret_ty` match `Result<_, ErrorGuaranteed>`?
+fn returns_error_guaranteed(ret_ty: &ReturnType) -> bool {
+    use ::syn::*;
+    if let ReturnType::Type(_, ret_ty) = ret_ty
+        && let Type::Path(type_path) = ret_ty.as_ref()
+        && let Some(PathSegment { ident, arguments }) = type_path.path.segments.last()
+        && ident == "Result"
+        && let PathArguments::AngleBracketed(args) = arguments
+        && args.args.len() == 2
+        && let GenericArgument::Type(ty) = &args.args[1]
+        && let Type::Path(type_path) = ty
+        && type_path.path.is_ident("ErrorGuaranteed")
+    {
+        true
+    } else {
+        false
+    }
+}
+
+fn make_modifiers_stream(query: &Query) -> proc_macro2::TokenStream {
     let QueryModifiers {
         // tidy-alphabetical-start
         anon,
@@ -249,10 +263,9 @@ fn make_modifiers_stream(query: &Query, modifiers: &QueryModifiers) -> proc_macr
         eval_always,
         feedable,
         no_hash,
-        return_result_from_ensure_ok,
         separate_provide_extern,
         // tidy-alphabetical-end
-    } = modifiers;
+    } = &query.modifiers;
 
     let anon = anon.is_some();
     let arena_cache = arena_cache.is_some();
@@ -270,7 +283,7 @@ fn make_modifiers_stream(query: &Query, modifiers: &QueryModifiers) -> proc_macr
     let eval_always = eval_always.is_some();
     let feedable = feedable.is_some();
     let no_hash = no_hash.is_some();
-    let return_result_from_ensure_ok = return_result_from_ensure_ok.is_some();
+    let returns_error_guaranteed = returns_error_guaranteed(&query.return_ty);
     let separate_provide_extern = separate_provide_extern.is_some();
 
     // Giving an input span to the modifier names in the modifier list seems
@@ -289,7 +302,7 @@ fn make_modifiers_stream(query: &Query, modifiers: &QueryModifiers) -> proc_macr
         eval_always: #eval_always,
         feedable: #feedable,
         no_hash: #no_hash,
-        return_result_from_ensure_ok: #return_result_from_ensure_ok,
+        returns_error_guaranteed: #returns_error_guaranteed,
         separate_provide_extern: #separate_provide_extern,
         // tidy-alphabetical-end
     }
@@ -406,7 +419,6 @@ macro_rules! doc_link {
         depth_limit,
         separate_provide_extern,
         feedable,
-        return_result_from_ensure_ok,
     );
 
     let name = &query.name;
@@ -479,7 +491,7 @@ macro_rules! assert {
             ReturnType::Type(..) => quote! { #return_ty },
         };
 
-        let modifiers_stream = make_modifiers_stream(&query, modifiers);
+        let modifiers_stream = make_modifiers_stream(&query);
 
         // Add the query to the group
         query_stream.extend(quote! {

compiler/rustc_middle/src/queries.rs

@@ -40,9 +40,6 @@
 //! - `depth_limit`: Impose a recursion depth limit on the query to prevent stack overflows.
 //! - `separate_provide_extern`: Use separate provider functions for local and external crates.
 //! - `feedable`: Allow the query result to be set from another query ("fed" externally).
-//! - `return_result_from_ensure_ok`: When called via `tcx.ensure_ok()`, return `Result<(), ErrorGuaranteed>` instead of `()`.
-//!   If the query needs to be executed and returns an error, the error is returned to the caller.
-//!   Only valid for queries returning `Result<_, ErrorGuaranteed>`.
 //!
 //! For the up-to-date list, see the `QueryModifiers` struct in
 //! [`rustc_macros/src/query.rs`](https://github.com/rust-lang/rust/blob/HEAD/compiler/rustc_macros/src/query.rs)
@@ -717,7 +714,6 @@
 
     query check_coroutine_obligations(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
         desc { "verify auto trait bounds for coroutine interior type `{}`", tcx.def_path_str(key) }
-        return_result_from_ensure_ok
     }
 
     /// Used in case `mir_borrowck` fails to prove an obligation. We generally assume that
@@ -1165,9 +1161,8 @@
     }
 
     /// Checks well-formedness of tail calls (`become f()`).
-    query check_tail_calls(key: LocalDefId) -> Result<(), rustc_errors::ErrorGuaranteed> {
+    query check_tail_calls(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
         desc { "tail-call-checking `{}`", tcx.def_path_str(key) }
-        return_result_from_ensure_ok
     }
 
     /// Returns the types assumed to be well formed while "inside" of the given item.
@@ -1239,7 +1234,6 @@
 
     query check_type_wf(key: ()) -> Result<(), ErrorGuaranteed> {
         desc { "checking that types are well-formed" }
-        return_result_from_ensure_ok
     }
 
     /// Caches `CoerceUnsized` kinds for impls on custom types.
@@ -1247,7 +1241,6 @@
         desc { "computing CoerceUnsized info for `{}`", tcx.def_path_str(key) }
         cache_on_disk_if { key.is_local() }
         separate_provide_extern
-        return_result_from_ensure_ok
     }
 
     query typeck(key: LocalDefId) -> &'tcx ty::TypeckResults<'tcx> {
@@ -1262,7 +1255,6 @@
 
     query coherent_trait(def_id: DefId) -> Result<(), ErrorGuaranteed> {
         desc { "coherence checking all impls of trait `{}`", tcx.def_path_str(def_id) }
-        return_result_from_ensure_ok
     }
 
     /// Borrow-checks the given typeck root, e.g. functions, const/static items,
@@ -1294,7 +1286,6 @@
     /// </div>
     query crate_inherent_impls_validity_check(_: ()) -> Result<(), ErrorGuaranteed> {
         desc { "check for inherent impls that should not be defined in crate" }
-        return_result_from_ensure_ok
     }
 
     /// Checks all types in the crate for overlap in their inherent impls. Reports errors.
@@ -1306,7 +1297,6 @@
     /// </div>
     query crate_inherent_impls_overlap_check(_: ()) -> Result<(), ErrorGuaranteed> {
         desc { "check for overlap between inherent impls defined in this crate" }
-        return_result_from_ensure_ok
     }
 
     /// Checks whether all impls in the crate pass the overlap check, returning
@@ -1316,7 +1306,6 @@
             "checking whether impl `{}` follows the orphan rules",
             tcx.def_path_str(key),
         }
-        return_result_from_ensure_ok
     }
 
     /// Return the set of (transitive) callees that may result in a recursive call to `key`,
@@ -1421,9 +1410,8 @@
         desc { "converting literal to const" }
     }
 
-    query check_match(key: LocalDefId) -> Result<(), rustc_errors::ErrorGuaranteed> {
+    query check_match(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
         desc { "match-checking `{}`", tcx.def_path_str(key) }
-        return_result_from_ensure_ok
     }
 
     /// Performs part of the privacy check and computes effective visibilities.
@@ -1652,7 +1640,6 @@
     query specialization_graph_of(trait_id: DefId) -> Result<&'tcx specialization_graph::Graph, ErrorGuaranteed> {
         desc { "building specialization graph of trait `{}`", tcx.def_path_str(trait_id) }
         cache_on_disk_if { true }
-        return_result_from_ensure_ok
     }
     query dyn_compatibility_violations(trait_id: DefId) -> &'tcx [DynCompatibilityViolation] {
         desc { "determining dyn-compatibility of trait `{}`", tcx.def_path_str(trait_id) }
@@ -1931,12 +1918,10 @@
 
     query check_well_formed(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
         desc { "checking that `{}` is well-formed", tcx.def_path_str(key) }
-        return_result_from_ensure_ok
     }
 
     query enforce_impl_non_lifetime_params_are_constrained(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
         desc { "checking that `{}`'s generics are constrained by the impl header", tcx.def_path_str(key) }
-        return_result_from_ensure_ok
     }
 
     // The `DefId`s of all non-generic functions and statics in the given crate
@@ -2698,7 +2683,6 @@
     /// Any other def id will ICE.
     query compare_impl_item(key: LocalDefId) -> Result<(), ErrorGuaranteed> {
         desc { "checking assoc item `{}` is compatible with trait definition", tcx.def_path_str(key) }
-        return_result_from_ensure_ok
     }
 
     query deduced_param_attrs(def_id: DefId) -> &'tcx [DeducedParamAttrs] {

compiler/rustc_middle/src/query/inner.rs

@@ -1,7 +1,6 @@
 //! Helper functions that serve as the immediate implementation of
 //! `tcx.$query(..)` and its variations.
 
-use rustc_data_structures::assert_matches;
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
 
 use crate::dep_graph;
@@ -48,10 +47,10 @@ pub(crate) fn query_get_at<'tcx, C>(
     }
 }
 
-/// Shared implementation of `tcx.ensure_ok().$query(..)` for most queries,
-/// and `tcx.ensure_done().$query(..)` for all queries.
+/// Shared implementation of `tcx.ensure_ok().$query(..)` and
+/// `tcx.ensure_done().$query(..)` for all queries.
 #[inline]
-pub(crate) fn query_ensure<'tcx, C>(
+pub(crate) fn query_ensure_ok_or_done<'tcx, C>(
     tcx: TyCtxt<'tcx>,
     query: &'tcx QueryVTable<'tcx, C>,
     key: C::Key,
@@ -59,41 +58,43 @@ pub(crate) fn query_ensure<'tcx, C>(
 ) where
     C: QueryCache,
 {
-    if try_get_cached(tcx, &query.cache, &key).is_none() {
-        (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode });
+    match try_get_cached(tcx, &query.cache, &key) {
+        Some(_value) => {}
+        None => {
+            (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode });
+        }
     }
 }
 
-/// Shared implementation of `tcx.ensure_ok().$query(..)` for queries that
-/// have the `return_result_from_ensure_ok` modifier.
+/// Implementation of `tcx.ensure_result().$query(..)` for queries that
+/// return `Result<_, ErrorGuaranteed>`.
 #[inline]
-pub(crate) fn query_ensure_error_guaranteed<'tcx, C, T>(
+pub(crate) fn query_ensure_result<'tcx, C, T>(
     tcx: TyCtxt<'tcx>,
     query: &'tcx QueryVTable<'tcx, C>,
     key: C::Key,
-    // This arg is needed to match the signature of `query_ensure`,
-    // but should always be `EnsureMode::Ok`.
-    ensure_mode: EnsureMode,
 ) -> Result<(), ErrorGuaranteed>
 where
     C: QueryCache<Value = Erased<Result<T, ErrorGuaranteed>>>,
     Result<T, ErrorGuaranteed>: Erasable,
 {
-    assert_matches!(ensure_mode, EnsureMode::Ok);
-
-    if let Some(res) = try_get_cached(tcx, &query.cache, &key) {
-        erase::restore_val(res).map(drop)
-    } else {
-        (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode })
-            .map(erase::restore_val)
-            .map(|res| res.map(drop))
-            // Either we actually executed the query, which means we got a full `Result`,
-            // or we can just assume the query succeeded, because it was green in the
-            // incremental cache. If it is green, that means that the previous compilation
-            // that wrote to the incremental cache compiles successfully. That is only
-            // possible if the cache entry was `Ok(())`, so we emit that here, without
-            // actually encoding the `Result` in the cache or loading it from there.
-            .unwrap_or(Ok(()))
+    match try_get_cached(tcx, &query.cache, &key) {
+        Some(value) => erase::restore_val(value).map(drop),
+        None => (query.execute_query_fn)(
+            tcx,
+            DUMMY_SP,
+            key,
+            QueryMode::Ensure { ensure_mode: EnsureMode::Ok },
+        )
+        .map(erase::restore_val)
+        .map(|value| value.map(drop))
+        // Either we actually executed the query, which means we got a full `Result`,
+        // or we can just assume the query succeeded, because it was green in the
+        // incremental cache. If it is green, that means that the previous compilation
+        // that wrote to the incremental cache compiles successfully. That is only
+        // possible if the cache entry was `Ok(())`, so we emit that here, without
+        // actually encoding the `Result` in the cache or loading it from there.
+        .unwrap_or(Ok(())),
     }
 }
 

compiler/rustc_middle/src/query/mod.rs

@@ -5,7 +5,7 @@
 pub use self::keys::{AsLocalQueryKey, LocalCrate, QueryKey};
 pub use self::plumbing::{
     ActiveKeyStatus, CycleError, CycleErrorHandling, EnsureMode, IntoQueryParam, QueryMode,
-    QueryState, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk,
+    QueryState, TyCtxtAt, TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult,
 };
 pub use self::stack::{QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra};
 pub use crate::queries::Providers;

compiler/rustc_middle/src/query/modifiers.rs

@@ -58,20 +58,6 @@
 /// Don't hash the result, instead just mark a query red if it runs
 pub(crate) struct no_hash;
 
-/// # `return_result_from_ensure_ok` query modifier
-///
-/// When this query is called via `tcx.ensure_ok()`, it returns
-/// `Result<(), ErrorGuaranteed>` instead of `()`. If the query needs to
-/// be executed, and that execution returns an error, the error result is
-/// returned to the caller.
-///
-/// If execution is skipped, a synthetic `Ok(())` is returned, on the
-/// assumption that a query with all-green inputs must have succeeded.
-///
-/// Can only be applied to queries with a return value of
-/// `Result<_, ErrorGuaranteed>`.
-pub(crate) struct return_result_from_ensure_ok;
-
 /// # `separate_provide_extern` query modifier
 ///
 /// Use a separate query provider for local and extern crates

compiler/rustc_middle/src/query/plumbing.rs

@@ -208,6 +208,12 @@ pub struct TyCtxtEnsureOk<'tcx> {
     pub tcx: TyCtxt<'tcx>,
 }
 
+#[derive(Copy, Clone)]
+#[must_use]
+pub struct TyCtxtEnsureResult<'tcx> {
+    pub tcx: TyCtxt<'tcx>,
+}
+
 #[derive(Copy, Clone)]
 #[must_use]
 pub struct TyCtxtEnsureDone<'tcx> {
@@ -215,6 +221,8 @@ pub struct TyCtxtEnsureDone<'tcx> {
 }
 
 impl<'tcx> TyCtxt<'tcx> {
+    /// FIXME: `ensure_ok`'s effects are subtle. Is this comment fully accurate?
+    ///
     /// Wrapper that calls queries in a special "ensure OK" mode, for callers
     /// that don't need the return value and just want to invoke a query for
     /// its potential side-effect of emitting fatal errors.
@@ -235,17 +243,21 @@ impl<'tcx> TyCtxt<'tcx> {
     ///
     /// Therefore, this call mode is not appropriate for callers that want to
     /// ensure that the query is _never_ executed in the future.
-    ///
-    /// ## `return_result_from_ensure_ok`
-    /// If a query has the `return_result_from_ensure_ok` modifier, calls via
-    /// `ensure_ok` will instead return `Result<(), ErrorGuaranteed>`. If the
-    /// query needs to be executed, and execution returns an error, that error
-    /// is returned to the caller.
     #[inline(always)]
     pub fn ensure_ok(self) -> TyCtxtEnsureOk<'tcx> {
         TyCtxtEnsureOk { tcx: self }
     }
 
+    /// This is a variant of `ensure_ok` only usable with queries that return
+    /// `Result<_, ErrorGuaranteed>`. Queries calls through this function will
+    /// return `Result<(), ErrorGuaranteed>`. I.e. the error status is returned
+    /// but nothing else. As with `ensure_ok`, this can be more efficient than
+    /// a normal query call.
+    #[inline(always)]
+    pub fn ensure_result(self) -> TyCtxtEnsureResult<'tcx> {
+        TyCtxtEnsureResult { tcx: self }
+    }
+
     /// Wrapper that calls queries in a special "ensure done" mode, for callers
     /// that don't need the return value and just want to guarantee that the
     /// query won't be executed in the future, by executing it now if necessary.
@@ -299,7 +311,7 @@ fn $name:ident($($K:tt)*) -> $V:ty
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
                     no_hash: $no_hash:literal,
-                    return_result_from_ensure_ok: $return_result_from_ensure_ok:literal,
+                    returns_error_guaranteed: $returns_error_guaranteed:literal,
                     separate_provide_extern: $separate_provide_extern:literal,
                 }
             )*
@@ -325,15 +337,6 @@ pub mod $name {
                 #[cfg(not($separate_provide_extern))]
                 pub type LocalKey<'tcx> = Key<'tcx>;
 
-                /// Return type of the `.ensure_ok()` method for this query,
-                /// which has the `return_result_from_ensure_ok` modifier.
-                #[cfg($return_result_from_ensure_ok)]
-                pub type EnsureOkReturnType = Result<(), rustc_errors::ErrorGuaranteed>;
-                /// Return type of the `.ensure_ok()` method for this query,
-                /// which does _not_ have the `return_result_from_ensure_ok` modifier.
-                #[cfg(not($return_result_from_ensure_ok))]
-                pub type EnsureOkReturnType = ();
-
                 /// Type returned from query providers and loaded from disk-cache.
                 #[cfg($arena_cache)]
                 pub type ProvidedValue<'tcx> =
@@ -425,17 +428,8 @@ impl<'tcx> $crate::query::TyCtxtEnsureOk<'tcx> {
             $(
                 $(#[$attr])*
                 #[inline(always)]
-                pub fn $name(
-                    self,
-                    key: query_helper_param_ty!($($K)*),
-                ) -> $crate::queries::$name::EnsureOkReturnType {
-
-                    #[cfg($return_result_from_ensure_ok)]
-                    let ensure_fn = crate::query::inner::query_ensure_error_guaranteed;
-                    #[cfg(not($return_result_from_ensure_ok))]
-                    let ensure_fn = crate::query::inner::query_ensure;
-
-                    ensure_fn(
+                pub fn $name(self, key: query_helper_param_ty!($($K)*)) {
+                    crate::query::inner::query_ensure_ok_or_done(
                         self.tcx,
                         &self.tcx.query_system.query_vtables.$name,
                         $crate::query::IntoQueryParam::into_query_param(key),
@@ -445,12 +439,31 @@ pub fn $name(
             )*
         }
 
+        // Only defined when the `ensure_result` modifier is present.
+        impl<'tcx> $crate::query::TyCtxtEnsureResult<'tcx> {
+            $(
+                #[cfg($returns_error_guaranteed)]
+                $(#[$attr])*
+                #[inline(always)]
+                pub fn $name(
+                    self,
+                    key: query_helper_param_ty!($($K)*),
+                ) -> Result<(), rustc_errors::ErrorGuaranteed> {
+                    crate::query::inner::query_ensure_result(
+                        self.tcx,
+                        &self.tcx.query_system.query_vtables.$name,
+                        $crate::query::IntoQueryParam::into_query_param(key),
+                    )
+                }
+            )*
+        }
+
         impl<'tcx> $crate::query::TyCtxtEnsureDone<'tcx> {
             $(
                 $(#[$attr])*
                 #[inline(always)]
                 pub fn $name(self, key: query_helper_param_ty!($($K)*)) {
-                    crate::query::inner::query_ensure(
+                    crate::query::inner::query_ensure_ok_or_done(
                         self.tcx,
                         &self.tcx.query_system.query_vtables.$name,
                         $crate::query::IntoQueryParam::into_query_param(key),

compiler/rustc_middle/src/ty/util.rs

@@ -402,7 +402,7 @@ pub fn calculate_dtor(
         validate: impl Fn(Self, LocalDefId) -> Result<(), ErrorGuaranteed>,
     ) -> Option<ty::Destructor> {
         let drop_trait = self.lang_items().drop_trait()?;
-        self.ensure_ok().coherent_trait(drop_trait).ok()?;
+        self.ensure_result().coherent_trait(drop_trait).ok()?;
 
         let mut dtor_candidate = None;
         // `Drop` impls can only be written in the same crate as the adt, and cannot be blanket impls
@@ -449,7 +449,7 @@ pub fn calculate_async_dtor(
         validate: impl Fn(Self, LocalDefId) -> Result<(), ErrorGuaranteed>,
     ) -> Option<ty::AsyncDestructor> {
         let async_drop_trait = self.lang_items().async_drop_trait()?;
-        self.ensure_ok().coherent_trait(async_drop_trait).ok()?;
+        self.ensure_result().coherent_trait(async_drop_trait).ok()?;
 
         let mut dtor_candidate = None;
         // `AsyncDrop` impls can only be written in the same crate as the adt, and cannot be blanket impls

compiler/rustc_mir_build/src/builder/mod.rs

@@ -68,11 +68,11 @@ pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
 /// be called by the query `mir_built`.
 pub(crate) fn build_mir_inner_impl<'tcx>(tcx: TyCtxt<'tcx>, def: LocalDefId) -> Body<'tcx> {
     tcx.ensure_done().thir_abstract_const(def);
-    if let Err(e) = tcx.ensure_ok().check_match(def) {
+    if let Err(e) = tcx.ensure_result().check_match(def) {
         return construct_error(tcx, def, e);
     }
 
-    if let Err(err) = tcx.ensure_ok().check_tail_calls(def) {
+    if let Err(err) = tcx.ensure_result().check_tail_calls(def) {
         return construct_error(tcx, def, err);
     }
 

compiler/rustc_mir_transform/src/lib.rs

@@ -562,7 +562,7 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &
         | DefKind::Static { .. }
         | DefKind::Const { .. }
         | DefKind::AssocConst { .. } => {
-            if let Err(guar) = tcx.ensure_ok().check_well_formed(root.expect_local()) {
+            if let Err(guar) = tcx.ensure_result().check_well_formed(root.expect_local()) {
                 body.tainted_by_errors = Some(guar);
             }
         }

compiler/rustc_query_impl/src/dep_kind_vtables.rs

@@ -141,7 +141,7 @@ fn $name:ident($K:ty) -> $V:ty
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
                     no_hash: $no_hash:literal,
-                    return_result_from_ensure_ok: $return_result_from_ensure_ok:literal,
+                    returns_error_guaranteed: $returns_error_guaranteed:literal,
                     separate_provide_extern: $separate_provide_extern:literal,
                 }
             )*

compiler/rustc_query_impl/src/plumbing.rs

@@ -325,7 +325,7 @@ fn $name:ident($K:ty) -> $V:ty
                     eval_always: $eval_always:literal,
                     feedable: $feedable:literal,
                     no_hash: $no_hash:literal,
-                    return_result_from_ensure_ok: $return_result_from_ensure_ok:literal,
+                    returns_error_guaranteed: $returns_error_guaranteed:literal,
                     separate_provide_extern: $separate_provide_extern:literal,
                 }
             )*

compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs

@@ -185,9 +185,9 @@ pub(super) fn maybe_report_ambiguity(
                     return e;
                 }
 
-                if let Err(guar) = self.tcx.ensure_ok().coherent_trait(trait_pred.def_id()) {
-                    // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case
-                    // other `Foo` impls are incoherent.
+                if let Err(guar) = self.tcx.ensure_result().coherent_trait(trait_pred.def_id()) {
+                    // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for
+                    // Foo` in case other `Foo` impls are incoherent.
                     return guar;
                 }
 
@@ -522,7 +522,7 @@ pub(super) fn maybe_report_ambiguity(
 
                 if let Err(guar) = self
                     .tcx
-                    .ensure_ok()
+                    .ensure_result()
                     .coherent_trait(self.tcx.parent(data.projection_term.def_id))
                 {
                     // Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case

compiler/rustc_trait_selection/src/traits/specialize/mod.rs

@@ -563,7 +563,7 @@ fn decorate<'tcx, G: EmissionGuarantee>(
     match used_to_be_allowed {
         None => {
             let reported = if overlap.with_impl.is_local()
-                || tcx.ensure_ok().orphan_check_impl(impl_def_id).is_ok()
+                || tcx.ensure_result().orphan_check_impl(impl_def_id).is_ok()
             {
                 let mut err = tcx.dcx().struct_span_err(impl_span, msg());
                 err.code(E0119);

compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs

@@ -367,7 +367,7 @@ pub(crate) fn assoc_def(
         // Ensure that the impl is constrained, otherwise projection may give us
         // bad unconstrained infer vars.
         if let Some(impl_def_id) = impl_def_id.as_local() {
-            tcx.ensure_ok().enforce_impl_non_lifetime_params_are_constrained(impl_def_id)?;
+            tcx.ensure_result().enforce_impl_non_lifetime_params_are_constrained(impl_def_id)?;
         }
 
         let item = tcx.associated_item(impl_item_id);
@@ -390,7 +390,7 @@ pub(crate) fn assoc_def(
         if let ty::AssocContainer::TraitImpl(_) = assoc_item.item.container
             && let Some(impl_def_id) = assoc_item.item.container_id(tcx).as_local()
         {
-            tcx.ensure_ok().enforce_impl_non_lifetime_params_are_constrained(impl_def_id)?;
+            tcx.ensure_result().enforce_impl_non_lifetime_params_are_constrained(impl_def_id)?;
         }
 
         Ok(assoc_item)

compiler/rustc_ty_utils/src/instance.rs

@@ -230,7 +230,7 @@ fn resolve_associated_item<'tcx>(
             if trait_item_id != leaf_def.item.def_id
                 && let Some(leaf_def_item) = leaf_def.item.def_id.as_local()
             {
-                tcx.ensure_ok().compare_impl_item(leaf_def_item)?;
+                tcx.ensure_result().compare_impl_item(leaf_def_item)?;
             }
 
             Some(ty::Instance::new_raw(leaf_def.item.def_id, args))

src/librustdoc/core.rs

@@ -351,7 +351,7 @@ pub(crate) fn run_global_ctxt(
     // (see `override_queries` in the `config`)
 
     // NOTE: These are copy/pasted from typeck/lib.rs and should be kept in sync with those changes.
-    let _ = tcx.sess.time("wf_checking", || tcx.ensure_ok().check_type_wf(()));
+    tcx.sess.time("wf_checking", || tcx.ensure_ok().check_type_wf(()));
 
     tcx.dcx().abort_if_errors();