Skip to content

Commit

Permalink
Auto merge of #96840 - cjgillot:query-feed, r=oli-obk
Browse files Browse the repository at this point in the history
Allow to feed a value in another query's cache and remove `WithOptConstParam`

I used it to remove `WithOptConstParam` queries, as an example.

The idea is that a query (here `typeck(function)`) can write into another query's cache (here `type_of(anon const)`). The dependency node for `type_of` would depend on all the current dependencies of `typeck`.

There is still an issue with cycles: if `type_of(anon const)` is accessed before `typeck(function)`, we will still have the usual cycle.  The way around this issue is to `ensure` that `typeck(function)` is called before accessing `type_of(anon const)`.

When replayed, we may the following cases:
- `typeck` is green, in that case `type_of` is green too, and all is right;
- `type_of` is green, `typeck` may still be marked as red (it depends on strictly more things than `type_of`) -> we verify that the saved value and the re-computed value of `type_of` have the same hash;
- `type_of` is red, then `typeck` is red -> it's the caller responsibility to ensure `typeck` is recomputed *before* `type_of`.

As `anon consts` have their own `DefPathData`, it's not possible to have the def-id of the anon-const point to something outside the original function, but the general case may have to be resolved before using this device more broadly.

There is an open question about loading from the on-disk cache.  If `typeck` is loaded from the on-disk cache, the side-effect does not happen. The regular `type_of` implementation can go and fetch the correct value from the decoded `typeck` results, and the dep-graph will check that the hashes match, but I'm not sure we want to rely on this behaviour.

I specifically allowed to feed the value to `type_of` from inside a call to `type_of`.  In that case, the dep-graph will check that the fingerprints of both values match.

This implementation is still very sensitive to cycles, and requires that we call `typeck(function)` before `typeck(anon const)`.  The reason is that `typeck(anon const)` calls `type_of(anon const)`, which calls `typeck(function)`, which feeds `type_of(anon const)`, and needs to build the MIR so needs `typeck(anon const)`.  The latter call would not cycle, since `type_of(anon const)` has been set, but I'd rather not remove the cycle check.
  • Loading branch information
bors committed Apr 21, 2023
2 parents b92a41c + 9bab866 commit 1f5768b
Show file tree
Hide file tree
Showing 81 changed files with 595 additions and 1,194 deletions.
9 changes: 3 additions & 6 deletions compiler/rustc_borrowck/src/consumers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hir::def_id::LocalDefId;
use rustc_index::vec::IndexSlice;
use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt};
use rustc_middle::mir::Body;
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::ty::TyCtxt;

