From 83da9a89d8316c76dd6262c6aa10b74e549308bd Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 9 Mar 2023 19:53:59 +0000 Subject: [PATCH 1/2] Directly construct Inherited. --- compiler/rustc_hir_typeck/src/inherited.rs | 39 +-- compiler/rustc_hir_typeck/src/lib.rs | 223 +++++++++--------- .../src/methods/unnecessary_to_owned.rs | 8 +- .../clippy_lints/src/transmute/utils.rs | 57 +++-- 4 files changed, 151 insertions(+), 176 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/inherited.rs b/compiler/rustc_hir_typeck/src/inherited.rs index 07fa7c55df660..4110b176b41b1 100644 --- a/compiler/rustc_hir_typeck/src/inherited.rs +++ b/compiler/rustc_hir_typeck/src/inherited.rs @@ -4,7 +4,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; use rustc_hir::HirIdMap; -use rustc_infer::infer; use rustc_infer::infer::{DefiningAnchor, InferCtxt, InferOk, TyCtxtInferExt}; use rustc_middle::ty::visit::TypeVisitableExt; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -73,40 +72,16 @@ impl<'tcx> Deref for Inherited<'tcx> { } } -/// A temporary returned by `Inherited::build(...)`. This is necessary -/// for multiple `InferCtxt` to share the same `typeck_results` -/// without using `Rc` or something similar. -pub struct InheritedBuilder<'tcx> { - infcx: infer::InferCtxtBuilder<'tcx>, - typeck_results: RefCell>, -} - impl<'tcx> Inherited<'tcx> { - pub fn build(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> InheritedBuilder<'tcx> { + pub fn new(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Self { let hir_owner = tcx.hir().local_def_id_to_hir_id(def_id).owner; - InheritedBuilder { - infcx: tcx - .infer_ctxt() - .ignoring_regions() - .with_opaque_type_inference(DefiningAnchor::Bind(hir_owner.def_id)), - typeck_results: RefCell::new(ty::TypeckResults::new(hir_owner)), - } - } -} - -impl<'tcx> InheritedBuilder<'tcx> { - pub fn enter(mut self, f: F) -> R - where - F: FnOnce(&Inherited<'tcx>) -> R, - { - f(&Inherited::new(self.infcx.build(), self.typeck_results)) - } -} - -impl<'tcx> Inherited<'tcx> { - fn new(infcx: InferCtxt<'tcx>, typeck_results: RefCell>) -> Self { - let tcx = infcx.tcx; + let infcx = tcx + .infer_ctxt() + .ignoring_regions() + .with_opaque_type_inference(DefiningAnchor::Bind(hir_owner.def_id)) + .build(); + let typeck_results = RefCell::new(ty::TypeckResults::new(hir_owner)); Inherited { typeck_results, diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index e397dfd45706b..1846ab4baf38d 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -45,13 +45,14 @@ mod rvalue_scopes; mod upvar; mod writeback; -pub use diverges::Diverges; -pub use expectation::Expectation; -pub use fn_ctxt::*; -pub use inherited::{Inherited, InheritedBuilder}; +pub use fn_ctxt::FnCtxt; +pub use inherited::Inherited; use crate::check::check_fn; use crate::coercion::DynamicCoerceMany; +use crate::diverges::Diverges; +use crate::expectation::Expectation; +use crate::fn_ctxt::RawTy; use crate::gather_locals::GatherLocalsVisitor; use rustc_data_structures::unord::UnordSet; use rustc_errors::{ @@ -206,135 +207,135 @@ fn typeck_with_fallback<'tcx>( }); let body = tcx.hir().body(body_id); - let typeck_results = Inherited::build(tcx, def_id).enter(|inh| { - let param_env = tcx.param_env(def_id); - let param_env = if tcx.has_attr(def_id.to_def_id(), sym::rustc_do_not_const_check) { - param_env.without_const() + let param_env = tcx.param_env(def_id); + let param_env = if tcx.has_attr(def_id.to_def_id(), sym::rustc_do_not_const_check) { + param_env.without_const() + } else { + param_env + }; + let inh = Inherited::new(tcx, def_id); + let mut fcx = FnCtxt::new(&inh, param_env, def_id); + + if let Some(hir::FnSig { header, decl, .. }) = fn_sig { + let fn_sig = if rustc_hir_analysis::collect::get_infer_ret_ty(&decl.output).is_some() { + fcx.astconv().ty_of_fn(id, header.unsafety, header.abi, decl, None, None) } else { - param_env + tcx.fn_sig(def_id).subst_identity() }; - let mut fcx = FnCtxt::new(&inh, param_env, def_id); - if let Some(hir::FnSig { header, decl, .. }) = fn_sig { - let fn_sig = if rustc_hir_analysis::collect::get_infer_ret_ty(&decl.output).is_some() { - fcx.astconv().ty_of_fn(id, header.unsafety, header.abi, decl, None, None) - } else { - tcx.fn_sig(def_id).subst_identity() - }; + check_abi(tcx, id, span, fn_sig.abi()); - check_abi(tcx, id, span, fn_sig.abi()); + // Compute the function signature from point of view of inside the fn. + let fn_sig = tcx.liberate_late_bound_regions(def_id.to_def_id(), fn_sig); + let fn_sig = fcx.normalize(body.value.span, fn_sig); - // Compute the function signature from point of view of inside the fn. - let fn_sig = tcx.liberate_late_bound_regions(def_id.to_def_id(), fn_sig); - let fn_sig = fcx.normalize(body.value.span, fn_sig); - - check_fn(&mut fcx, fn_sig, decl, def_id, body, None); - } else { - let expected_type = body_ty - .and_then(|ty| match ty.kind { - hir::TyKind::Infer => Some(fcx.astconv().ast_ty_to_ty(ty)), - _ => None, - }) - .unwrap_or_else(|| match tcx.hir().get(id) { - Node::AnonConst(_) => match tcx.hir().get(tcx.hir().parent_id(id)) { - Node::Expr(&hir::Expr { - kind: hir::ExprKind::ConstBlock(ref anon_const), - .. - }) if anon_const.hir_id == id => fcx.next_ty_var(TypeVariableOrigin { - kind: TypeVariableOriginKind::TypeInference, - span, - }), - Node::Ty(&hir::Ty { - kind: hir::TyKind::Typeof(ref anon_const), .. - }) if anon_const.hir_id == id => fcx.next_ty_var(TypeVariableOrigin { + check_fn(&mut fcx, fn_sig, decl, def_id, body, None); + } else { + let expected_type = body_ty + .and_then(|ty| match ty.kind { + hir::TyKind::Infer => Some(fcx.astconv().ast_ty_to_ty(ty)), + _ => None, + }) + .unwrap_or_else(|| match tcx.hir().get(id) { + Node::AnonConst(_) => match tcx.hir().get(tcx.hir().parent_id(id)) { + Node::Expr(&hir::Expr { + kind: hir::ExprKind::ConstBlock(ref anon_const), + .. + }) if anon_const.hir_id == id => fcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::TypeInference, + span, + }), + Node::Ty(&hir::Ty { kind: hir::TyKind::Typeof(ref anon_const), .. }) + if anon_const.hir_id == id => + { + fcx.next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span, - }), - Node::Expr(&hir::Expr { kind: hir::ExprKind::InlineAsm(asm), .. }) - | Node::Item(&hir::Item { kind: hir::ItemKind::GlobalAsm(asm), .. }) => { - let operand_ty = - asm.operands.iter().find_map(|(op, _op_sp)| match op { - hir::InlineAsmOperand::Const { anon_const } - if anon_const.hir_id == id => - { - // Inline assembly constants must be integers. - Some(fcx.next_int_var()) - } - hir::InlineAsmOperand::SymFn { anon_const } - if anon_const.hir_id == id => - { - Some(fcx.next_ty_var(TypeVariableOrigin { - kind: TypeVariableOriginKind::MiscVariable, - span, - })) - } - _ => None, - }); - operand_ty.unwrap_or_else(fallback) - } - _ => fallback(), - }, + }) + } + Node::Expr(&hir::Expr { kind: hir::ExprKind::InlineAsm(asm), .. }) + | Node::Item(&hir::Item { kind: hir::ItemKind::GlobalAsm(asm), .. }) => { + let operand_ty = asm.operands.iter().find_map(|(op, _op_sp)| match op { + hir::InlineAsmOperand::Const { anon_const } + if anon_const.hir_id == id => + { + // Inline assembly constants must be integers. + Some(fcx.next_int_var()) + } + hir::InlineAsmOperand::SymFn { anon_const } + if anon_const.hir_id == id => + { + Some(fcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::MiscVariable, + span, + })) + } + _ => None, + }); + operand_ty.unwrap_or_else(fallback) + } _ => fallback(), - }); + }, + _ => fallback(), + }); - let expected_type = fcx.normalize(body.value.span, expected_type); - fcx.require_type_is_sized(expected_type, body.value.span, traits::ConstSized); + let expected_type = fcx.normalize(body.value.span, expected_type); + fcx.require_type_is_sized(expected_type, body.value.span, traits::ConstSized); - // Gather locals in statics (because of block expressions). - GatherLocalsVisitor::new(&fcx).visit_body(body); + // Gather locals in statics (because of block expressions). + GatherLocalsVisitor::new(&fcx).visit_body(body); - fcx.check_expr_coercable_to_type(&body.value, expected_type, None); + fcx.check_expr_coercable_to_type(&body.value, expected_type, None); - fcx.write_ty(id, expected_type); - }; + fcx.write_ty(id, expected_type); + }; - fcx.type_inference_fallback(); - - // Even though coercion casts provide type hints, we check casts after fallback for - // backwards compatibility. This makes fallback a stronger type hint than a cast coercion. - fcx.check_casts(); - fcx.select_obligations_where_possible(|_| {}); - - // Closure and generator analysis may run after fallback - // because they don't constrain other type variables. - // Closure analysis only runs on closures. Therefore they only need to fulfill non-const predicates (as of now) - let prev_constness = fcx.param_env.constness(); - fcx.param_env = fcx.param_env.without_const(); - fcx.closure_analyze(body); - fcx.param_env = fcx.param_env.with_constness(prev_constness); - assert!(fcx.deferred_call_resolutions.borrow().is_empty()); - // Before the generator analysis, temporary scopes shall be marked to provide more - // precise information on types to be captured. - fcx.resolve_rvalue_scopes(def_id.to_def_id()); - - for (ty, span, code) in fcx.deferred_sized_obligations.borrow_mut().drain(..) { - let ty = fcx.normalize(span, ty); - fcx.require_type_is_sized(ty, span, code); - } + fcx.type_inference_fallback(); + + // Even though coercion casts provide type hints, we check casts after fallback for + // backwards compatibility. This makes fallback a stronger type hint than a cast coercion. + fcx.check_casts(); + fcx.select_obligations_where_possible(|_| {}); + + // Closure and generator analysis may run after fallback + // because they don't constrain other type variables. + // Closure analysis only runs on closures. Therefore they only need to fulfill non-const predicates (as of now) + let prev_constness = fcx.param_env.constness(); + fcx.param_env = fcx.param_env.without_const(); + fcx.closure_analyze(body); + fcx.param_env = fcx.param_env.with_constness(prev_constness); + assert!(fcx.deferred_call_resolutions.borrow().is_empty()); + // Before the generator analysis, temporary scopes shall be marked to provide more + // precise information on types to be captured. + fcx.resolve_rvalue_scopes(def_id.to_def_id()); + + for (ty, span, code) in fcx.deferred_sized_obligations.borrow_mut().drain(..) { + let ty = fcx.normalize(span, ty); + fcx.require_type_is_sized(ty, span, code); + } - fcx.select_obligations_where_possible(|_| {}); + fcx.select_obligations_where_possible(|_| {}); - debug!(pending_obligations = ?fcx.fulfillment_cx.borrow().pending_obligations()); + debug!(pending_obligations = ?fcx.fulfillment_cx.borrow().pending_obligations()); - // This must be the last thing before `report_ambiguity_errors`. - fcx.resolve_generator_interiors(def_id.to_def_id()); + // This must be the last thing before `report_ambiguity_errors`. + fcx.resolve_generator_interiors(def_id.to_def_id()); - debug!(pending_obligations = ?fcx.fulfillment_cx.borrow().pending_obligations()); + debug!(pending_obligations = ?fcx.fulfillment_cx.borrow().pending_obligations()); - if let None = fcx.infcx.tainted_by_errors() { - fcx.report_ambiguity_errors(); - } + if let None = fcx.infcx.tainted_by_errors() { + fcx.report_ambiguity_errors(); + } - if let None = fcx.infcx.tainted_by_errors() { - fcx.check_transmutes(); - } + if let None = fcx.infcx.tainted_by_errors() { + fcx.check_transmutes(); + } - fcx.check_asms(); + fcx.check_asms(); - fcx.infcx.skip_region_resolution(); + fcx.infcx.skip_region_resolution(); - fcx.resolve_type_vars_in_body(body) - }); + let typeck_results = fcx.resolve_type_vars_in_body(body); // Consistency check our TypeckResults instance can hold all ItemLocalIds // it will need to hold. diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs index df26b36b7b32a..4c4c003ca4691 100644 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -369,10 +369,10 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty< Node::Item(item) => { if let ItemKind::Fn(_, _, body_id) = &item.kind && let output_ty = return_ty(cx, item.owner_id) - && Inherited::build(cx.tcx, item.owner_id.def_id).enter(|inherited| { - let fn_ctxt = FnCtxt::new(inherited, cx.param_env, item.owner_id.def_id); - fn_ctxt.can_coerce(ty, output_ty) - }) { + && let inherited = Inherited::new(cx.tcx, item.owner_id.def_id) + && let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, item.owner_id.def_id) + && fn_ctxt.can_coerce(ty, output_ty) + { if has_lifetime(output_ty) && has_lifetime(ty) { return false; } diff --git a/src/tools/clippy/clippy_lints/src/transmute/utils.rs b/src/tools/clippy/clippy_lints/src/transmute/utils.rs index cddaf9450eabc..62efd13b8d909 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/utils.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/utils.rs @@ -33,38 +33,37 @@ pub(super) fn check_cast<'tcx>( let hir_id = e.hir_id; let local_def_id = hir_id.owner.def_id; - Inherited::build(cx.tcx, local_def_id).enter(|inherited| { - let fn_ctxt = FnCtxt::new(inherited, cx.param_env, local_def_id); + let inherited = Inherited::new(cx.tcx, local_def_id); + let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, local_def_id); - // If we already have errors, we can't be sure we can pointer cast. + // If we already have errors, we can't be sure we can pointer cast. + assert!( + !fn_ctxt.errors_reported_since_creation(), + "Newly created FnCtxt contained errors" + ); + + if let Ok(check) = cast::CastCheck::new( + &fn_ctxt, + e, + from_ty, + to_ty, + // We won't show any error to the user, so we don't care what the span is here. + DUMMY_SP, + DUMMY_SP, + hir::Constness::NotConst, + ) { + let res = check.do_check(&fn_ctxt); + + // do_check's documentation says that it might return Ok and create + // errors in the fcx instead of returning Err in some cases. Those cases + // should be filtered out before getting here. assert!( !fn_ctxt.errors_reported_since_creation(), - "Newly created FnCtxt contained errors" + "`fn_ctxt` contained errors after cast check!" ); - if let Ok(check) = cast::CastCheck::new( - &fn_ctxt, - e, - from_ty, - to_ty, - // We won't show any error to the user, so we don't care what the span is here. - DUMMY_SP, - DUMMY_SP, - hir::Constness::NotConst, - ) { - let res = check.do_check(&fn_ctxt); - - // do_check's documentation says that it might return Ok and create - // errors in the fcx instead of returning Err in some cases. Those cases - // should be filtered out before getting here. - assert!( - !fn_ctxt.errors_reported_since_creation(), - "`fn_ctxt` contained errors after cast check!" - ); - - res.ok() - } else { - None - } - }) + res.ok() + } else { + None + } } From 391ef47d8a8322fab1aede78bed4b5e7493ff15e Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 9 Mar 2023 20:15:10 +0000 Subject: [PATCH 2/2] Simplify typeck entry. --- compiler/rustc_hir_typeck/src/lib.rs | 97 +++++++++++++--------------- 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index 1846ab4baf38d..70124a7736406 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -106,10 +106,9 @@ pub struct LocalTy<'tcx> { /// (notably closures), `typeck_results(def_id)` would wind up /// redirecting to the owning function. fn primary_body_of( - tcx: TyCtxt<'_>, - id: hir::HirId, + node: Node<'_>, ) -> Option<(hir::BodyId, Option<&hir::Ty<'_>>, Option<&hir::FnSig<'_>>)> { - match tcx.hir().get(id) { + match node { Node::Item(item) => match item.kind { hir::ItemKind::Const(ty, body) | hir::ItemKind::Static(ty, _, body) => { Some((body, Some(ty), None)) @@ -143,8 +142,7 @@ fn has_typeck_results(tcx: TyCtxt<'_>, def_id: DefId) -> bool { } if let Some(def_id) = def_id.as_local() { - let id = tcx.hir().local_def_id_to_hir_id(def_id); - primary_body_of(tcx, id).is_some() + primary_body_of(tcx.hir().get_by_def_id(def_id)).is_some() } else { false } @@ -199,10 +197,11 @@ fn typeck_with_fallback<'tcx>( } let id = tcx.hir().local_def_id_to_hir_id(def_id); + let node = tcx.hir().get(id); let span = tcx.hir().span(id); // Figure out what primary body this item has. - let (body_id, body_ty, fn_sig) = primary_body_of(tcx, id).unwrap_or_else(|| { + let (body_id, body_ty, fn_sig) = primary_body_of(node).unwrap_or_else(|| { span_bug!(span, "can't type-check body of {:?}", def_id); }); let body = tcx.hir().body(body_id); @@ -231,53 +230,49 @@ fn typeck_with_fallback<'tcx>( check_fn(&mut fcx, fn_sig, decl, def_id, body, None); } else { - let expected_type = body_ty - .and_then(|ty| match ty.kind { - hir::TyKind::Infer => Some(fcx.astconv().ast_ty_to_ty(ty)), - _ => None, - }) - .unwrap_or_else(|| match tcx.hir().get(id) { - Node::AnonConst(_) => match tcx.hir().get(tcx.hir().parent_id(id)) { - Node::Expr(&hir::Expr { - kind: hir::ExprKind::ConstBlock(ref anon_const), - .. - }) if anon_const.hir_id == id => fcx.next_ty_var(TypeVariableOrigin { + let expected_type = if let Some(&hir::Ty { kind: hir::TyKind::Infer, span, .. }) = body_ty { + Some(fcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::TypeInference, + span, + })) + } else if let Node::AnonConst(_) = node { + match tcx.hir().get(tcx.hir().parent_id(id)) { + Node::Expr(&hir::Expr { + kind: hir::ExprKind::ConstBlock(ref anon_const), .. + }) if anon_const.hir_id == id => Some(fcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::TypeInference, + span, + })), + Node::Ty(&hir::Ty { kind: hir::TyKind::Typeof(ref anon_const), .. }) + if anon_const.hir_id == id => + { + Some(fcx.next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span, - }), - Node::Ty(&hir::Ty { kind: hir::TyKind::Typeof(ref anon_const), .. }) - if anon_const.hir_id == id => - { - fcx.next_ty_var(TypeVariableOrigin { - kind: TypeVariableOriginKind::TypeInference, - span, - }) - } - Node::Expr(&hir::Expr { kind: hir::ExprKind::InlineAsm(asm), .. }) - | Node::Item(&hir::Item { kind: hir::ItemKind::GlobalAsm(asm), .. }) => { - let operand_ty = asm.operands.iter().find_map(|(op, _op_sp)| match op { - hir::InlineAsmOperand::Const { anon_const } - if anon_const.hir_id == id => - { - // Inline assembly constants must be integers. - Some(fcx.next_int_var()) - } - hir::InlineAsmOperand::SymFn { anon_const } - if anon_const.hir_id == id => - { - Some(fcx.next_ty_var(TypeVariableOrigin { - kind: TypeVariableOriginKind::MiscVariable, - span, - })) - } - _ => None, - }); - operand_ty.unwrap_or_else(fallback) - } - _ => fallback(), - }, - _ => fallback(), - }); + })) + } + Node::Expr(&hir::Expr { kind: hir::ExprKind::InlineAsm(asm), .. }) + | Node::Item(&hir::Item { kind: hir::ItemKind::GlobalAsm(asm), .. }) => { + asm.operands.iter().find_map(|(op, _op_sp)| match op { + hir::InlineAsmOperand::Const { anon_const } if anon_const.hir_id == id => { + // Inline assembly constants must be integers. + Some(fcx.next_int_var()) + } + hir::InlineAsmOperand::SymFn { anon_const } if anon_const.hir_id == id => { + Some(fcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::MiscVariable, + span, + })) + } + _ => None, + }) + } + _ => None, + } + } else { + None + }; + let expected_type = expected_type.unwrap_or_else(fallback); let expected_type = fcx.normalize(body.value.span, expected_type); fcx.require_type_is_sized(expected_type, body.value.span, traits::ConstSized);