From c3802087dcc841aae7bf0825ff31600d23ca2f01 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 7 Jan 2024 11:37:32 +0100 Subject: [PATCH] fix: Fix mir evaluator not respecting block local items --- crates/hir-ty/src/method_resolution.rs | 122 ++++++++++++++-------- crates/hir-ty/src/mir.rs | 6 +- crates/hir-ty/src/mir/borrowck.rs | 14 ++- crates/hir-ty/src/mir/eval.rs | 42 +++++--- crates/hir-ty/src/mir/eval/tests.rs | 35 +++++++ crates/hir-ty/src/mir/lower.rs | 78 ++++++++++---- crates/hir-ty/src/mir/monomorphization.rs | 4 +- crates/hir-ty/src/mir/pretty.rs | 4 + 8 files changed, 221 insertions(+), 84 deletions(-) diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index 041d61c1b153..1359d94f5709 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -2,7 +2,7 @@ //! For details about how this works in rustc, see the method lookup page in the //! [rustc guide](https://rust-lang.github.io/rustc-guide/method-lookup.html) //! and the corresponding code mostly in rustc_hir_analysis/check/method/probe.rs. -use std::ops::ControlFlow; +use std::{iter, ops::ControlFlow}; use base_db::{CrateId, Edition}; use chalk_ir::{cast::Cast, Mutability, TyKind, UniverseIndex, WhereClause}; @@ -705,15 +705,13 @@ pub(crate) fn lookup_impl_method_query( }; let name = &db.function_data(func).name; - let Some((impl_fn, impl_subst)) = - lookup_impl_assoc_item_for_trait_ref(trait_ref, db, env, name).and_then(|assoc| { - if let (AssocItemId::FunctionId(id), subst) = assoc { - Some((id, subst)) - } else { - None - } - }) - else { + let Some((impl_fn, impl_subst)) = lookup_impl_assoc_item_for_trait_ref( + trait_ref, db, env, name, + ) + .and_then(|assoc| match assoc { + (AssocItemId::FunctionId(id), subst) => Some((id, subst)), + _ => None, + }) else { return (func, fn_subst); }; ( @@ -734,21 +732,54 @@ fn lookup_impl_assoc_item_for_trait_ref( let hir_trait_id = trait_ref.hir_trait_id(); let self_ty = trait_ref.self_type_parameter(Interner); let self_ty_fp = TyFingerprint::for_trait_impl(&self_ty)?; - let impls = db.trait_impls_in_deps(env.krate); - let self_impls = match self_ty.kind(Interner) { - TyKind::Adt(id, _) => { - id.0.module(db.upcast()).containing_block().map(|it| db.trait_impls_in_block(it)) + + let trait_module = hir_trait_id.module(db.upcast()); + let type_module = match self_ty_fp { + TyFingerprint::Adt(adt_id) => Some(adt_id.module(db.upcast())), + TyFingerprint::ForeignType(type_id) => { + Some(from_foreign_def_id(type_id).module(db.upcast())) } + TyFingerprint::Dyn(trait_id) => Some(trait_id.module(db.upcast())), _ => None, }; - let impls = impls - .iter() - .chain(self_impls.as_ref()) - .flat_map(|impls| impls.for_trait_and_self_ty(hir_trait_id, self_ty_fp)); - let table = InferenceTable::new(db, env); + let mut def_blocks = + [trait_module.containing_block(), type_module.and_then(|it| it.containing_block())]; + + let impls = db.trait_impls_in_deps(env.krate); + let in_self = db.trait_impls_in_crate(env.krate); + let block_impls = iter::successors(env.block, |&block_id| { + db.block_def_map(block_id).parent().and_then(|module| module.containing_block()) + }) + .inspect(|&block_id| { + // make sure we don't search the same block twice + def_blocks.iter_mut().for_each(|block| { + if *block == Some(block_id) { + *block = None; + } + }); + }) + .map(|block_id| db.trait_impls_in_block(block_id)); + + let impls = block_impls.chain(iter::once(in_self).chain(impls.iter().cloned())); + + let mut table = InferenceTable::new(db, env); - let (impl_data, impl_subst) = find_matching_impl(impls, table, trait_ref)?; + let (impl_data, impl_subst) = + match find_matching_impl(impls, &mut table, trait_ref.clone(), self_ty_fp) { + Some(it) => it, + None => { + if def_blocks.iter().all(Option::is_none) { + return None; + } + find_matching_impl( + def_blocks.into_iter().flatten().map(|b| db.trait_impls_in_block(b)), + &mut table, + trait_ref, + self_ty_fp, + )? + } + }; let item = impl_data.items.iter().find_map(|&it| match it { AssocItemId::FunctionId(f) => { (db.function_data(f).name == *name).then_some(AssocItemId::FunctionId(f)) @@ -765,33 +796,38 @@ fn lookup_impl_assoc_item_for_trait_ref( } fn find_matching_impl( - mut impls: impl Iterator, - mut table: InferenceTable<'_>, + mut impls: impl Iterator>, + table: &mut InferenceTable<'_>, actual_trait_ref: TraitRef, + self_ty_fp: TyFingerprint, ) -> Option<(Arc, Substitution)> { let db = table.db; - impls.find_map(|impl_| { - table.run_in_snapshot(|table| { - let impl_data = db.impl_data(impl_); - let impl_substs = - TyBuilder::subst_for_def(db, impl_, None).fill_with_inference_vars(table).build(); - let trait_ref = db - .impl_trait(impl_) - .expect("non-trait method in find_matching_impl") - .substitute(Interner, &impl_substs); - - if !table.unify(&trait_ref, &actual_trait_ref) { - return None; - } + let hir_trait_id = actual_trait_ref.hir_trait_id(); + impls.find_map(|impls| { + for impl_ in impls.for_trait_and_self_ty(hir_trait_id, self_ty_fp) { + return table.run_in_snapshot(|table| { + let impl_substs = TyBuilder::subst_for_def(db, impl_, None) + .fill_with_inference_vars(table) + .build(); + let trait_ref = db + .impl_trait(impl_) + .expect("non-trait method in find_matching_impl") + .substitute(Interner, &impl_substs); + + if !table.unify(&trait_ref, &actual_trait_ref) { + return None; + } - let wcs = crate::chalk_db::convert_where_clauses(db, impl_.into(), &impl_substs) - .into_iter() - .map(|b| b.cast(Interner)); - let goal = crate::Goal::all(Interner, wcs); - table.try_obligation(goal.clone())?; - table.register_obligation(goal); - Some((impl_data, table.resolve_completely(impl_substs))) - }) + let wcs = crate::chalk_db::convert_where_clauses(db, impl_.into(), &impl_substs) + .into_iter() + .map(|b| b.cast(Interner)); + let goal = crate::Goal::all(Interner, wcs); + table.try_obligation(goal.clone())?; + table.register_obligation(goal); + Some((db.impl_data(impl_), table.resolve_completely(impl_substs))) + }); + } + None }) } diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index 7bef6f0d0f71..3e0430e2c870 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -1016,6 +1016,8 @@ pub enum Rvalue { #[derive(Debug, PartialEq, Eq, Clone)] pub enum StatementKind { + TraitEnvBlockEnter(hir_def::BlockId), + TraitEnvBlockExit, Assign(Place, Rvalue), FakeRead(Place), //SetDiscriminant { @@ -1126,7 +1128,9 @@ impl MirBody { StatementKind::FakeRead(p) | StatementKind::Deinit(p) => { f(p, &mut self.projection_store) } - StatementKind::StorageLive(_) + StatementKind::TraitEnvBlockEnter(_) + | StatementKind::TraitEnvBlockExit + | StatementKind::StorageLive(_) | StatementKind::StorageDead(_) | StatementKind::Nop => (), } diff --git a/crates/hir-ty/src/mir/borrowck.rs b/crates/hir-ty/src/mir/borrowck.rs index e79c87a02f40..f3f19bb0ea05 100644 --- a/crates/hir-ty/src/mir/borrowck.rs +++ b/crates/hir-ty/src/mir/borrowck.rs @@ -149,7 +149,9 @@ fn moved_out_of_ref(db: &dyn HirDatabase, body: &MirBody) -> Vec | StatementKind::Deinit(_) | StatementKind::StorageLive(_) | StatementKind::StorageDead(_) - | StatementKind::Nop => (), + | StatementKind::Nop + | StatementKind::TraitEnvBlockEnter(_) + | StatementKind::TraitEnvBlockExit => (), } } match &block.terminator { @@ -269,7 +271,9 @@ fn ever_initialized_map( StatementKind::Deinit(_) | StatementKind::FakeRead(_) | StatementKind::Nop - | StatementKind::StorageLive(_) => (), + | StatementKind::StorageLive(_) + | StatementKind::TraitEnvBlockEnter(_) + | StatementKind::TraitEnvBlockExit => (), } } let Some(terminator) = &block.terminator else { @@ -424,7 +428,11 @@ fn mutability_of_locals( StatementKind::StorageDead(p) => { ever_init_map.insert(*p, false); } - StatementKind::Deinit(_) | StatementKind::StorageLive(_) | StatementKind::Nop => (), + StatementKind::Deinit(_) + | StatementKind::StorageLive(_) + | StatementKind::Nop + | StatementKind::TraitEnvBlockEnter(_) + | StatementKind::TraitEnvBlockExit => (), } } let Some(terminator) = &block.terminator else { diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs index 5650956d2f4c..14e76365660f 100644 --- a/crates/hir-ty/src/mir/eval.rs +++ b/crates/hir-ty/src/mir/eval.rs @@ -144,7 +144,7 @@ enum MirOrDynIndex { pub struct Evaluator<'a> { db: &'a dyn HirDatabase, - trait_env: Arc, + trait_env: Vec>, stack: Vec, heap: Vec, code_stack: Vec, @@ -578,7 +578,7 @@ impl Evaluator<'_> { static_locations: Default::default(), db, random_state: oorandom::Rand64::new(0), - trait_env: trait_env.unwrap_or_else(|| db.trait_environment_for_body(owner)), + trait_env: vec![trait_env.unwrap_or_else(|| db.trait_environment_for_body(owner))], crate_id, stdout: vec![], stderr: vec![], @@ -784,7 +784,7 @@ impl Evaluator<'_> { } let r = self .db - .layout_of_ty(ty.clone(), self.trait_env.clone()) + .layout_of_ty(ty.clone(), self.trait_env.last().unwrap().clone()) .map_err(|e| MirEvalError::LayoutError(e, ty.clone()))?; self.layout_cache.borrow_mut().insert(ty.clone(), r.clone()); Ok(r) @@ -861,6 +861,12 @@ impl Evaluator<'_> { | StatementKind::FakeRead(_) | StatementKind::StorageDead(_) | StatementKind::Nop => (), + &StatementKind::TraitEnvBlockEnter(block) => { + let mut with_block = self.trait_env.last().unwrap().clone(); + TraitEnvironment::with_block(&mut with_block, block); + self.trait_env.push(with_block); + } + StatementKind::TraitEnvBlockExit => _ = self.trait_env.pop(), } } let Some(terminator) = current_block.terminator.as_ref() else { @@ -1689,13 +1695,22 @@ impl Evaluator<'_> { let mut const_id = *const_id; let mut subst = subst.clone(); if let hir_def::GeneralConstId::ConstId(c) = const_id { - let (c, s) = lookup_impl_const(self.db, self.trait_env.clone(), c, subst); + let (c, s) = lookup_impl_const( + self.db, + self.trait_env.last().unwrap().clone(), + c, + subst, + ); const_id = hir_def::GeneralConstId::ConstId(c); subst = s; } result_owner = self .db - .const_eval(const_id.into(), subst, Some(self.trait_env.clone())) + .const_eval( + const_id.into(), + subst, + Some(self.trait_env.last().unwrap().clone()), + ) .map_err(|e| { let name = const_id.name(self.db.upcast()); MirEvalError::ConstEvalError(name, Box::new(e)) @@ -2015,7 +2030,7 @@ impl Evaluator<'_> { if let Some((v, l)) = detect_variant_from_bytes( &layout, this.db, - this.trait_env.clone(), + this.trait_env.last().unwrap().clone(), bytes, e, ) { @@ -2096,7 +2111,7 @@ impl Evaluator<'_> { if let Some((variant, layout)) = detect_variant_from_bytes( &layout, self.db, - self.trait_env.clone(), + self.trait_env.last().unwrap().clone(), self.read_memory(addr, layout.size.bytes_usize())?, e, ) { @@ -2197,7 +2212,7 @@ impl Evaluator<'_> { .monomorphized_mir_body_for_closure( closure, generic_args.clone(), - self.trait_env.clone(), + self.trait_env.last().unwrap().clone(), ) .map_err(|it| MirEvalError::MirLowerErrorForClosure(closure, it))?; let closure_data = if mir_body.locals[mir_body.param_locals[0]].ty.as_reference().is_some() @@ -2295,17 +2310,16 @@ impl Evaluator<'_> { return Ok(r.clone()); } let (def, generic_args) = pair; + let env = self.trait_env.last().unwrap().clone(); let r = if let Some(self_ty_idx) = - is_dyn_method(self.db, self.trait_env.clone(), def, generic_args.clone()) + is_dyn_method(self.db, env.clone(), def, generic_args.clone()) { MirOrDynIndex::Dyn(self_ty_idx) } else { let (imp, generic_args) = - self.db.lookup_impl_method(self.trait_env.clone(), def, generic_args.clone()); - let mir_body = self - .db - .monomorphized_mir_body(imp.into(), generic_args, self.trait_env.clone()) - .map_err(|e| { + self.db.lookup_impl_method(env.clone(), def, generic_args.clone()); + let mir_body = + self.db.monomorphized_mir_body(imp.into(), generic_args, env).map_err(|e| { MirEvalError::InFunction( Box::new(MirEvalError::MirLowerError(imp, e)), vec![(Either::Left(imp), span, locals.body.owner)], diff --git a/crates/hir-ty/src/mir/eval/tests.rs b/crates/hir-ty/src/mir/eval/tests.rs index 6552bf493377..54d71fa3eab4 100644 --- a/crates/hir-ty/src/mir/eval/tests.rs +++ b/crates/hir-ty/src/mir/eval/tests.rs @@ -833,3 +833,38 @@ fn main() { "#, ); } + +#[test] +fn regression_block_def_map() { + check_pass( + r#" +//- minicore: sized + +trait Write { + fn write(&mut self) { + trait SpecWriteFmt { + fn spec_write_fmt(self); + } + + impl SpecWriteFmt for &mut W { + #[inline] + default fn spec_write_fmt(self) {} + } + + impl SpecWriteFmt for &mut W { + #[inline] + fn spec_write_fmt(self) {} + } + self.spec_write_fmt() + } +} + +struct C; +impl Write for C {} + +fn main() { + C.write(); +} +"#, + ); +} diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 947fa3c21d6b..c6314a3b23f1 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -14,8 +14,8 @@ use hir_def::{ lang_item::{LangItem, LangItemTarget}, path::Path, resolver::{resolver_for_expr, HasResolver, ResolveValueResult, ValueNs}, - AdtId, DefWithBodyId, EnumVariantId, GeneralConstId, HasModule, ItemContainerId, LocalFieldId, - Lookup, TraitId, TupleId, TypeOrConstParamId, + AdtId, BlockId, DefWithBodyId, EnumVariantId, GeneralConstId, HasModule, ItemContainerId, + LocalFieldId, Lookup, TraitId, TupleId, TypeOrConstParamId, }; use hir_expand::name::Name; use la_arena::ArenaMap; @@ -51,10 +51,17 @@ struct LoopBlocks { drop_scope_index: usize, } -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] struct DropScope { /// locals, in order of definition (so we should run drop glues in reverse order) locals: Vec, + pop_trait_env_block: bool, +} + +impl DropScope { + fn new(pop_trait_env_block: bool) -> Self { + Self { locals: Default::default(), pop_trait_env_block } + } } struct MirLowerCtx<'a> { @@ -119,9 +126,9 @@ impl DropScopeToken { /// code. Either when the control flow is diverging (so drop code doesn't reached) or when drop is handled /// for us (for example a block that ended with a return statement. Return will drop everything, so the block shouldn't /// do anything) - fn pop_assume_dropped(self, ctx: &mut MirLowerCtx<'_>) { + fn pop_assume_dropped(self, ctx: &mut MirLowerCtx<'_>, current: BasicBlockId, span: MirSpan) { std::mem::forget(self); - ctx.pop_drop_scope_assume_dropped_internal(); + ctx.pop_drop_scope_assume_dropped_internal(current, span); } } @@ -267,7 +274,7 @@ impl<'ctx> MirLowerCtx<'ctx> { current_loop_blocks: None, labeled_loop_blocks: Default::default(), discr_temp: None, - drop_scopes: vec![DropScope::default()], + drop_scopes: vec![DropScope::new(false)], }; ctx } @@ -555,10 +562,10 @@ impl<'ctx> MirLowerCtx<'ctx> { } Ok(self.merge_blocks(Some(then_target), else_target, expr_id.into())) } - Expr::Unsafe { id: _, statements, tail } => { - self.lower_block_to_place(statements, current, *tail, place, expr_id.into()) + Expr::Unsafe { id, statements, tail } => { + self.lower_block_to_place(statements, *id, current, *tail, place, expr_id.into()) } - Expr::Block { id: _, statements, tail, label } => { + Expr::Block { id, statements, tail, label } => { if let Some(label) = label { self.lower_loop( current, @@ -568,6 +575,7 @@ impl<'ctx> MirLowerCtx<'ctx> { |this, begin| { if let Some(current) = this.lower_block_to_place( statements, + *id, begin, *tail, place, @@ -580,17 +588,24 @@ impl<'ctx> MirLowerCtx<'ctx> { }, ) } else { - self.lower_block_to_place(statements, current, *tail, place, expr_id.into()) + self.lower_block_to_place( + statements, + *id, + current, + *tail, + place, + expr_id.into(), + ) } } Expr::Loop { body, label } => { self.lower_loop(current, place, *label, expr_id.into(), |this, begin| { - let scope = this.push_drop_scope(); + let scope = this.push_drop_scope(false); if let Some((_, mut current)) = this.lower_expr_as_place(begin, *body, true)? { current = scope.pop_and_drop(this, current, body.into()); this.set_goto(current, begin, expr_id.into()); } else { - scope.pop_assume_dropped(this); + scope.pop_assume_dropped(this, current, body.into()); } Ok(()) }) @@ -1590,6 +1605,14 @@ impl<'ctx> MirLowerCtx<'ctx> { self.result.basic_blocks[block].statements.push(statement); } + fn push_enter_trait_block(&mut self, block: BasicBlockId, id: hir_def::BlockId, span: MirSpan) { + self.push_statement(block, StatementKind::TraitEnvBlockEnter(id).with_span(span)); + } + + fn push_exit_trait_block(&mut self, block: BasicBlockId, span: MirSpan) { + self.push_statement(block, StatementKind::TraitEnvBlockExit.with_span(span)); + } + fn push_fake_read(&mut self, block: BasicBlockId, p: Place, span: MirSpan) { self.push_statement(block, StatementKind::FakeRead(p).with_span(span)); } @@ -1740,12 +1763,16 @@ impl<'ctx> MirLowerCtx<'ctx> { fn lower_block_to_place( &mut self, statements: &[hir_def::hir::Statement], + id: Option, mut current: BasicBlockId, tail: Option, place: Place, span: MirSpan, ) -> Result>> { - let scope = self.push_drop_scope(); + let scope = self.push_drop_scope(id.is_some()); + if let Some(id) = id { + self.push_enter_trait_block(current, id, span); + } for statement in statements.iter() { match statement { hir_def::hir::Statement::Let { pat, initializer, else_branch, type_ref: _ } => { @@ -1754,7 +1781,7 @@ impl<'ctx> MirLowerCtx<'ctx> { let Some((init_place, c)) = self.lower_expr_as_place(current, *expr_id, true)? else { - scope.pop_assume_dropped(self); + scope.pop_assume_dropped(self, current, span); return Ok(None); }; current = c; @@ -1787,10 +1814,10 @@ impl<'ctx> MirLowerCtx<'ctx> { } } &hir_def::hir::Statement::Expr { expr, has_semi: _ } => { - let scope2 = self.push_drop_scope(); + let scope2 = self.push_drop_scope(false); let Some((p, c)) = self.lower_expr_as_place(current, expr, true)? else { - scope2.pop_assume_dropped(self); - scope.pop_assume_dropped(self); + scope2.pop_assume_dropped(self, current, span); + scope.pop_assume_dropped(self, current, span); return Ok(None); }; self.push_fake_read(c, p, expr.into()); @@ -1800,7 +1827,7 @@ impl<'ctx> MirLowerCtx<'ctx> { } if let Some(tail) = tail { let Some(c) = self.lower_expr_to_place(tail, place, current)? else { - scope.pop_assume_dropped(self); + scope.pop_assume_dropped(self, current, span); return Ok(None); }; current = c; @@ -1897,14 +1924,18 @@ impl<'ctx> MirLowerCtx<'ctx> { current } - fn push_drop_scope(&mut self) -> DropScopeToken { - self.drop_scopes.push(DropScope::default()); + fn push_drop_scope(&mut self, pop_trait_env_block: bool) -> DropScopeToken { + self.drop_scopes.push(DropScope::new(pop_trait_env_block)); DropScopeToken } /// Don't call directly - fn pop_drop_scope_assume_dropped_internal(&mut self) { - self.drop_scopes.pop(); + fn pop_drop_scope_assume_dropped_internal(&mut self, current: BasicBlockId, span: MirSpan) { + if let Some(scope) = self.drop_scopes.pop() { + if scope.pop_trait_env_block { + self.push_exit_trait_block(current, span); + } + } } /// Don't call directly @@ -1915,6 +1946,9 @@ impl<'ctx> MirLowerCtx<'ctx> { ) -> BasicBlockId { let scope = self.drop_scopes.pop().unwrap(); self.emit_drop_and_storage_dead_for_scope(&scope, &mut current, span); + if scope.pop_trait_env_block { + self.push_exit_trait_block(current, span); + } current } diff --git a/crates/hir-ty/src/mir/monomorphization.rs b/crates/hir-ty/src/mir/monomorphization.rs index 8da03eef2e0c..91f4491971d5 100644 --- a/crates/hir-ty/src/mir/monomorphization.rs +++ b/crates/hir-ty/src/mir/monomorphization.rs @@ -252,7 +252,9 @@ impl Filler<'_> { | StatementKind::FakeRead(_) | StatementKind::StorageLive(_) | StatementKind::StorageDead(_) - | StatementKind::Nop => (), + | StatementKind::Nop + | StatementKind::TraitEnvBlockEnter(_) + | StatementKind::TraitEnvBlockExit => (), } } if let Some(terminator) = &mut bb.terminator { diff --git a/crates/hir-ty/src/mir/pretty.rs b/crates/hir-ty/src/mir/pretty.rs index 366c2f662b54..caaaa37821f6 100644 --- a/crates/hir-ty/src/mir/pretty.rs +++ b/crates/hir-ty/src/mir/pretty.rs @@ -240,6 +240,10 @@ impl<'a> MirPrettyCtx<'a> { wln!(this, ");"); } StatementKind::Nop => wln!(this, "Nop;"), + StatementKind::TraitEnvBlockEnter(block) => { + w!(this, "EnterItemBlock({block:?});") + } + StatementKind::TraitEnvBlockExit => wln!(this, "ExitItemBlock;"), } } match &block.terminator {