Skip to content

Commit

Permalink
Rollup merge of #101447 - cjgillot:no-remap-resolver, r=spastorino
Browse files Browse the repository at this point in the history
Remove generics_def_id_map from the resolver.

This is internal state for lowering.  This does not belong in the resolver.

r? ``@spastorino``
  • Loading branch information
JohnTitor authored Sep 5, 2022
2 parents c6f6b18 + 037ab1d commit 0d8a1f4
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 58 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
allow_try_trait: Some([sym::try_trait_v2, sym::yeet_desugar_details][..].into()),
allow_gen_future: Some([sym::gen_future][..].into()),
allow_into_future: Some([sym::into_future][..].into()),
generics_def_id_map: Default::default(),
};
lctx.with_hir_id_owner(owner, |lctx| f(lctx));

Expand Down
83 changes: 38 additions & 45 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ struct LoweringContext<'a, 'hir> {
allow_try_trait: Option<Lrc<[Symbol]>>,
allow_gen_future: Option<Lrc<[Symbol]>>,
allow_into_future: Option<Lrc<[Symbol]>>,

/// Mapping from generics `def_id`s to TAIT generics `def_id`s.
/// For each captured lifetime (e.g., 'a), we create a new lifetime parameter that is a generic
/// defined on the TAIT, so we have type Foo<'a1> = ... and we establish a mapping in this
/// field from the original parameter 'a to the new parameter 'a1.
generics_def_id_map: Vec<FxHashMap<LocalDefId, LocalDefId>>,
}

trait ResolverAstLoweringExt {
Expand All @@ -142,12 +148,6 @@ trait ResolverAstLoweringExt {
fn get_lifetime_res(&self, id: NodeId) -> Option<LifetimeRes>;
fn take_extra_lifetime_params(&mut self, id: NodeId) -> Vec<(Ident, NodeId, LifetimeRes)>;
fn decl_macro_kind(&self, def_id: LocalDefId) -> MacroKind;
/// Record the map from `from` local def id to `to` local def id, on `generics_def_id_map`
/// field.
fn record_def_id_remap(&mut self, from: LocalDefId, to: LocalDefId);
/// Get the previously recorded `to` local def id given the `from` local def id, obtained using
/// `generics_def_id_map` field.
fn get_remapped_def_id(&self, local_def_id: LocalDefId) -> LocalDefId;
}

impl ResolverAstLoweringExt for ResolverAstLowering {
Expand Down Expand Up @@ -215,41 +215,6 @@ impl ResolverAstLoweringExt for ResolverAstLowering {
fn decl_macro_kind(&self, def_id: LocalDefId) -> MacroKind {
self.builtin_macro_kinds.get(&def_id).copied().unwrap_or(MacroKind::Bang)
}

/// Push a remapping into the top-most map.
/// Panics if no map has been pushed.
/// Remapping is used when creating lowering `-> impl Trait` return
/// types to create the resulting opaque type.
#[instrument(level = "debug", skip(self))]
fn record_def_id_remap(&mut self, from: LocalDefId, to: LocalDefId) {
self.generics_def_id_map.last_mut().expect("no map pushed").insert(from, to);
}

fn get_remapped_def_id(&self, mut local_def_id: LocalDefId) -> LocalDefId {
// `generics_def_id_map` is a stack of mappings. As we go deeper in impl traits nesting we
// push new mappings so we need to try first the latest mappings, hence `iter().rev()`.
//
// Consider:
//
// `fn test<'a, 'b>() -> impl Trait<&'a u8, Ty = impl Sized + 'b> {}`
//
// We would end with a generics_def_id_map like:
//
// `[[fn#'b -> impl_trait#'b], [fn#'b -> impl_sized#'b]]`
//
// for the opaque type generated on `impl Sized + 'b`, We want the result to be:
// impl_sized#'b, so iterating forward is the wrong thing to do.
for map in self.generics_def_id_map.iter().rev() {
if let Some(r) = map.get(&local_def_id) {
debug!("def_id_remapper: remapping from `{local_def_id:?}` to `{r:?}`");
local_def_id = *r;
} else {
debug!("def_id_remapper: no remapping for `{local_def_id:?}` found in map");
}
}

local_def_id
}
}

/// Context of `impl Trait` in code, which determines whether it is allowed in an HIR subtree,
Expand Down Expand Up @@ -522,13 +487,41 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.resolver
.node_id_to_def_id
.get(&node)
.map(|local_def_id| self.resolver.get_remapped_def_id(*local_def_id))
.map(|local_def_id| self.get_remapped_def_id(*local_def_id))
}

fn local_def_id(&self, node: NodeId) -> LocalDefId {
self.opt_local_def_id(node).unwrap_or_else(|| panic!("no entry for node id: `{:?}`", node))
}

/// Get the previously recorded `to` local def id given the `from` local def id, obtained using
/// `generics_def_id_map` field.
fn get_remapped_def_id(&self, mut local_def_id: LocalDefId) -> LocalDefId {
// `generics_def_id_map` is a stack of mappings. As we go deeper in impl traits nesting we
// push new mappings so we need to try first the latest mappings, hence `iter().rev()`.
//
// Consider:
//
// `fn test<'a, 'b>() -> impl Trait<&'a u8, Ty = impl Sized + 'b> {}`
//
// We would end with a generics_def_id_map like:
//
// `[[fn#'b -> impl_trait#'b], [fn#'b -> impl_sized#'b]]`
//
// for the opaque type generated on `impl Sized + 'b`, We want the result to be:
// impl_sized#'b, so iterating forward is the wrong thing to do.
for map in self.generics_def_id_map.iter().rev() {
if let Some(r) = map.get(&local_def_id) {
debug!("def_id_remapper: remapping from `{local_def_id:?}` to `{r:?}`");
local_def_id = *r;
} else {
debug!("def_id_remapper: no remapping for `{local_def_id:?}` found in map");
}
}

local_def_id
}

/// Freshen the `LoweringContext` and ready it to lower a nested item.
/// The lowered item is registered into `self.children`.
///
Expand Down Expand Up @@ -597,9 +590,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
remap: FxHashMap<LocalDefId, LocalDefId>,
f: impl FnOnce(&mut Self) -> R,
) -> R {
self.resolver.generics_def_id_map.push(remap);
self.generics_def_id_map.push(remap);
let res = f(self);
self.resolver.generics_def_id_map.pop();
self.generics_def_id_map.pop();
res
}

