Auto merge of #95487 - cjgillot:menhir, r=oli-obk

Avoid accessing HIR from MIR passes

`hir_owner_nodes` contains a lot of information, and the query result is typically dirty. This forces dependent queries to be re-executed needlessly.

This PR refactors some accesses to HIR to go through more targeted queries that yield the same result.

Based on #95435 and #95436

Author: bors

compiler/rustc_borrowck/src/lib.rs

@@ -23,7 +23,6 @@ use rustc_data_structures::graph::dominators::Dominators;
 use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
 use rustc_hir as hir;
 use rustc_hir::def_id::LocalDefId;
-use rustc_hir::Node;
 use rustc_index::bit_set::ChunkedBitSet;
 use rustc_index::vec::IndexVec;
 use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
@@ -288,14 +287,16 @@ fn do_mir_borrowck<'a, 'tcx>(
         .pass_name("borrowck")
         .iterate_to_fixpoint();
 
-    let def_hir_id = tcx.hir().local_def_id_to_hir_id(def.did);
-    let movable_generator = !matches!(
-        tcx.hir().get(def_hir_id),
-        Node::Expr(&hir::Expr {
-            kind: hir::ExprKind::Closure(.., Some(hir::Movability::Static)),
-            ..
-        })
-    );
+    let movable_generator =
+        // The first argument is the generator type passed by value
+        if let Some(local) = body.local_decls.raw.get(1)
+        // Get the interior types and substs which typeck computed
+        && let ty::Generator(_, _, hir::Movability::Static) = local.ty.kind()
+    {
+        false
+    } else {
+        true
+    };
 
     for (idx, move_data_results) in promoted_errors {
         let promoted_body = &promoted[idx];
@@ -385,7 +386,7 @@ fn do_mir_borrowck<'a, 'tcx>(
         let scope = mbcx.body.source_info(location).scope;
         let lint_root = match &mbcx.body.source_scopes[scope].local_data {
             ClearCrossCrate::Set(data) => data.lint_root,
-            _ => def_hir_id,
+            _ => tcx.hir().local_def_id_to_hir_id(def.did),
         };
 
         // Span and message don't matter; we overwrite them below anyway

compiler/rustc_borrowck/src/universal_regions.rs

@@ -828,10 +828,8 @@ fn for_each_late_bound_region_defined_on<'tcx>(
     mut f: impl FnMut(ty::Region<'tcx>),
 ) {
     if let Some((owner, late_bounds)) = tcx.is_late_bound_map(fn_def_id.expect_local()) {
-        for &late_bound in late_bounds.iter() {
-            let hir_id = HirId { owner, local_id: late_bound };
-            let name = tcx.hir().name(hir_id);
-            let region_def_id = tcx.hir().local_def_id(hir_id);
+        for &region_def_id in late_bounds.iter() {
+            let name = tcx.item_name(region_def_id.to_def_id());
             let liberated_region = tcx.mk_region(ty::ReFree(ty::FreeRegion {
                 scope: owner.to_def_id(),
                 bound_region: ty::BoundRegionKind::BrNamed(region_def_id.to_def_id(), name),

compiler/rustc_const_eval/src/transform/check_consts/check.rs

@@ -222,7 +222,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
 
         // `async` functions cannot be `const fn`. This is checked during AST lowering, so there's
         // no need to emit duplicate errors here.
-        if is_async_fn(self.ccx) || body.generator.is_some() {
+        if self.ccx.is_async() || body.generator.is_some() {
             tcx.sess.delay_span_bug(body.span, "`async` functions cannot be `const fn`");
             return;
         }
@@ -1056,12 +1056,8 @@ fn is_int_bool_or_char(ty: Ty<'_>) -> bool {
     ty.is_bool() || ty.is_integral() || ty.is_char()
 }
 
-fn is_async_fn(ccx: &ConstCx<'_, '_>) -> bool {
-    ccx.fn_sig().map_or(false, |sig| sig.header.asyncness == hir::IsAsync::Async)
-}
-
 fn emit_unstable_in_stable_error(ccx: &ConstCx<'_, '_>, span: Span, gate: Symbol) {
-    let attr_span = ccx.fn_sig().map_or(ccx.body.span, |sig| sig.span.shrink_to_lo());
+    let attr_span = ccx.tcx.def_span(ccx.def_id()).shrink_to_lo();
 
     ccx.tcx
         .sess

compiler/rustc_const_eval/src/transform/check_consts/mod.rs

@@ -61,14 +61,8 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> {
             && is_const_stable_const_fn(self.tcx, self.def_id().to_def_id())
     }
 
-    /// Returns the function signature of the item being const-checked if it is a `fn` or `const fn`.
-    pub fn fn_sig(&self) -> Option<&'tcx hir::FnSig<'tcx>> {
-        // Get this from the HIR map instead of a query to avoid cycle errors.
-        //
-        // FIXME: Is this still an issue?
-        let hir_map = self.tcx.hir();
-        let hir_id = hir_map.local_def_id_to_hir_id(self.def_id());
-        hir_map.fn_sig_by_hir_id(hir_id)
+    fn is_async(&self) -> bool {
+        self.tcx.asyncness(self.def_id()) == hir::IsAsync::Async
     }
 }
 

compiler/rustc_hir/src/def.rs

@@ -223,6 +223,14 @@ impl DefKind {
             | DefKind::Impl => None,
         }
     }
+
+    #[inline]
+    pub fn is_fn_like(self) -> bool {
+        match self {
+            DefKind::Fn | DefKind::AssocFn | DefKind::Closure | DefKind::Generator => true,
+            _ => false,
+        }
+    }
 }
 
 /// The resolution of a path or export.

compiler/rustc_middle/src/middle/resolve_lifetime.rs

@@ -64,7 +64,7 @@ pub struct ResolveLifetimes {
     /// Set of lifetime def ids that are late-bound; a region can
     /// be late-bound if (a) it does NOT appear in a where-clause and
     /// (b) it DOES appear in the arguments.
-    pub late_bound: FxHashMap<LocalDefId, FxHashSet<ItemLocalId>>,
+    pub late_bound: FxHashMap<LocalDefId, FxHashSet<LocalDefId>>,
 
     pub late_bound_vars: FxHashMap<LocalDefId, FxHashMap<ItemLocalId, Vec<ty::BoundVariableKind>>>,
 }

compiler/rustc_middle/src/query/mod.rs

@@ -1502,8 +1502,7 @@ rustc_queries! {
         Option<&'tcx FxHashMap<ItemLocalId, Region>> {
         desc { "looking up a named region" }
     }
-    query is_late_bound_map(_: LocalDefId) ->
-        Option<(LocalDefId, &'tcx FxHashSet<ItemLocalId>)> {
+    query is_late_bound_map(_: LocalDefId) -> Option<(LocalDefId, &'tcx FxHashSet<LocalDefId>)> {
         desc { "testing if a region is late bound" }
     }
     /// For a given item (like a struct), gets the default lifetimes to be used

compiler/rustc_middle/src/ty/context.rs

@@ -2772,11 +2772,6 @@ impl<'tcx> TyCtxt<'tcx> {
         self.named_region_map(id.owner).and_then(|map| map.get(&id.local_id).cloned())
     }
 
-    pub fn is_late_bound(self, id: HirId) -> bool {
-        self.is_late_bound_map(id.owner)
-            .map_or(false, |(owner, set)| owner == id.owner && set.contains(&id.local_id))
-    }
-
     pub fn late_bound_vars(self, id: HirId) -> &'tcx List<ty::BoundVariableKind> {
         self.mk_bound_variable_kinds(
             self.late_bound_vars_map(id.owner)

compiler/rustc_mir_transform/src/const_prop.rs

@@ -71,8 +71,9 @@ impl<'tcx> MirPass<'tcx> for ConstProp {
         }
 
         let def_id = body.source.def_id().expect_local();
-        let is_fn_like = tcx.hir().get_by_def_id(def_id).fn_kind().is_some();
-        let is_assoc_const = tcx.def_kind(def_id) == DefKind::AssocConst;
+        let def_kind = tcx.def_kind(def_id);
+        let is_fn_like = def_kind.is_fn_like();
+        let is_assoc_const = def_kind == DefKind::AssocConst;
 
         // Only run const prop on functions, methods, closures and associated constants
         if !is_fn_like && !is_assoc_const {

compiler/rustc_mir_transform/src/const_prop_lint.rs

@@ -67,7 +67,7 @@ impl<'tcx> MirLint<'tcx> for ConstProp {
         }
 
         let def_id = body.source.def_id().expect_local();
-        let is_fn_like = tcx.hir().get_by_def_id(def_id).fn_kind().is_some();
+        let is_fn_like = tcx.def_kind(def_id).is_fn_like();
         let is_assoc_const = tcx.def_kind(def_id) == DefKind::AssocConst;
 
         // Only run const prop on functions, methods, closures and associated constants

compiler/rustc_mir_transform/src/lib.rs

@@ -366,7 +366,7 @@ fn mir_drops_elaborated_and_const_checked<'tcx>(
 
     let mir_borrowck = tcx.mir_borrowck_opt_const_arg(def);
 
-    let is_fn_like = tcx.hir().get_by_def_id(def.did).fn_kind().is_some();
+    let is_fn_like = tcx.def_kind(def.did).is_fn_like();
     if is_fn_like {
         let did = def.did.to_def_id();
         let def = ty::WithOptConstParam::unknown(did);

compiler/rustc_mir_transform/src/shim.rs

@@ -725,9 +725,6 @@ fn build_call_shim<'tcx>(
 pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> Body<'_> {
     debug_assert!(tcx.is_constructor(ctor_id));
 
-    let span =
-        tcx.hir().span_if_local(ctor_id).unwrap_or_else(|| bug!("no span for ctor {:?}", ctor_id));
-
     let param_env = tcx.param_env(ctor_id);
 
     // Normalize the sig.
@@ -740,6 +737,8 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> Body<'_> {
 
     debug!("build_ctor: ctor_id={:?} sig={:?}", ctor_id, sig);
 
+    let span = tcx.def_span(ctor_id);
+
     let local_decls = local_decls_for_sig(&sig, span);
 
     let source_info = SourceInfo::outermost(span);

compiler/rustc_resolve/src/late/lifetimes.rs

@@ -427,15 +427,15 @@ fn resolve_lifetimes_trait_definition(
     tcx: TyCtxt<'_>,
     local_def_id: LocalDefId,
 ) -> ResolveLifetimes {
-    convert_named_region_map(do_resolve(tcx, local_def_id, true, false))
+    convert_named_region_map(tcx, do_resolve(tcx, local_def_id, true, false))
 }
 
 /// Computes the `ResolveLifetimes` map that contains data for an entire `Item`.
 /// You should not read the result of this query directly, but rather use
 /// `named_region_map`, `is_late_bound_map`, etc.
 #[tracing::instrument(level = "debug", skip(tcx))]
 fn resolve_lifetimes(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> ResolveLifetimes {
-    convert_named_region_map(do_resolve(tcx, local_def_id, false, false))
+    convert_named_region_map(tcx, do_resolve(tcx, local_def_id, false, false))
 }
 
 fn do_resolve(
@@ -468,7 +468,7 @@ fn do_resolve(
     named_region_map
 }
 
-fn convert_named_region_map(named_region_map: NamedRegionMap) -> ResolveLifetimes {
+fn convert_named_region_map(tcx: TyCtxt<'_>, named_region_map: NamedRegionMap) -> ResolveLifetimes {
     let mut rl = ResolveLifetimes::default();
 
     for (hir_id, v) in named_region_map.defs {
@@ -477,7 +477,8 @@ fn convert_named_region_map(named_region_map: NamedRegionMap) -> ResolveLifetime
     }
     for hir_id in named_region_map.late_bound {
         let map = rl.late_bound.entry(hir_id.owner).or_default();
-        map.insert(hir_id.local_id);
+        let def_id = tcx.hir().local_def_id(hir_id);
+        map.insert(def_id);
     }
     for (hir_id, v) in named_region_map.late_bound_vars {
         let map = rl.late_bound_vars.entry(hir_id.owner).or_default();
@@ -537,7 +538,7 @@ fn item_for(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> LocalDefId {
 fn is_late_bound_map<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
-) -> Option<(LocalDefId, &'tcx FxHashSet<ItemLocalId>)> {
+) -> Option<(LocalDefId, &'tcx FxHashSet<LocalDefId>)> {
     match tcx.def_kind(def_id) {
         DefKind::AnonConst | DefKind::InlineConst => {
             let mut def_id = tcx
@@ -774,8 +775,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
                                 });
                             }
                             for (&owner, late_bound) in resolved_lifetimes.late_bound.iter() {
-                                late_bound.iter().for_each(|&local_id| {
-                                    self.map.late_bound.insert(hir::HirId { owner, local_id });
+                                late_bound.iter().for_each(|&id| {
+                                    let hir_id = self.tcx.local_def_id_to_hir_id(id);
+                                    debug_assert_eq!(owner, hir_id.owner);
+                                    self.map.late_bound.insert(hir_id);
                                 });
                             }
                             for (&owner, late_bound_vars) in

compiler/rustc_symbol_mangling/src/lib.rs

@@ -97,7 +97,6 @@
 extern crate rustc_middle;
 
 use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
-use rustc_hir::Node;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::mir::mono::{InstantiationMode, MonoItem};
 use rustc_middle::ty::query::Providers;
@@ -168,17 +167,14 @@ fn compute_symbol_name<'tcx>(
 
     debug!("symbol_name(def_id={:?}, substs={:?})", def_id, substs);
 
-    // FIXME(eddyb) Precompute a custom symbol name based on attributes.
-    let is_foreign = if let Some(def_id) = def_id.as_local() {
+    if let Some(def_id) = def_id.as_local() {
         if tcx.proc_macro_decls_static(()) == Some(def_id) {
             let stable_crate_id = tcx.sess.local_stable_crate_id();
             return tcx.sess.generate_proc_macro_decls_symbol(stable_crate_id);
         }
-        matches!(tcx.hir().get_by_def_id(def_id), Node::ForeignItem(_))
-    } else {
-        tcx.is_foreign_item(def_id)
-    };
+    }
 
+    // FIXME(eddyb) Precompute a custom symbol name based on attributes.
     let attrs = tcx.codegen_fn_attrs(def_id);
 
     // Foreign items by default use no mangling for their symbol name. There's a
@@ -197,7 +193,7 @@ fn compute_symbol_name<'tcx>(
     //   show up in the `wasm-import-name` custom attribute in LLVM IR.
     //
     // [1]: https://bugs.llvm.org/show_bug.cgi?id=44316
-    if is_foreign
+    if tcx.is_foreign_item(def_id)
         && (!tcx.sess.target.is_like_wasm
             || !tcx.wasm_import_module_map(def_id.krate).contains_key(&def_id))
     {

compiler/rustc_ty_utils/src/ty.rs

@@ -414,12 +414,7 @@ fn issue33140_self_ty(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Ty<'_>> {
 /// Check if a function is async.
 fn asyncness(tcx: TyCtxt<'_>, def_id: DefId) -> hir::IsAsync {
     let node = tcx.hir().get_by_def_id(def_id.expect_local());
-
-    let fn_kind = node.fn_kind().unwrap_or_else(|| {
-        bug!("asyncness: expected fn-like node but got `{:?}`", def_id);
-    });
-
-    fn_kind.asyncness()
+    if let Some(fn_kind) = node.fn_kind() { fn_kind.asyncness() } else { hir::IsAsync::NotAsync }
 }
 
 /// Don't call this directly: use ``tcx.conservative_is_privately_uninhabited`` instead.

compiler/rustc_typeck/src/collect.rs

@@ -1388,6 +1388,7 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
 
     fn has_late_bound_regions<'tcx>(
         tcx: TyCtxt<'tcx>,
+        def_id: LocalDefId,
         generics: &'tcx hir::Generics<'tcx>,
         decl: &'tcx hir::FnDecl<'tcx>,
     ) -> Option<Span> {
@@ -1396,9 +1397,14 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
             outer_index: ty::INNERMOST,
             has_late_bound_regions: None,
         };
+        let late_bound_map = tcx.is_late_bound_map(def_id);
+        let is_late_bound = |id| {
+            let id = tcx.hir().local_def_id(id);
+            late_bound_map.map_or(false, |(_, set)| set.contains(&id))
+        };
         for param in generics.params {
             if let GenericParamKind::Lifetime { .. } = param.kind {
-                if tcx.is_late_bound(param.hir_id) {
+                if is_late_bound(param.hir_id) {
                     return Some(param.span);
                 }
             }
@@ -1410,25 +1416,25 @@ fn has_late_bound_regions<'tcx>(tcx: TyCtxt<'tcx>, node: Node<'tcx>) -> Option<S
     match node {
         Node::TraitItem(item) => match item.kind {
             hir::TraitItemKind::Fn(ref sig, _) => {
-                has_late_bound_regions(tcx, &item.generics, sig.decl)
+                has_late_bound_regions(tcx, item.def_id, &item.generics, sig.decl)
             }
             _ => None,
         },
         Node::ImplItem(item) => match item.kind {
             hir::ImplItemKind::Fn(ref sig, _) => {
-                has_late_bound_regions(tcx, &item.generics, sig.decl)
+                has_late_bound_regions(tcx, item.def_id, &item.generics, sig.decl)
             }
             _ => None,
         },
         Node::ForeignItem(item) => match item.kind {
             hir::ForeignItemKind::Fn(fn_decl, _, ref generics) => {
-                has_late_bound_regions(tcx, generics, fn_decl)
+                has_late_bound_regions(tcx, item.def_id, generics, fn_decl)
             }
             _ => None,
         },
         Node::Item(item) => match item.kind {
             hir::ItemKind::Fn(ref sig, .., ref generics, _) => {
-                has_late_bound_regions(tcx, generics, sig.decl)
+                has_late_bound_regions(tcx, item.def_id, generics, sig.decl)
             }
             _ => None,
         },
@@ -1677,7 +1683,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics {
         params.push(opt_self);
     }
 
-    let early_lifetimes = early_bound_lifetimes_from_generics(tcx, ast_generics);
+    let early_lifetimes = early_bound_lifetimes_from_generics(tcx, hir_id.owner, ast_generics);
     params.extend(early_lifetimes.enumerate().map(|(i, param)| ty::GenericParamDef {
         name: param.name.ident().name,
         index: own_start + i as u32,
@@ -2034,10 +2040,23 @@ fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ImplPolarity {
 /// `resolve_lifetime::early_bound_lifetimes`.
 fn early_bound_lifetimes_from_generics<'a, 'tcx: 'a>(
     tcx: TyCtxt<'tcx>,
+    def_id: LocalDefId,
     generics: &'a hir::Generics<'a>,
 ) -> impl Iterator<Item = &'a hir::GenericParam<'a>> + Captures<'tcx> {
+    let late_bound_map = if generics.params.is_empty() {
+        // This function may be called on `def_id == CRATE_DEF_ID`,
+        // which makes `is_late_bound_map` ICE.  Don't even try if there
+        // is no generic parameter.
+        None
+    } else {
+        tcx.is_late_bound_map(def_id)
+    };
+    let is_late_bound = move |hir_id| {
+        let id = tcx.hir().local_def_id(hir_id);
+        late_bound_map.map_or(false, |(_, set)| set.contains(&id))
+    };
     generics.params.iter().filter(move |param| match param.kind {
-        GenericParamKind::Lifetime { .. } => !tcx.is_late_bound(param.hir_id),
+        GenericParamKind::Lifetime { .. } => !is_late_bound(param.hir_id),
         _ => false,
     })
 }
@@ -2221,7 +2240,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
     // well. In the case of parameters declared on a fn or method, we
     // have to be careful to only iterate over early-bound regions.
     let mut index = parent_count + has_own_self as u32;
-    for param in early_bound_lifetimes_from_generics(tcx, ast_generics) {
+    for param in early_bound_lifetimes_from_generics(tcx, hir_id.owner, ast_generics) {
         let region = tcx.mk_region(ty::ReEarlyBound(ty::EarlyBoundRegion {
             def_id: tcx.hir().local_def_id(param.hir_id).to_def_id(),
             index,