Skip to content

Commit

Permalink
Auto merge of rust-lang#95487 - cjgillot:menhir, r=oli-obk
Browse files Browse the repository at this point in the history
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 rust-lang#95435 and rust-lang#95436
  • Loading branch information
bors committed Apr 10, 2022
2 parents 18f32b7 + bbacfcb commit 027a232
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 110 deletions.
21 changes: 11 additions & 10 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_borrowck/src/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
10 changes: 2 additions & 8 deletions compiler/rustc_const_eval/src/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>>,
}
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down
17 changes: 10 additions & 7 deletions compiler/rustc_resolve/src/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 4 additions & 8 deletions compiler/rustc_symbol_mangling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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))
{
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
35 changes: 27 additions & 8 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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);
}
}
Expand All @@ -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,
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
})
}
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 027a232

Please sign in to comment.