From 4b2d9e60f41c11238d60ee9ca319136f61e72ca1 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 26 Jun 2020 06:34:43 +0300 Subject: [PATCH] Remove `TypeckTables::empty(None)` and make hir_owner non-optional. --- .../infer/error_reporting/mod.rs | 2 +- src/librustc_infer/infer/mod.rs | 2 +- src/librustc_middle/ty/context.rs | 69 ++++------ .../traits/error_reporting/suggestions.rs | 2 +- src/librustc_typeck/check/method/suggest.rs | 127 +++++++++--------- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/check/writeback.rs | 15 +-- 7 files changed, 98 insertions(+), 121 deletions(-) diff --git a/src/librustc_infer/infer/error_reporting/mod.rs b/src/librustc_infer/infer/error_reporting/mod.rs index 7fdcbd31df3c5..b1ea222370742 100644 --- a/src/librustc_infer/infer/error_reporting/mod.rs +++ b/src/librustc_infer/infer/error_reporting/mod.rs @@ -1684,7 +1684,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // Attempt to obtain the span of the parameter so we can // suggest adding an explicit lifetime bound to it. let generics = - self.in_progress_tables.and_then(|table| table.borrow().hir_owner).map(|table_owner| { + self.in_progress_tables.map(|table| table.borrow().hir_owner).map(|table_owner| { let hir_id = hir.as_local_hir_id(table_owner); let parent_id = hir.get_parent_item(hir_id); ( diff --git a/src/librustc_infer/infer/mod.rs b/src/librustc_infer/infer/mod.rs index 76ac61c067280..27da514a17f4d 100644 --- a/src/librustc_infer/infer/mod.rs +++ b/src/librustc_infer/infer/mod.rs @@ -588,7 +588,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> { /// Used only by `rustc_typeck` during body type-checking/inference, /// will initialize `in_progress_tables` with fresh `TypeckTables`. pub fn with_fresh_in_progress_tables(mut self, table_owner: LocalDefId) -> Self { - self.fresh_tables = Some(RefCell::new(ty::TypeckTables::empty(Some(table_owner)))); + self.fresh_tables = Some(RefCell::new(ty::TypeckTables::new(table_owner))); self } diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index e2f601371b1ee..10bb788c9e9a6 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -188,7 +188,7 @@ pub struct CommonConsts<'tcx> { } pub struct LocalTableInContext<'a, V> { - hir_owner: Option, + hir_owner: LocalDefId, data: &'a ItemLocalMap, } @@ -199,42 +199,27 @@ pub struct LocalTableInContext<'a, V> { /// would be in a different frame of reference and using its `local_id` /// would result in lookup errors, or worse, in silently wrong data being /// stored/returned. -fn validate_hir_id_for_typeck_tables( - hir_owner: Option, - hir_id: hir::HirId, - mut_access: bool, -) { - if let Some(hir_owner) = hir_owner { - if hir_id.owner != hir_owner { - ty::tls::with(|tcx| { - bug!( - "node {} with HirId::owner {:?} cannot be placed in TypeckTables with hir_owner {:?}", - tcx.hir().node_to_string(hir_id), - hir_id.owner, - hir_owner - ) - }); - } - } else { - // We use "Null Object" TypeckTables in some of the analysis passes. - // These are just expected to be empty and their `hir_owner` is - // `None`. Therefore we cannot verify whether a given `HirId` would - // be a valid key for the given table. Instead we make sure that - // nobody tries to write to such a Null Object table. - if mut_access { - bug!("access to invalid TypeckTables") - } +fn validate_hir_id_for_typeck_tables(hir_owner: LocalDefId, hir_id: hir::HirId) { + if hir_id.owner != hir_owner { + ty::tls::with(|tcx| { + bug!( + "node {} with HirId::owner {:?} cannot be placed in TypeckTables with hir_owner {:?}", + tcx.hir().node_to_string(hir_id), + hir_id.owner, + hir_owner + ) + }); } } impl<'a, V> LocalTableInContext<'a, V> { pub fn contains_key(&self, id: hir::HirId) -> bool { - validate_hir_id_for_typeck_tables(self.hir_owner, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.data.contains_key(&id.local_id) } pub fn get(&self, id: hir::HirId) -> Option<&V> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.data.get(&id.local_id) } @@ -252,28 +237,28 @@ impl<'a, V> ::std::ops::Index for LocalTableInContext<'a, V> { } pub struct LocalTableInContextMut<'a, V> { - hir_owner: Option, + hir_owner: LocalDefId, data: &'a mut ItemLocalMap, } impl<'a, V> LocalTableInContextMut<'a, V> { pub fn get_mut(&mut self, id: hir::HirId) -> Option<&mut V> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.data.get_mut(&id.local_id) } pub fn entry(&mut self, id: hir::HirId) -> Entry<'_, hir::ItemLocalId, V> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.data.entry(id.local_id) } pub fn insert(&mut self, id: hir::HirId, val: V) -> Option { - validate_hir_id_for_typeck_tables(self.hir_owner, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.data.insert(id.local_id, val) } pub fn remove(&mut self, id: hir::HirId) -> Option { - validate_hir_id_for_typeck_tables(self.hir_owner, id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.data.remove(&id.local_id) } } @@ -324,7 +309,7 @@ pub struct GeneratorInteriorTypeCause<'tcx> { #[derive(RustcEncodable, RustcDecodable, Debug)] pub struct TypeckTables<'tcx> { /// The `HirId::owner` all `ItemLocalId`s in this table are relative to. - pub hir_owner: Option, + pub hir_owner: LocalDefId, /// Resolved definitions for `::X` associated paths and /// method calls, including those of overloaded operators. @@ -432,7 +417,7 @@ pub struct TypeckTables<'tcx> { } impl<'tcx> TypeckTables<'tcx> { - pub fn empty(hir_owner: Option) -> TypeckTables<'tcx> { + pub fn new(hir_owner: LocalDefId) -> TypeckTables<'tcx> { TypeckTables { hir_owner, type_dependent_defs: Default::default(), @@ -474,7 +459,7 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn type_dependent_def(&self, id: HirId) -> Option<(DefKind, DefId)> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.type_dependent_defs.get(&id.local_id).cloned().and_then(|r| r.ok()) } @@ -521,7 +506,7 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn node_type_opt(&self, id: hir::HirId) -> Option> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.node_types.get(&id.local_id).cloned() } @@ -530,12 +515,12 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn node_substs(&self, id: hir::HirId) -> SubstsRef<'tcx> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.node_substs.get(&id.local_id).cloned().unwrap_or_else(|| InternalSubsts::empty()) } pub fn node_substs_opt(&self, id: hir::HirId) -> Option> { - validate_hir_id_for_typeck_tables(self.hir_owner, id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, id); self.node_substs.get(&id.local_id).cloned() } @@ -578,7 +563,7 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn expr_adjustments(&self, expr: &hir::Expr<'_>) -> &[ty::adjustment::Adjustment<'tcx>] { - validate_hir_id_for_typeck_tables(self.hir_owner, expr.hir_id, false); + validate_hir_id_for_typeck_tables(self.hir_owner, expr.hir_id); self.adjustments.get(&expr.hir_id.local_id).map_or(&[], |a| &a[..]) } @@ -657,7 +642,7 @@ impl<'tcx> TypeckTables<'tcx> { } pub fn is_coercion_cast(&self, hir_id: hir::HirId) -> bool { - validate_hir_id_for_typeck_tables(self.hir_owner, hir_id, true); + validate_hir_id_for_typeck_tables(self.hir_owner, hir_id); self.coercion_casts.contains(&hir_id.local_id) } @@ -710,7 +695,7 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { hash_stable_hashmap(hcx, hasher, upvar_capture_map, |up_var_id, hcx| { let ty::UpvarId { var_path, closure_expr_id } = *up_var_id; - assert_eq!(Some(var_path.hir_id.owner), hir_owner); + assert_eq!(var_path.hir_id.owner, hir_owner); ( hcx.local_def_path_hash(var_path.hir_id.owner), diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 1fafa9ec035ce..cdfe5f9f92db0 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1407,7 +1407,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); let query_tables; let tables: &TypeckTables<'tcx> = match &in_progress_tables { - Some(t) if t.hir_owner.map(|owner| owner.to_def_id()) == Some(generator_did_root) => t, + Some(t) if t.hir_owner.to_def_id() == generator_did_root => t, _ => { query_tables = self.tcx.typeck_tables_of(generator_did.expect_local()); &query_tables diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index b75dac52b93e5..fbf81e94d03d5 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -1039,22 +1039,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut suggested = false; if let (Some(ref param), Some(ref table)) = (param_type, self.in_progress_tables) { let table_owner = table.borrow().hir_owner; - if let Some(table_owner) = table_owner { - let generics = self.tcx.generics_of(table_owner.to_def_id()); - let type_param = generics.type_param(param, self.tcx); - let hir = &self.tcx.hir(); - if let Some(def_id) = type_param.def_id.as_local() { - let id = hir.as_local_hir_id(def_id); - // Get the `hir::Param` to verify whether it already has any bounds. - // We do this to avoid suggesting code that ends up as `T: FooBar`, - // instead we suggest `T: Foo + Bar` in that case. - match hir.get(id) { - Node::GenericParam(ref param) => { - let mut impl_trait = false; - let has_bounds = if let hir::GenericParamKind::Type { - synthetic: Some(_), - .. - } = ¶m.kind + let generics = self.tcx.generics_of(table_owner.to_def_id()); + let type_param = generics.type_param(param, self.tcx); + let hir = &self.tcx.hir(); + if let Some(def_id) = type_param.def_id.as_local() { + let id = hir.as_local_hir_id(def_id); + // Get the `hir::Param` to verify whether it already has any bounds. + // We do this to avoid suggesting code that ends up as `T: FooBar`, + // instead we suggest `T: Foo + Bar` in that case. + match hir.get(id) { + Node::GenericParam(ref param) => { + let mut impl_trait = false; + let has_bounds = + if let hir::GenericParamKind::Type { synthetic: Some(_), .. } = + ¶m.kind { // We've found `fn foo(x: impl Trait)` instead of // `fn foo(x: T)`. We want to suggest the correct @@ -1065,64 +1063,63 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else { param.bounds.get(0) }; - let sp = hir.span(id); - let sp = if let Some(first_bound) = has_bounds { - // `sp` only covers `T`, change it so that it covers - // `T:` when appropriate - sp.until(first_bound.span()) - } else { - sp - }; - let trait_def_ids: FxHashSet = param - .bounds - .iter() - .filter_map(|bound| Some(bound.trait_ref()?.trait_def_id()?)) - .collect(); - if !candidates.iter().any(|t| trait_def_ids.contains(&t.def_id)) { - err.span_suggestions( - sp, - &message(format!( - "restrict type parameter `{}` with", - param.name.ident(), - )), - candidates.iter().map(|t| { - format!( - "{}{} {}{}", - param.name.ident(), - if impl_trait { " +" } else { ":" }, - self.tcx.def_path_str(t.def_id), - if has_bounds.is_some() { " + " } else { "" }, - ) - }), - Applicability::MaybeIncorrect, - ); - } - suggested = true; - } - Node::Item(hir::Item { - kind: hir::ItemKind::Trait(.., bounds, _), - ident, - .. - }) => { - let (sp, sep, article) = if bounds.is_empty() { - (ident.span.shrink_to_hi(), ":", "a") - } else { - (bounds.last().unwrap().span().shrink_to_hi(), " +", "another") - }; + let sp = hir.span(id); + let sp = if let Some(first_bound) = has_bounds { + // `sp` only covers `T`, change it so that it covers + // `T:` when appropriate + sp.until(first_bound.span()) + } else { + sp + }; + let trait_def_ids: FxHashSet = param + .bounds + .iter() + .filter_map(|bound| Some(bound.trait_ref()?.trait_def_id()?)) + .collect(); + if !candidates.iter().any(|t| trait_def_ids.contains(&t.def_id)) { err.span_suggestions( sp, - &message(format!("add {} supertrait for", article)), + &message(format!( + "restrict type parameter `{}` with", + param.name.ident(), + )), candidates.iter().map(|t| { - format!("{} {}", sep, self.tcx.def_path_str(t.def_id),) + format!( + "{}{} {}{}", + param.name.ident(), + if impl_trait { " +" } else { ":" }, + self.tcx.def_path_str(t.def_id), + if has_bounds.is_some() { " + " } else { "" }, + ) }), Applicability::MaybeIncorrect, ); - suggested = true; } - _ => {} + suggested = true; + } + Node::Item(hir::Item { + kind: hir::ItemKind::Trait(.., bounds, _), + ident, + .. + }) => { + let (sp, sep, article) = if bounds.is_empty() { + (ident.span.shrink_to_hi(), ":", "a") + } else { + (bounds.last().unwrap().span().shrink_to_hi(), " +", "another") + }; + err.span_suggestions( + sp, + &message(format!("add {} supertrait for", article)), + candidates.iter().map(|t| { + format!("{} {}", sep, self.tcx.def_path_str(t.def_id),) + }), + Applicability::MaybeIncorrect, + ); + suggested = true; } + _ => {} } - }; + } } if !suggested { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index b617937d6bd54..58fd0f989c678 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1126,7 +1126,7 @@ fn typeck_tables_of_with_fallback<'tcx>( // Consistency check our TypeckTables instance can hold all ItemLocalIds // it will need to hold. - assert_eq!(tables.hir_owner, Some(id.owner)); + assert_eq!(tables.hir_owner, id.owner); tables } diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 4704d8fc7666f..fa17696e02b30 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -109,12 +109,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { ) -> WritebackCx<'cx, 'tcx> { let owner = body.id().hir_id.owner; - WritebackCx { - fcx, - tables: ty::TypeckTables::empty(Some(owner)), - body, - rustc_dump_user_substs, - } + WritebackCx { fcx, tables: ty::TypeckTables::new(owner), body, rustc_dump_user_substs } } fn tcx(&self) -> TyCtxt<'tcx> { @@ -342,7 +337,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_closures(&mut self) { let fcx_tables = self.fcx.tables.borrow(); assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); - let common_hir_owner = fcx_tables.hir_owner.unwrap(); + let common_hir_owner = fcx_tables.hir_owner; for (&id, &origin) in fcx_tables.closure_kind_origins().iter() { let hir_id = hir::HirId { owner: common_hir_owner, local_id: id }; @@ -363,7 +358,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_user_provided_tys(&mut self) { let fcx_tables = self.fcx.tables.borrow(); assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); - let common_hir_owner = fcx_tables.hir_owner.unwrap(); + let common_hir_owner = fcx_tables.hir_owner; let mut errors_buffer = Vec::new(); for (&local_id, c_ty) in fcx_tables.user_provided_types().iter() { @@ -561,7 +556,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_liberated_fn_sigs(&mut self) { let fcx_tables = self.fcx.tables.borrow(); assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); - let common_hir_owner = fcx_tables.hir_owner.unwrap(); + let common_hir_owner = fcx_tables.hir_owner; for (&local_id, fn_sig) in fcx_tables.liberated_fn_sigs().iter() { let hir_id = hir::HirId { owner: common_hir_owner, local_id }; @@ -573,7 +568,7 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_fru_field_types(&mut self) { let fcx_tables = self.fcx.tables.borrow(); assert_eq!(fcx_tables.hir_owner, self.tables.hir_owner); - let common_hir_owner = fcx_tables.hir_owner.unwrap(); + let common_hir_owner = fcx_tables.hir_owner; for (&local_id, ftys) in fcx_tables.fru_field_types().iter() { let hir_id = hir::HirId { owner: common_hir_owner, local_id };