Expand Down Expand Up @@ -2027,7 +2020,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let name = match res {
LifetimeRes::Param { param, .. } => {
let p_name = ParamName::Plain(ident);
let param = self.resolver.get_remapped_def_id(param);
let param = self.get_remapped_def_id(param);

hir::LifetimeName::Param(param, p_name)
}
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,6 @@ pub struct ResolverAstLowering {
pub label_res_map: NodeMap<ast::NodeId>,
/// Resolutions for lifetimes.
pub lifetimes_res_map: NodeMap<LifetimeRes>,
/// Mapping from generics `def_id`s to TAIT generics `def_id`s.
/// For each captured lifetime (e.g., 'a), we create a new lifetime parameter that is a generic
/// defined on the TAIT, so we have type Foo<'a1> = ... and we establish a mapping in this
/// field from the original parameter 'a to the new parameter 'a1.
pub generics_def_id_map: Vec<FxHashMap<LocalDefId, LocalDefId>>,
/// Lifetime parameters that lowering will have to introduce.
pub extra_lifetime_params_map: NodeMap<Vec<(Ident, ast::NodeId, LifetimeRes)>>,

Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,11 +911,6 @@ pub struct Resolver<'a> {
label_res_map: NodeMap<NodeId>,
/// Resolutions for lifetimes.
lifetimes_res_map: NodeMap<LifetimeRes>,
/// Mapping from generics `def_id`s to TAIT generics `def_id`s.
/// For each captured lifetime (e.g., 'a), we create a new lifetime parameter that is a generic
/// defined on the TAIT, so we have type Foo<'a1> = ... and we establish a mapping in this
/// field from the original parameter 'a to the new parameter 'a1.
generics_def_id_map: Vec<FxHashMap<LocalDefId, LocalDefId>>,
/// Lifetime parameters that lowering will have to introduce.
extra_lifetime_params_map: NodeMap<Vec<(Ident, NodeId, LifetimeRes)>>,

Expand Down Expand Up @@ -1278,7 +1273,6 @@ impl<'a> Resolver<'a> {
import_res_map: Default::default(),
label_res_map: Default::default(),
lifetimes_res_map: Default::default(),
generics_def_id_map: Vec::new(),
extra_lifetime_params_map: Default::default(),
extern_crate_map: Default::default(),
reexport_map: FxHashMap::default(),
Expand Down Expand Up @@ -1445,7 +1439,6 @@ impl<'a> Resolver<'a> {
import_res_map: self.import_res_map,
label_res_map: self.label_res_map,
lifetimes_res_map: self.lifetimes_res_map,
generics_def_id_map: self.generics_def_id_map,
extra_lifetime_params_map: self.extra_lifetime_params_map,
next_node_id: self.next_node_id,
node_id_to_def_id: self.node_id_to_def_id,
Expand Down Expand Up @@ -1490,7 +1483,6 @@ impl<'a> Resolver<'a> {
import_res_map: self.import_res_map.clone(),
label_res_map: self.label_res_map.clone(),
lifetimes_res_map: self.lifetimes_res_map.clone(),
generics_def_id_map: self.generics_def_id_map.clone(),
extra_lifetime_params_map: self.extra_lifetime_params_map.clone(),
next_node_id: self.next_node_id.clone(),
node_id_to_def_id: self.node_id_to_def_id.clone(),
Expand Down

0 comments on commit 0d8a1f4

Please sign in to comment.