Skip to content

Commit

Permalink
Remove TypeckTables::empty(None) and make hir_owner non-optional.
Browse files Browse the repository at this point in the history
  • Loading branch information
@eddyb
eddyb committed Jul 2, 2020
1 parent 547be8c commit 4b2d9e6
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 121 deletions.
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
69 changes: 27 additions & 42 deletions src/librustc_middle/ty/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ pub struct CommonConsts<'tcx> {
}

pub struct LocalTableInContext<'a, V> {
hir_owner: Option<LocalDefId>,
hir_owner: LocalDefId,
data: &'a ItemLocalMap<V>,
}

Expand All @@ -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<LocalDefId>,
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)
}

Expand All @@ -252,28 +237,28 @@ impl<'a, V> ::std::ops::Index<hir::HirId> for LocalTableInContext<'a, V> {
}

pub struct LocalTableInContextMut<'a, V> {
hir_owner: Option<LocalDefId>,
hir_owner: LocalDefId,
data: &'a mut ItemLocalMap<V>,
}

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<V> {
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<V> {
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)
}
}
Expand Down Expand Up @@ -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<LocalDefId>,
pub hir_owner: LocalDefId,

/// Resolved definitions for `<T>::X` associated paths and
/// method calls, including those of overloaded operators.
Expand Down Expand Up @@ -432,7 +417,7 @@ pub struct TypeckTables<'tcx> {
}

impl<'tcx> TypeckTables<'tcx> {
pub fn empty(hir_owner: Option<LocalDefId>) -> TypeckTables<'tcx> {
pub fn new(hir_owner: LocalDefId) -> TypeckTables<'tcx> {
TypeckTables {
hir_owner,
type_dependent_defs: Default::default(),
Expand Down Expand Up @@ -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())
}

Expand Down Expand Up @@ -521,7 +506,7 @@ impl<'tcx> TypeckTables<'tcx> {
}

pub fn node_type_opt(&self, id: hir::HirId) -> Option<Ty<'tcx>> {
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()
}

Expand All @@ -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<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()
}

Expand Down Expand Up @@ -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[..])
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -710,7 +695,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> 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),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trait_selection/traits/error_reporting/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
127 changes: 62 additions & 65 deletions src/librustc_typeck/check/method/suggest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(_),
..
} = &param.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(_), .. } =
&param.kind
{
// We've found `fn foo(x: impl Trait)` instead of
// `fn foo<T>(x: T)`. We want to suggest the correct
Expand All @@ -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<DefId> = 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<DefId> = 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 {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 4b2d9e6

Please sign in to comment.