Skip to content

Commit

Permalink
Auto merge of rust-lang#97383 - dingxiangfei2009:restore-region-scope…
Browse files Browse the repository at this point in the history
…-tree-query, r=dingxiangfei2009

Try to cache region_scope_tree as a query

This PR will attempt to restore `region_scope_tree` as a query so that caching works again. It seems that `region_scope_tree` could be re-computed for nested items after all, which could explain the performance regression introduced by rust-lang#95563.

cc `@Mark-Simulacrum` `@pnkfelix` I will try to trigger a perf run here.
  • Loading branch information
bors committed May 28, 2022
2 parents 6831417 + cd76f2e commit 4f39fb1
Show file tree
Hide file tree
Showing 16 changed files with 35 additions and 42 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ macro_rules! arena_types {
[decode] unsafety_check_result: rustc_middle::mir::UnsafetyCheckResult,
[decode] code_region: rustc_middle::mir::coverage::CodeRegion,
[] const_allocs: rustc_middle::mir::interpret::Allocation,
[] region_scope_tree: rustc_middle::middle::region::ScopeTree,
// Required for the incremental on-disk cache
[] mir_keys: rustc_hir::def_id::DefIdSet,
[] dropck_outlives:
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,12 @@ rustc_queries! {
desc { "reachability" }
}

/// Per-body `region::ScopeTree`. The `DefId` should be the owner `DefId` for the body;
/// in the case of closures, this will be redirected to the enclosing function.
query region_scope_tree(def_id: DefId) -> &'tcx crate::middle::region::ScopeTree {
desc { |tcx| "computing drop scopes for `{}`", tcx.def_path_str(def_id) }
}

