Skip to content

Commit

Permalink
Auto merge of #108487 - cjgillot:no-typeck-mir, r=oli-obk
Browse files Browse the repository at this point in the history
Avoid invoking typeck from borrowck

This PR attempts to reduce direct dependencies between typeck and MIR-related queries. The goal is to have all the information transit either through THIR or through dedicated queries that avoid depending on the whole `TypeckResults`.

In a first commit, we store the type information that MIR building requires into THIR. This avoids edges between mir_built and typeck.

In the second and third commit, we wrap informations around closures (upvars, kind origin and user-provided signature) to avoid borrowck depending on typeck information.

There should be a single remaining borrowck -> typeck edge in the good path, due to inline consts.
  • Loading branch information
bors committed Feb 27, 2023
2 parents 7281249 + cb51d2d commit 6290ae9
Show file tree
Hide file tree
Showing 20 changed files with 223 additions and 207 deletions.
23 changes: 6 additions & 17 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_errors::{
struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_block, walk_expr, Visitor};
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, LangItem};
use rustc_infer::infer::TyCtxtInferExt;
Expand Down Expand Up @@ -236,10 +236,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let ty = used_place.ty(self.body, self.infcx.tcx).ty;
let needs_note = match ty.kind() {
ty::Closure(id, _) => {
let tables = self.infcx.tcx.typeck(id.expect_local());
let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(id.expect_local());

tables.closure_kind_origins().get(hir_id).is_none()
self.infcx.tcx.closure_kind_origin(id.expect_local()).is_none()
}
_ => true,
};
Expand Down Expand Up @@ -1670,27 +1667,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
format!("`{}` would have to be valid for `{}`...", name, region_name),
);