pub use super::{
facts::{AllFacts as PoloniusInput, RustcFacts},
Expand All @@ -28,12 +28,9 @@ pub use super::{
/// that shows how to do this at `tests/run-make/obtain-borrowck/`.
///
/// * Polonius is highly unstable, so expect regular changes in its signature or other details.
pub fn get_body_with_borrowck_facts(
tcx: TyCtxt<'_>,
def: ty::WithOptConstParam<LocalDefId>,
) -> BodyWithBorrowckFacts<'_> {
pub fn get_body_with_borrowck_facts(tcx: TyCtxt<'_>, def: LocalDefId) -> BodyWithBorrowckFacts<'_> {
let (input_body, promoted) = tcx.mir_promoted(def);
let infcx = tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bind(def.did)).build();
let infcx = tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bind(def)).build();
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexSlice<_, _> = &promoted.borrow();
*super::do_mir_borrowck(&infcx, input_body, promoted, true).1.unwrap()
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc_middle::mir::{
FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef,
ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, VarBindingForm,
};
use rustc_middle::ty::{self, suggest_constraining_type_params, PredicateKind, Ty, TypeckResults};
use rustc_middle::ty::{self, suggest_constraining_type_params, PredicateKind, Ty};
use rustc_middle::util::CallKind;
use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex};
use rustc_span::def_id::LocalDefId;
Expand Down Expand Up @@ -1350,8 +1350,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
finder.visit_expr(body_expr);
let Some((closure_expr, closure)) = finder.res else { return };

let typeck_results: &TypeckResults<'_> =
tcx.typeck_opt_const_arg(self.body.source.with_opt_param().as_local().unwrap());
let typeck_results = tcx.typeck(self.mir_def_id());

// Check that the parent of the closure is a method call,
// with receiver matching with local's type (modulo refs)
Expand Down
30 changes: 9 additions & 21 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,24 +119,12 @@ impl<'tcx> TyCtxtConsts<'tcx> {
}

pub fn provide(providers: &mut Providers) {
*providers = Providers {
mir_borrowck: |tcx, did| {
if let Some(def) = ty::WithOptConstParam::try_lookup(did, tcx) {
tcx.mir_borrowck_const_arg(def)
} else {
mir_borrowck(tcx, ty::WithOptConstParam::unknown(did))
}
},
mir_borrowck_const_arg: |tcx, (did, param_did)| {
mir_borrowck(tcx, ty::WithOptConstParam { did, const_param_did: Some(param_did) })
},
..*providers
};
*providers = Providers { mir_borrowck, ..*providers };
}

fn mir_borrowck(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> &BorrowCheckResult<'_> {
fn mir_borrowck(tcx: TyCtxt<'_>, def: LocalDefId) -> &BorrowCheckResult<'_> {
let (input_body, promoted) = tcx.mir_promoted(def);
debug!("run query mir_borrowck: {}", tcx.def_path_str(def.did.to_def_id()));
debug!("run query mir_borrowck: {}", tcx.def_path_str(def.to_def_id()));

if input_body.borrow().should_skip() {
debug!("Skipping borrowck because of injected body");
Expand All @@ -150,7 +138,7 @@ fn mir_borrowck(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> &Bor
return tcx.arena.alloc(result);
}

let hir_owner = tcx.hir().local_def_id_to_hir_id(def.did).owner;
let hir_owner = tcx.hir().local_def_id_to_hir_id(def).owner;

let infcx =
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bind(hir_owner.def_id)).build();
Expand All @@ -167,19 +155,19 @@ fn mir_borrowck(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> &Bor
/// If `return_body_with_facts` is true, then return the body with non-erased
/// region ids on which the borrow checking was performed together with Polonius
/// facts.
#[instrument(skip(infcx, input_body, input_promoted), fields(id=?input_body.source.with_opt_param().as_local().unwrap()), level = "debug")]
#[instrument(skip(infcx, input_body, input_promoted), fields(id=?input_body.source.def_id()), level = "debug")]
fn do_mir_borrowck<'tcx>(
infcx: &InferCtxt<'tcx>,
input_body: &Body<'tcx>,
input_promoted: &IndexSlice<Promoted, Body<'tcx>>,
return_body_with_facts: bool,
) -> (BorrowCheckResult<'tcx>, Option<Box<BodyWithBorrowckFacts<'tcx>>>) {
let def = input_body.source.with_opt_param().as_local().unwrap();
let def = input_body.source.def_id().expect_local();
debug!(?def);

let tcx = infcx.tcx;
let infcx = BorrowckInferCtxt::new(infcx);
let param_env = tcx.param_env(def.did);
let param_env = tcx.param_env(def);

let mut local_names = IndexVec::from_elem(None, &input_body.local_decls);
for var_debug_info in &input_body.var_debug_info {
Expand Down Expand Up @@ -207,7 +195,7 @@ fn do_mir_borrowck<'tcx>(
errors.set_tainted_by_errors(e);
}
let upvars: Vec<_> = tcx
.closure_captures(def.did)
.closure_captures(def)
.iter()
.map(|&captured_place| {
let capture = captured_place.info.capture_kind;
Expand Down Expand Up @@ -249,7 +237,7 @@ fn do_mir_borrowck<'tcx>(
.iterate_to_fixpoint()
.into_results_cursor(&body);

let locals_are_invalidated_at_exit = tcx.hir().body_owner_kind(def.did).is_fn_or_closure();
let locals_are_invalidated_at_exit = tcx.hir().body_owner_kind(def).is_fn_or_closure();
let borrow_set =
Rc::new(BorrowSet::build(tcx, body, locals_are_invalidated_at_exit, &mdpe.move_data));

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub(crate) fn replace_regions_in_mir<'tcx>(
body: &mut Body<'tcx>,
promoted: &mut IndexSlice<Promoted, Body<'tcx>>,
) -> UniversalRegions<'tcx> {
let def = body.source.with_opt_param().as_local().unwrap();
let def = body.source.def_id().expect_local();

debug!(?def);

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
self.cx.ascribe_user_type(
constant.literal.ty(),
UserType::TypeOf(
uv.def.did,
uv.def,
UserSubsts { substs: uv.substs, user_self_ty: None },
),
locations.span(&self.cx.body),
Expand Down Expand Up @@ -1766,7 +1766,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if let Some(uv) = maybe_uneval {
if uv.promoted.is_none() {
let tcx = self.tcx();
let def_id = uv.def.def_id_for_type_of();
let def_id = uv.def;
if tcx.def_kind(def_id) == DefKind::InlineConst {
let def_id = def_id.expect_local();
let predicates =
Expand Down
48 changes: 24 additions & 24 deletions compiler/rustc_borrowck/src/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ impl<'tcx> UniversalRegions<'tcx> {
/// known between those regions.
pub fn new(
infcx: &BorrowckInferCtxt<'_, 'tcx>,
mir_def: ty::WithOptConstParam<LocalDefId>,
mir_def: LocalDefId,
param_env: ty::ParamEnv<'tcx>,
) -> Self {
UniversalRegionsBuilder { infcx, mir_def, param_env }.build()
Expand Down Expand Up @@ -388,7 +388,7 @@ impl<'tcx> UniversalRegions<'tcx> {

struct UniversalRegionsBuilder<'cx, 'tcx> {
infcx: &'cx BorrowckInferCtxt<'cx, 'tcx>,
mir_def: ty::WithOptConstParam<LocalDefId>,
mir_def: LocalDefId,
param_env: ty::ParamEnv<'tcx>,
}

Expand Down Expand Up @@ -417,12 +417,12 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
let mut indices = self.compute_indices(fr_static, defining_ty);
debug!("build: indices={:?}", indices);

let typeck_root_def_id = self.infcx.tcx.typeck_root_def_id(self.mir_def.did.to_def_id());
let typeck_root_def_id = self.infcx.tcx.typeck_root_def_id(self.mir_def.to_def_id());

// If this is a 'root' body (not a closure/generator/inline const), then
// there are no extern regions, so the local regions start at the same
// position as the (empty) sub-list of extern regions
let first_local_index = if self.mir_def.did.to_def_id() == typeck_root_def_id {
let first_local_index = if self.mir_def.to_def_id() == typeck_root_def_id {
first_extern_index
} else {
// If this is a closure, generator, or inline-const, then the late-bound regions from the enclosing
Expand All @@ -433,7 +433,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
// }
for_each_late_bound_region_in_recursive_scope(
self.infcx.tcx,
self.infcx.tcx.local_parent(self.mir_def.did),
self.infcx.tcx.local_parent(self.mir_def),
|r| {
debug!(?r);
if !indices.indices.contains_key(&r) {
Expand Down Expand Up @@ -462,13 +462,13 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {

let inputs_and_output = self.infcx.replace_bound_regions_with_nll_infer_vars(
FR,
self.mir_def.did,
self.mir_def,
bound_inputs_and_output,
&mut indices,
);
// Converse of above, if this is a function/closure then the late-bound regions declared on its
// signature are local.
for_each_late_bound_region_in_item(self.infcx.tcx, self.mir_def.did, |r| {
for_each_late_bound_region_in_item(self.infcx.tcx, self.mir_def, |r| {
debug!(?r);
if !indices.indices.contains_key(&r) {
let region_vid = {
Expand All @@ -492,7 +492,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
if self.infcx.tcx.fn_sig(def_id).skip_binder().c_variadic() {
let va_list_did = self.infcx.tcx.require_lang_item(
LangItem::VaList,
Some(self.infcx.tcx.def_span(self.mir_def.did)),
Some(self.infcx.tcx.def_span(self.mir_def)),
);

let reg_vid = self
Expand Down Expand Up @@ -544,11 +544,11 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
/// see `DefiningTy` for details.
fn defining_ty(&self) -> DefiningTy<'tcx> {
let tcx = self.infcx.tcx;
let typeck_root_def_id = tcx.typeck_root_def_id(self.mir_def.did.to_def_id());
let typeck_root_def_id = tcx.typeck_root_def_id(self.mir_def.to_def_id());

match tcx.hir().body_owner_kind(self.mir_def.did) {
match tcx.hir().body_owner_kind(self.mir_def) {
BodyOwnerKind::Closure | BodyOwnerKind::Fn => {
let defining_ty = tcx.type_of(self.mir_def.def_id_for_type_of()).subst_identity();
let defining_ty = tcx.type_of(self.mir_def).subst_identity();

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

Expand All @@ -562,20 +562,20 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
}
ty::FnDef(def_id, substs) => DefiningTy::FnDef(def_id, substs),
_ => span_bug!(
tcx.def_span(self.mir_def.did),
tcx.def_span(self.mir_def),
"expected defining type for `{:?}`: `{:?}`",
self.mir_def.did,
self.mir_def,
defining_ty
),
}
}

BodyOwnerKind::Const | BodyOwnerKind::Static(..) => {
let identity_substs = InternalSubsts::identity_for_item(tcx, typeck_root_def_id);
if self.mir_def.did.to_def_id() == typeck_root_def_id {
if self.mir_def.to_def_id() == typeck_root_def_id {
let substs =
self.infcx.replace_free_regions_with_nll_infer_vars(FR, identity_substs);
DefiningTy::Const(self.mir_def.did.to_def_id(), substs)
DefiningTy::Const(self.mir_def.to_def_id(), substs)
} else {
// FIXME this line creates a dependency between borrowck and typeck.
//
Expand All @@ -587,15 +587,15 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
// 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));
.typeck(self.mir_def)
.node_type(tcx.local_def_id_to_hir_id(self.mir_def));
let substs = InlineConstSubsts::new(
tcx,
InlineConstSubstsParts { parent_substs: identity_substs, ty },
)
.substs;
let substs = self.infcx.replace_free_regions_with_nll_infer_vars(FR, substs);
DefiningTy::InlineConst(self.mir_def.did.to_def_id(), substs)
DefiningTy::InlineConst(self.mir_def.to_def_id(), substs)
}
}
}
Expand All @@ -611,7 +611,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
defining_ty: DefiningTy<'tcx>,
) -> UniversalRegionIndices<'tcx> {
let tcx = self.infcx.tcx;
let typeck_root_def_id = tcx.typeck_root_def_id(self.mir_def.did.to_def_id());
let typeck_root_def_id = tcx.typeck_root_def_id(self.mir_def.to_def_id());
let identity_substs = InternalSubsts::identity_for_item(tcx, typeck_root_def_id);
let fr_substs = match defining_ty {
DefiningTy::Closure(_, substs)
Expand Down Expand Up @@ -647,7 +647,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
let tcx = self.infcx.tcx;
match defining_ty {
DefiningTy::Closure(def_id, substs) => {
assert_eq!(self.mir_def.did.to_def_id(), def_id);
assert_eq!(self.mir_def.to_def_id(), def_id);
let closure_sig = substs.as_closure().sig();
let inputs_and_output = closure_sig.inputs_and_output();
let bound_vars = tcx.mk_bound_variable_kinds_from_iter(
Expand Down Expand Up @@ -682,7 +682,7 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
}

DefiningTy::Generator(def_id, substs, movability) => {
assert_eq!(self.mir_def.did.to_def_id(), def_id);
assert_eq!(self.mir_def.to_def_id(), def_id);
let resume_ty = substs.as_generator().resume_ty();
let output = substs.as_generator().return_ty();
let generator_ty = tcx.mk_generator(def_id, substs, movability);
Expand All @@ -700,14 +700,14 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
DefiningTy::Const(def_id, _) => {
// For a constant body, there are no inputs, and one
// "output" (the type of the constant).
assert_eq!(self.mir_def.did.to_def_id(), def_id);
let ty = tcx.type_of(self.mir_def.def_id_for_type_of()).subst_identity();
assert_eq!(self.mir_def.to_def_id(), def_id);
let ty = tcx.type_of(self.mir_def).subst_identity();
let ty = indices.fold_to_region_vids(tcx, ty);
ty::Binder::dummy(tcx.mk_type_list(&[ty]))
}

DefiningTy::InlineConst(def_id, substs) => {
assert_eq!(self.mir_def.did.to_def_id(), def_id);
assert_eq!(self.mir_def.to_def_id(), def_id);
let ty = substs.as_inline_const().ty();
ty::Binder::dummy(tcx.mk_type_list(&[ty]))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub(crate) fn eval_mir_constant<'tcx>(
),
},
ConstantKind::Unevaluated(mir::UnevaluatedConst { def, .. }, _)
if fx.tcx.is_static(def.did) =>
if fx.tcx.is_static(def) =>
{
span_bug!(constant.span, "MIR constant refers to static");
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ fn exported_symbols_provider_local(
match *mono_item {
MonoItem::Fn(Instance { def: InstanceDef::Item(def), substs }) => {
if substs.non_erasable_generics().next().is_some() {
let symbol = ExportedSymbol::Generic(def.did, substs);
let symbol = ExportedSymbol::Generic(def, substs);
symbols.push((
symbol,
SymbolExportInfo {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,12 +296,12 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
}

let cid = key.value;
let def = cid.instance.def.with_opt_param();
let is_static = tcx.is_static(def.did);
let def = cid.instance.def.def_id();
let is_static = tcx.is_static(def);

let mut ecx = InterpCx::new(
tcx,
tcx.def_span(def.did),
tcx.def_span(def),
key.param_env,
// Statics (and promoteds inside statics) may access other statics, because unlike consts
// they do not have to behave "as if" they were evaluated at runtime.
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
) -> InterpResult<'tcx, &'tcx mir::Body<'tcx>> {
match instance {
ty::InstanceDef::Item(def) => {
if ecx.tcx.is_ctfe_mir_available(def.did) {
Ok(ecx.tcx.mir_for_ctfe_opt_const_arg(def))
} else if ecx.tcx.def_kind(def.did) == DefKind::AssocConst {
if ecx.tcx.is_ctfe_mir_available(def) {
Ok(ecx.tcx.mir_for_ctfe(def))
} else if ecx.tcx.def_kind(def) == DefKind::AssocConst {
let guar = ecx.tcx.sess.delay_span_bug(
rustc_span::DUMMY_SP,
"This is likely a const item that is missing from its impl",
Expand All @@ -386,7 +386,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
} else {
// `find_mir_or_eval_fn` checks that this is a const fn before even calling us,
// so this should be unreachable.
let path = ecx.tcx.def_path_str(def.did);
let path = ecx.tcx.def_path_str(def);
bug!("trying to call extern function `{path}` at compile-time");
}
}
Expand All @@ -410,9 +410,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// Execution might have wandered off into other crates, so we cannot do a stability-
// sensitive check here. But we can at least rule out functions that are not const
// at all.
if !ecx.tcx.is_const_fn_raw(def.did) {
if !ecx.tcx.is_const_fn_raw(def) {
// allow calling functions inside a trait marked with #[const_trait].
if !ecx.tcx.is_const_default_method(def.did) {
if !ecx.tcx.is_const_default_method(def) {
// We certainly do *not* want to actually call the fn
// though, so be sure we return here.
throw_unsup_format!("calling non-const function `{}`", instance)
Expand Down
Loading

0 comments on commit 1f5768b

Please sign in to comment.