/// Generates a MIR body for the shim.
query mir_shims(key: ty::InstanceDef<'tcx>) -> mir::Body<'tcx> {
storage(ArenaCacheSelector<'tcx>)
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::hir::place::Place as HirPlace;
use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};
use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource};
use crate::middle::codegen_fn_attrs::CodegenFnAttrs;
use crate::middle::region::ScopeTree;
use crate::middle::resolve_lifetime::{self, LifetimeScopeForPath};
use crate::middle::stability;
use crate::mir::interpret::{self, Allocation, ConstAllocation, ConstValue, Scalar};
Expand Down Expand Up @@ -538,12 +537,6 @@ pub struct TypeckResults<'tcx> {
/// issue by fake reading `t`.
pub closure_fake_reads: FxHashMap<DefId, Vec<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>>,

/// Tracks critical information about regions in a body.
/// This includes containment relationship between regions,
/// liveness relationship between variables and regions and
/// information about yield points.
pub region_scope_tree: ScopeTree,

/// Tracks the rvalue scoping rules which defines finer scoping for rvalue expressions
/// by applying extended parameter rules.
/// Details may be find in `rustc_typeck::check::rvalue_scopes`.
Expand Down Expand Up @@ -586,7 +579,6 @@ impl<'tcx> TypeckResults<'tcx> {
concrete_opaque_types: Default::default(),
closure_min_captures: Default::default(),
closure_fake_reads: Default::default(),
region_scope_tree: Default::default(),
rvalue_scopes: Default::default(),
generator_interior_types: ty::Binder::dummy(Default::default()),
treat_byte_string_as_slice: Default::default(),
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let_scope_stack.push(remainder_scope);

// Declare the bindings, which may create a source scope.
let remainder_span =
remainder_scope.span(this.tcx, &this.typeck_results.region_scope_tree);
let remainder_span = remainder_scope.span(this.tcx, this.region_scope_tree);

let visibility_scope =
Some(this.new_source_scope(remainder_span, LintLevel::Inherited, None));
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.push(block, Statement { source_info, kind: StatementKind::StorageLive(local_id) });
// Altough there is almost always scope for given variable in corner cases
// like #92893 we might get variable with no scope.
if let Some(region_scope) = self.typeck_results.region_scope_tree.var_scope(var.local_id) && schedule_drop{
if let Some(region_scope) = self.region_scope_tree.var_scope(var.local_id) && schedule_drop{
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
}
Place::from(local_id)
Expand All @@ -713,7 +713,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
for_guard: ForGuard,
) {
let local_id = self.var_local_id(var, for_guard);
if let Some(region_scope) = self.typeck_results.region_scope_tree.var_scope(var.local_id) {
if let Some(region_scope) = self.region_scope_tree.var_scope(var.local_id) {
self.schedule_drop(span, region_scope, local_id, DropKind::Value);
}
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ struct Builder<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
infcx: &'a InferCtxt<'a, 'tcx>,
typeck_results: &'tcx TypeckResults<'tcx>,
region_scope_tree: &'tcx region::ScopeTree,
param_env: ty::ParamEnv<'tcx>,

thir: &'a Thir<'tcx>,
Expand Down Expand Up @@ -880,6 +881,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
tcx,
infcx,
typeck_results: tcx.typeck_opt_const_arg(def),
region_scope_tree: tcx.region_scope_tree(def.did),
param_env,
def_id: def.did.to_def_id(),
hir_id,
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -916,8 +916,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}

if scope.region_scope == region_scope {
let region_scope_span =
region_scope.span(self.tcx, &self.typeck_results.region_scope_tree);
let region_scope_span = region_scope.span(self.tcx, &self.region_scope_tree);
// Attribute scope exit drops to scope's closing brace.
let scope_end = self.tcx.sess.source_map().end_point(region_scope_span);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<'tcx> Cx<'tcx> {
tcx,
thir: Thir::new(),
param_env: tcx.param_env(def.did),
region_scope_tree: &typeck_results.region_scope_tree,
region_scope_tree: tcx.region_scope_tree(def.did),
typeck_results,
rvalue_scopes: &typeck_results.rvalue_scopes,
body_owner: def.did.to_def_id(),
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::astconv::{
};
use crate::check::callee::{self, DeferredCallResolution};
use crate::check::method::{self, MethodCallee, SelfSource};
use crate::check::{region, rvalue_scopes};
use crate::check::rvalue_scopes;
use crate::check::{BreakableCtxt, Diverges, Expectation, FnCtxt, LocalTy};

use rustc_data_structures::captures::Captures;
Expand Down Expand Up @@ -622,10 +622,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

pub(in super::super) fn resolve_rvalue_scopes(&self, def_id: DefId) {
let scope_tree = region::region_scope_tree(self.tcx, def_id);
let scope_tree = self.tcx.region_scope_tree(def_id);
let rvalue_scopes = { rvalue_scopes::resolve_rvalue_scopes(self, &scope_tree, def_id) };
let mut typeck_results = self.inh.typeck_results.borrow_mut();
typeck_results.region_scope_tree = scope_tree;
typeck_results.rvalue_scopes = rvalue_scopes;
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/check/generator_interior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub fn resolve_interior<'a, 'tcx>(
let mut visitor = InteriorVisitor {
fcx,
types: FxIndexSet::default(),
region_scope_tree: &typeck_results.region_scope_tree,
region_scope_tree: fcx.tcx.region_scope_tree(def_id),
rvalue_scopes: &typeck_results.rvalue_scopes,
expr_count: 0,
kind,
Expand All @@ -195,7 +195,7 @@ pub fn resolve_interior<'a, 'tcx>(
intravisit::walk_body(&mut visitor, body);

// Check that we visited the same amount of expressions as the RegionResolutionVisitor
let region_expr_count = typeck_results.region_scope_tree.body_expr_count(body_id).unwrap();
let region_expr_count = fcx.tcx.region_scope_tree(def_id).body_expr_count(body_id).unwrap();
assert_eq!(region_expr_count, visitor.expr_count);

// The types are already kept in insertion order.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub fn compute_drop_ranges<'a, 'tcx>(
let consumed_borrowed_places = find_consumed_and_borrowed(fcx, def_id, body);

let typeck_results = &fcx.typeck_results.borrow();
let num_exprs = typeck_results.region_scope_tree.body_expr_count(body.id()).unwrap_or(0);
let num_exprs = fcx.tcx.region_scope_tree(def_id).body_expr_count(body.id()).unwrap_or(0);
let (mut drop_ranges, borrowed_temporaries) = build_control_flow_graph(
fcx.tcx.hir(),
fcx.tcx,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_typeck/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ use crate::require_c_abi_if_c_variadic;
use crate::util::common::indenter;

use self::coercion::DynamicCoerceMany;
use self::region::region_scope_tree;
pub use self::Expectation::*;

#[macro_export]
Expand Down Expand Up @@ -256,6 +257,7 @@ pub fn provide(providers: &mut Providers) {
check_trait_item_well_formed,
check_impl_item_well_formed,
check_mod_item_types,
region_scope_tree,
..*providers
};
}
Expand Down
15 changes: 11 additions & 4 deletions compiler/rustc_typeck/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -797,14 +797,19 @@ impl<'tcx> Visitor<'tcx> for RegionResolutionVisitor<'tcx> {

/// Per-body `region::ScopeTree`. The `DefId` should be the owner `DefId` for the body;
/// in the case of closures, this will be redirected to the enclosing function.
pub fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> ScopeTree {
///
/// Performance: This is a query rather than a simple function to enable
/// re-use in incremental scenarios. We may sometimes need to rerun the
/// type checker even when the HIR hasn't changed, and in those cases
/// we can avoid reconstructing the region scope tree.
pub fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
let typeck_root_def_id = tcx.typeck_root_def_id(def_id);
if typeck_root_def_id != def_id {
return region_scope_tree(tcx, typeck_root_def_id);
return tcx.region_scope_tree(typeck_root_def_id);
}

let id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local());
if let Some(body_id) = tcx.hir().maybe_body_owned_by(id) {
let scope_tree = if let Some(body_id) = tcx.hir().maybe_body_owned_by(id) {
let mut visitor = RegionResolutionVisitor {
tcx,
scope_tree: ScopeTree::default(),
Expand All @@ -821,5 +826,7 @@ pub fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> ScopeTree {
visitor.scope_tree
} else {
ScopeTree::default()
}
};

tcx.arena.alloc(scope_tree)
}
2 changes: 0 additions & 2 deletions compiler/rustc_typeck/src/check/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
wbcx.visit_user_provided_sigs();
wbcx.visit_generator_interior_types();

wbcx.typeck_results.region_scope_tree =
mem::take(&mut self.typeck_results.borrow_mut().region_scope_tree);
wbcx.typeck_results.rvalue_scopes =
mem::take(&mut self.typeck_results.borrow_mut().rvalue_scopes);

Expand Down
13 changes: 2 additions & 11 deletions src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ pub(super) fn check<'tcx>(
// ensure that the indexed variable was declared before the loop, see #601
if let Some(indexed_extent) = indexed_extent {
let parent_def_id = cx.tcx.hir().get_parent_item(expr.hir_id);
let parent_body_id = cx
.tcx
.hir()
.body_owned_by(cx.tcx.hir().local_def_id_to_hir_id(parent_def_id));
let region_scope_tree = &cx.tcx.typeck_body(parent_body_id).region_scope_tree;
let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id);
let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).unwrap();
if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) {
return;
Expand Down Expand Up @@ -282,14 +278,9 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
match res {
Res::Local(hir_id) => {
let parent_def_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
let parent_body_id = self.cx
.tcx
.hir()
.body_owned_by(self.cx.tcx.hir().local_def_id_to_hir_id(parent_def_id));
let extent = self.cx
.tcx
.typeck_body(parent_body_id)
.region_scope_tree
.region_scope_tree(parent_def_id)
.var_scope(hir_id.local_id)
.unwrap();
if index_used_directly {
Expand Down
5 changes: 1 addition & 4 deletions src/tools/clippy/clippy_lints/src/shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,7 @@ fn is_shadow(
first: ItemLocalId,
second: ItemLocalId,
) -> bool {
let scope_tree = &cx
.tcx
.typeck_body(cx.tcx.hir().body_owned_by(cx.tcx.hir().local_def_id_to_hir_id(owner)))
.region_scope_tree;
let scope_tree = cx.tcx.region_scope_tree(owner);
let first_scope = scope_tree.var_scope(first).unwrap();
let second_scope = scope_tree.var_scope(second).unwrap();
scope_tree.is_subscope_of(second_scope, first_scope)
Expand Down

0 comments on commit 4f39fb1

Please sign in to comment.