let fn_hir_id = self.mir_hir_id();
err.span_label(
drop_span,
format!(
"...but `{}` will be dropped here, when the {} returns",
name,
self.infcx
.tcx
.hir()
.opt_name(fn_hir_id)
.opt_item_name(self.mir_def_id().to_def_id())
.map(|name| format!("function `{}`", name))
.unwrap_or_else(|| {
match &self
.infcx
.tcx
.typeck(self.mir_def_id())
.node_type(fn_hir_id)
.kind()
{
ty::Closure(..) => "enclosing closure",
ty::Generator(..) => "enclosing generator",
match &self.infcx.tcx.def_kind(self.mir_def_id()) {
DefKind::Closure => "enclosing closure",
DefKind::Generator => "enclosing generator",
kind => bug!("expected closure or generator, found {:?}", kind),
}
.to_string()
Expand Down
24 changes: 5 additions & 19 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
debug!("add_moved_or_invoked_closure_note: closure={:?}", closure);
if let ty::Closure(did, _) = self.body.local_decls[closure].ty.kind() {
let did = did.expect_local();
let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did);

if let Some((span, hir_place)) =
self.infcx.tcx.typeck(did).closure_kind_origins().get(hir_id)
{
if let Some((span, hir_place)) = self.infcx.tcx.closure_kind_origin(did) {
diag.span_note(
*span,
&format!(
Expand All @@ -139,11 +135,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if let Some(target) = target {
if let ty::Closure(did, _) = self.body.local_decls[target].ty.kind() {
let did = did.expect_local();
let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did);

if let Some((span, hir_place)) =
self.infcx.tcx.typeck(did).closure_kind_origins().get(hir_id)
{
if let Some((span, hir_place)) = self.infcx.tcx.closure_kind_origin(did) {
diag.span_note(
*span,
&format!(
Expand Down Expand Up @@ -373,14 +365,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
//
// We know the field exists so it's safe to call operator[] and `unwrap` here.
let def_id = def_id.expect_local();
let var_id = self
.infcx
.tcx
.typeck(def_id)
.closure_min_captures_flattened(def_id)
.nth(field.index())
.unwrap()
.get_root_variable();
let var_id =
self.infcx.tcx.closure_captures(def_id)[field.index()].get_root_variable();

Some(self.infcx.tcx.hir().name(var_id).to_string())
}
Expand Down Expand Up @@ -987,7 +973,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
if let hir::ExprKind::Closure(&hir::Closure { body, fn_decl_span, .. }) = expr {
for (captured_place, place) in
self.infcx.tcx.typeck(def_id).closure_min_captures_flattened(def_id).zip(places)
self.infcx.tcx.closure_captures(def_id).iter().zip(places)
{
match place {
Operand::Copy(place) | Operand::Move(place)
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,10 +901,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
err: &mut Diagnostic,
) {
let tables = tcx.typeck(closure_local_def_id);
let closure_hir_id = tcx.hir().local_def_id_to_hir_id(closure_local_def_id);
if let Some((span, closure_kind_origin)) =
&tables.closure_kind_origins().get(closure_hir_id)
{
if let Some((span, closure_kind_origin)) = tcx.closure_kind_origin(closure_local_def_id) {
let reason = if let PlaceBase::Upvar(upvar_id) = closure_kind_origin.base {
let upvar = ty::place_to_string_for_capture(tcx, closure_kind_origin);
let root_hir_id = upvar_id.var_path.hir_id;
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,14 @@ fn do_mir_borrowck<'tcx>(
let mut errors = error::BorrowckErrors::new(infcx.tcx);

// Gather the upvars of a closure, if any.
let tables = tcx.typeck_opt_const_arg(def);
if let Some(e) = tables.tainted_by_errors {
if let Some(e) = input_body.tainted_by_errors {
infcx.set_tainted_by_errors(e);
errors.set_tainted_by_errors(e);
}
let upvars: Vec<_> = tables
.closure_min_captures_flattened(def.did)
.map(|captured_place| {
let upvars: Vec<_> = tcx
.closure_captures(def.did)
.iter()
.map(|&captured_place| {
let capture = captured_place.info.capture_kind;
let by_ref = match capture {
ty::UpvarCapture::ByValue => false,
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_borrowck/src/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if !self.tcx().is_closure(mir_def_id.to_def_id()) {
return;
}
let Some(user_provided_poly_sig) =
self.tcx().typeck(mir_def_id).user_provided_sigs.get(&mir_def_id)
else {
return;
};
let user_provided_poly_sig = self.tcx().closure_user_provided_sig(mir_def_id);

// Instantiate the canonicalized variables from user-provided signature
// (e.g., the `_` in the code above) with fresh variables.
Expand Down
27 changes: 15 additions & 12 deletions compiler/rustc_borrowck/src/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_errors::Diagnostic;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::lang_items::LangItem;
use rustc_hir::{BodyOwnerKind, HirId};
use rustc_hir::BodyOwnerKind;
use rustc_index::vec::{Idx, IndexVec};
use rustc_infer::infer::NllRegionVariableOrigin;
use rustc_middle::ty::fold::TypeFoldable;
Expand Down Expand Up @@ -231,9 +231,7 @@ impl<'tcx> UniversalRegions<'tcx> {
mir_def: ty::WithOptConstParam<LocalDefId>,
param_env: ty::ParamEnv<'tcx>,
) -> Self {
let tcx = infcx.tcx;
let mir_hir_id = tcx.hir().local_def_id_to_hir_id(mir_def.did);
UniversalRegionsBuilder { infcx, mir_def, mir_hir_id, param_env }.build()
UniversalRegionsBuilder { infcx, mir_def, param_env }.build()
}

/// Given a reference to a closure type, extracts all the values
Expand Down Expand Up @@ -390,7 +388,6 @@ impl<'tcx> UniversalRegions<'tcx> {
struct UniversalRegionsBuilder<'cx, 'tcx> {
infcx: &'cx BorrowckInferCtxt<'cx, 'tcx>,
mir_def: ty::WithOptConstParam<LocalDefId>,
mir_hir_id: HirId,
param_env: ty::ParamEnv<'tcx>,
}

Expand Down Expand Up @@ -560,12 +557,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {

match tcx.hir().body_owner_kind(self.mir_def.did) {
BodyOwnerKind::Closure | BodyOwnerKind::Fn => {
let defining_ty = if self.mir_def.did.to_def_id() == typeck_root_def_id {
tcx.type_of(typeck_root_def_id).subst_identity()
} else {
let tables = tcx.typeck(self.mir_def.did);
tables.node_type(self.mir_hir_id)
};
let defining_ty = tcx.type_of(self.mir_def.def_id_for_type_of()).subst_identity();

debug!("defining_ty (pre-replacement): {:?}", defining_ty);

Expand Down Expand Up @@ -594,7 +586,18 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
self.infcx.replace_free_regions_with_nll_infer_vars(FR, identity_substs);
DefiningTy::Const(self.mir_def.did.to_def_id(), substs)
} else {
let ty = tcx.typeck(self.mir_def.did).node_type(self.mir_hir_id);
// FIXME this line creates a dependency between borrowck and typeck.
//
// This is required for `AscribeUserType` canonical query, which will call
// `type_of(inline_const_def_id)`. That `type_of` would inject erased lifetimes
// into borrowck, which is ICE #78174.
//
// As a workaround, inline consts have an additional generic param (`ty`
// below), so that `type_of(inline_const_def_id).substs(substs)` uses the
// proper type with NLL infer vars.
let ty = tcx
.typeck(self.mir_def.did)
.node_type(tcx.local_def_id_to_hir_id(self.mir_def.did));
let substs = InlineConstSubsts::new(
tcx,
InlineConstSubstsParts { parent_substs: identity_substs, ty },
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// FIXME this should be more descriptive i.e. CapturePlace instead of CapturedVar
// https://github.com/rust-lang/project-rfc-2229/issues/46
if let Some(local_def_id) = def_id.as_local() {
let tables = self.ecx.tcx.typeck(local_def_id);
if let Some(captured_place) =
tables.closure_min_captures_flattened(local_def_id).nth(field)
{
let captures = self.ecx.tcx.closure_captures(local_def_id);
if let Some(captured_place) = captures.get(field) {
// Sometimes the index is beyond the number of upvars (seen
// for a generator).
let var_hir_id = captured_place.get_root_variable();
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
PlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
base => bug!("Expected upvar, found={:?}", base),
};
let var_ident = self.tcx.hir().ident(var_hir_id);

let Some(min_cap_list) = root_var_min_capture_list.get_mut(&var_hir_id) else {
let mutability = self.determine_capture_mutability(&typeck_results, &place);
let min_cap_list = vec![ty::CapturedPlace {
var_ident,
place,
info: capture_info,
mutability,
Expand Down Expand Up @@ -628,6 +630,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if !ancestor_found {
let mutability = self.determine_capture_mutability(&typeck_results, &place);
let captured_place = ty::CapturedPlace {
var_ident,
place,
info: updated_capture_info,
mutability,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ macro_rules! arena_types {
[] bit_set_u32: rustc_index::bit_set::BitSet<u32>,
[] external_constraints: rustc_middle::traits::solve::ExternalConstraintsData<'tcx>,
[decode] doc_link_resolutions: rustc_hir::def::DocLinkResMap,
[] closure_kind_origin: (rustc_span::Span, rustc_middle::hir::place::Place<'tcx>),
]);
)
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,11 @@ impl<'hir> Map<'hir> {
self.opt_ident(id).map(|ident| ident.span)
}

#[inline]
pub fn ident(self, id: HirId) -> Ident {
self.opt_ident(id).unwrap()
}

#[inline]
pub fn opt_name(self, id: HirId) -> Option<Symbol> {
self.opt_ident(id).map(|ident| ident.name)
Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,14 +475,10 @@ rustc_queries! {
}
}

query symbols_for_closure_captures(
key: (LocalDefId, LocalDefId)
) -> &'tcx Vec<rustc_span::Symbol> {
arena_cache
query closure_typeinfo(key: LocalDefId) -> ty::ClosureTypeInfo<'tcx> {
desc {
|tcx| "finding symbols for captures of closure `{}` in `{}`",
tcx.def_path_str(key.1.to_def_id()),
tcx.def_path_str(key.0.to_def_id())
|tcx| "finding symbols for captures of closure `{}`",
tcx.def_path_str(key.to_def_id())
}
}

Expand Down
26 changes: 23 additions & 3 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::mir::interpret::AllocId;
use rustc_middle::mir::{self, BinOp, BorrowKind, FakeReadCause, Field, Mutability, UnOp};
use rustc_middle::ty::adjustment::PointerCast;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AdtDef, Ty, UpvarSubsts};
use rustc_middle::ty::{self, AdtDef, FnSig, Ty, UpvarSubsts};
use rustc_middle::ty::{CanonicalUserType, CanonicalUserTypeAnnotation};
use rustc_span::def_id::LocalDefId;
use rustc_span::{sym, Span, Symbol, DUMMY_SP};
Expand All @@ -32,7 +32,12 @@ use std::ops::Index;
pub mod visit;

macro_rules! thir_with_elements {
($($name:ident: $id:ty => $value:ty => $format:literal,)*) => {
(
$($field_name:ident: $field_ty:ty,)*

@elements:
$($name:ident: $id:ty => $value:ty => $format:literal,)*
) => {
$(
newtype_index! {
#[derive(HashStable)]
Expand All @@ -46,14 +51,20 @@ macro_rules! thir_with_elements {
/// This can be indexed directly by any THIR index (e.g. [`ExprId`]).
#[derive(Debug, HashStable, Clone)]
pub struct Thir<'tcx> {
$(
pub $field_name: $field_ty,
)*
$(
pub $name: IndexVec<$id, $value>,
)*
}

impl<'tcx> Thir<'tcx> {
pub fn new() -> Thir<'tcx> {
pub fn new($($field_name: $field_ty,)*) -> Thir<'tcx> {
Thir {
$(
$field_name,
)*
$(
$name: IndexVec::new(),
)*
Expand All @@ -75,13 +86,22 @@ macro_rules! thir_with_elements {
pub const UPVAR_ENV_PARAM: ParamId = ParamId::from_u32(0);

thir_with_elements! {
body_type: BodyTy<'tcx>,

@elements:
arms: ArmId => Arm<'tcx> => "a{}",
blocks: BlockId => Block => "b{}",
exprs: ExprId => Expr<'tcx> => "e{}",
stmts: StmtId => Stmt<'tcx> => "s{}",
params: ParamId => Param<'tcx> => "p{}",
}

#[derive(Debug, HashStable, Clone)]
pub enum BodyTy<'tcx> {
Const(Ty<'tcx>),
Fn(FnSig<'tcx>),
}

/// Description of a type-checked function parameter.
#[derive(Clone, Debug, HashStable)]
pub struct Param<'tcx> {
Expand Down
Loading

0 comments on commit 6290ae9

Please sign in to comment.