Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify resolve_lifetimes #57408

Closed
wants to merge 4 commits into from
Closed

Simplify resolve_lifetimes #57408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a6bde67
hir: add hir_id to hir::Lifetime
ljedrz Jan 7, 2019
c840c30
hir: add hir_id to hir::GenericParam
ljedrz Jan 7, 2019
9012e56
middle: HirId-ify NamedRegionMap
ljedrz Jan 7, 2019
a82a741
middle: use ItemLocal map aliases
ljedrz Jan 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions src/librustc/hir/lowering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ impl<'a> LoweringContext<'a> {
let params = lifetimes_to_define
.into_iter()
.map(|(span, hir_name)| {
let def_node_id = self.next_id().node_id;
let LoweredNodeId { node_id, hir_id } = self.next_id();

// Get the name we'll use to make the def-path. Note
// that collisions are ok here and this shouldn't
Expand All @@ -736,15 +736,16 @@ impl<'a> LoweringContext<'a> {
// Add a definition for the in-band lifetime def.
self.resolver.definitions().create_def_with_parent(
parent_id.index,
def_node_id,
node_id,
DefPathData::LifetimeParam(str_name),
DefIndexAddressSpace::High,
Mark::root(),
span,
);

hir::GenericParam {
id: def_node_id,
id: node_id,
hir_id,
name: hir_name,
attrs: hir_vec![],
bounds: hir_vec![],
Expand Down Expand Up @@ -1233,7 +1234,7 @@ impl<'a> LoweringContext<'a> {
)
}
ImplTraitContext::Universal(in_band_ty_params) => {
self.lower_node_id(def_node_id);
let LoweredNodeId { hir_id, .. } = self.lower_node_id(def_node_id);
// Add a definition for the in-band `Param`.
let def_index = self
.resolver
Expand All @@ -1249,6 +1250,7 @@ impl<'a> LoweringContext<'a> {
let ident = Ident::from_str(&pprust::ty_to_string(t)).with_span_pos(span);
in_band_ty_params.push(hir::GenericParam {
id: def_node_id,
hir_id,
name: ParamName::Plain(ident),
pure_wrt_drop: false,
attrs: hir_vec![],
Expand Down Expand Up @@ -1477,8 +1479,11 @@ impl<'a> LoweringContext<'a> {
&& !self.already_defined_lifetimes.contains(&name) {
self.already_defined_lifetimes.insert(name);

let LoweredNodeId { node_id, hir_id } = self.context.next_id();

self.output_lifetimes.push(hir::GenericArg::Lifetime(hir::Lifetime {
id: self.context.next_id().node_id,
id: node_id,
hir_id,
span: lifetime.span,
name,
}));
Expand Down Expand Up @@ -1511,6 +1516,7 @@ impl<'a> LoweringContext<'a> {

self.output_lifetime_params.push(hir::GenericParam {
id: def_node_id,
hir_id,
name,
span: lifetime.span,
pure_wrt_drop: false,
Expand Down Expand Up @@ -2282,8 +2288,10 @@ impl<'a> LoweringContext<'a> {
];

if let Some((name, span)) = bound_lifetime {
let LoweredNodeId { node_id, hir_id } = this.next_id();

bounds.push(hir::GenericBound::Outlives(
hir::Lifetime { id: this.next_id().node_id, name, span }));
hir::Lifetime { id: node_id, hir_id, name, span }));
}

hir::HirVec::from(bounds)
Expand Down Expand Up @@ -2350,8 +2358,11 @@ impl<'a> LoweringContext<'a> {
span: Span,
name: hir::LifetimeName,
) -> hir::Lifetime {
let LoweredNodeId { node_id, hir_id } = self.lower_node_id(id);

hir::Lifetime {
id: self.lower_node_id(id).node_id,
id: node_id,
hir_id,
span,
name: name,
}
Expand Down Expand Up @@ -2396,6 +2407,7 @@ impl<'a> LoweringContext<'a> {
};
let param = hir::GenericParam {
id: lt.id,
hir_id: lt.hir_id,
name: param_name,
span: lt.span,
pure_wrt_drop: attr::contains_name(&param.attrs, "may_dangle"),
Expand Down Expand Up @@ -2428,8 +2440,11 @@ impl<'a> LoweringContext<'a> {
.collect();
}

let LoweredNodeId { node_id, hir_id } = self.lower_node_id(param.id);

hir::GenericParam {
id: self.lower_node_id(param.id).node_id,
id: node_id,
hir_id,
name: hir::ParamName::Plain(ident),
pure_wrt_drop: attr::contains_name(&param.attrs, "may_dangle"),
attrs: self.lower_attrs(&param.attrs),
Expand Down Expand Up @@ -4957,8 +4972,11 @@ impl<'a> LoweringContext<'a> {
// `'f`.
AnonymousLifetimeMode::CreateParameter => {
let fresh_name = self.collect_fresh_in_band_lifetime(span);
let LoweredNodeId { node_id, hir_id } = self.next_id();

hir::Lifetime {
id: self.next_id().node_id,
id: node_id,
hir_id,
span,
name: hir::LifetimeName::Param(fresh_name),
}
Expand Down Expand Up @@ -5058,8 +5076,11 @@ impl<'a> LoweringContext<'a> {
}

fn new_implicit_lifetime(&mut self, span: Span) -> hir::Lifetime {
let LoweredNodeId { node_id, hir_id } = self.next_id();

hir::Lifetime {
id: self.next_id().node_id,
id: node_id,
hir_id,
span,
name: hir::LifetimeName::Implicit,
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/hir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl fmt::Debug for Label {
#[derive(Clone, RustcEncodable, RustcDecodable, Copy)]
pub struct Lifetime {
pub id: NodeId,
pub hir_id: HirId,
pub span: Span,

/// Either "'a", referring to a named lifetime definition,
Expand Down Expand Up @@ -538,6 +539,7 @@ pub enum GenericParamKind {
#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
pub struct GenericParam {
pub id: NodeId,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid having NodeId at all? Then this would be a clear win. Have you looked at the remaining uses of id for GenericParam and Lifetime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have, and there were quite a few. I can try giving it a shot 👍.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varkor Actually this is super complex; NodeId is still used in lots of places and just removing it causes lots of breakage, making it hard to debug. I can see that most Hir objects still point to NodeId, probably for the same reason. I have a suggestion: I will try to migrate methods working on NodeId to HirId in following PRs; at one point the compiler should just tell us that the NodeId field is not used anymore - that approach should be a lot cleaner.

pub hir_id: HirId,
pub name: ParamName,
pub attrs: HirVec<Attribute>,
pub bounds: GenericBounds,
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ich/impls_hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ impl_stable_hash_for!(struct hir::Label {

impl_stable_hash_for!(struct hir::Lifetime {
id,
hir_id,
span,
name
});
Expand Down Expand Up @@ -200,6 +201,7 @@ impl_stable_hash_for!(enum hir::TraitBoundModifier {

impl_stable_hash_for!(struct hir::GenericParam {
id,
hir_id,
name,
pure_wrt_drop,
attrs,
Expand Down
55 changes: 27 additions & 28 deletions src/librustc/middle/resolve_lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use hir::def::Def;
use hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE};
use hir::map::Map;
use hir::{GenericArg, GenericParam, ItemLocalId, LifetimeName, Node, ParamName};
use hir::{GenericArg, GenericParam, LifetimeName, Node, ParamName};
use ty::{self, DefIdTree, GenericParamDefKind, TyCtxt};

use errors::{Applicability, DiagnosticBuilder};
Expand All @@ -23,7 +23,7 @@ use syntax::attr;
use syntax::ptr::P;
use syntax::symbol::keywords;
use syntax_pos::Span;
use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap, NodeSet};
use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, HirIdMap, HirIdSet, ItemLocalMap, ItemLocalSet};

use hir::intravisit::{self, NestedVisitorMap, Visitor};
use hir::{self, GenericParamKind, LifetimeParamKind};
Expand Down Expand Up @@ -151,7 +151,7 @@ impl Region {
if let Region::EarlyBound(index, _, _) = self {
params
.nth(index as usize)
.and_then(|lifetime| map.defs.get(&lifetime.id).cloned())
.and_then(|lifetime| map.defs.get(&lifetime.hir_id).cloned())
} else {
Some(self)
}
Expand Down Expand Up @@ -195,25 +195,25 @@ pub type ObjectLifetimeDefault = Set1<Region>;
struct NamedRegionMap {
// maps from every use of a named (not anonymous) lifetime to a
// `Region` describing how that region is bound
pub defs: NodeMap<Region>,
pub defs: HirIdMap<Region>,

// the set of lifetime def ids that are late-bound; a region can
// be late-bound if (a) it does NOT appear in a where-clause and
// (b) it DOES appear in the arguments.
pub late_bound: NodeSet,
pub late_bound: HirIdSet,

// For each type and trait definition, maps type parameters
// to the trait object lifetime defaults computed from them.
pub object_lifetime_defaults: NodeMap<Vec<ObjectLifetimeDefault>>,
pub object_lifetime_defaults: HirIdMap<Vec<ObjectLifetimeDefault>>,
}

/// See `NamedRegionMap`.
#[derive(Default)]
pub struct ResolveLifetimes {
defs: FxHashMap<LocalDefId, Lrc<FxHashMap<ItemLocalId, Region>>>,
late_bound: FxHashMap<LocalDefId, Lrc<FxHashSet<ItemLocalId>>>,
defs: FxHashMap<LocalDefId, Lrc<ItemLocalMap<Region>>>,
late_bound: FxHashMap<LocalDefId, Lrc<ItemLocalSet>>,
object_lifetime_defaults:
FxHashMap<LocalDefId, Lrc<FxHashMap<ItemLocalId, Lrc<Vec<ObjectLifetimeDefault>>>>>,
FxHashMap<LocalDefId, Lrc<ItemLocalMap<Lrc<Vec<ObjectLifetimeDefault>>>>>,
}

impl_stable_hash_for!(struct ::middle::resolve_lifetime::ResolveLifetimes {
Expand Down Expand Up @@ -387,20 +387,17 @@ fn resolve_lifetimes<'tcx>(

let mut rl = ResolveLifetimes::default();

for (k, v) in named_region_map.defs {
let hir_id = tcx.hir().node_to_hir_id(k);
for (hir_id, v) in named_region_map.defs {
let map = rl.defs.entry(hir_id.owner_local_def_id()).or_default();
Lrc::get_mut(map).unwrap().insert(hir_id.local_id, v);
}
for k in named_region_map.late_bound {
let hir_id = tcx.hir().node_to_hir_id(k);
for hir_id in named_region_map.late_bound {
let map = rl.late_bound
.entry(hir_id.owner_local_def_id())
.or_default();
Lrc::get_mut(map).unwrap().insert(hir_id.local_id);
}
for (k, v) in named_region_map.object_lifetime_defaults {
let hir_id = tcx.hir().node_to_hir_id(k);
for (hir_id, v) in named_region_map.object_lifetime_defaults {
let map = rl.object_lifetime_defaults
.entry(hir_id.owner_local_def_id())
.or_default();
Expand Down Expand Up @@ -634,7 +631,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
hir::TyKind::Rptr(ref lifetime_ref, ref mt) => {
self.visit_lifetime(lifetime_ref);
let scope = Scope::ObjectLifetimeDefault {
lifetime: self.map.defs.get(&lifetime_ref.id).cloned(),
lifetime: self.map.defs.get(&lifetime_ref.hir_id).cloned(),
s: self.scope,
};
self.with(scope, |_, this| this.visit_ty(&mt.ty));
Expand Down Expand Up @@ -677,7 +674,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
// and ban them. Type variables instantiated inside binders aren't
// well-supported at the moment, so this doesn't work.
// In the future, this should be fixed and this error should be removed.
let def = self.map.defs.get(&lifetime.id).cloned();
let def = self.map.defs.get(&lifetime.hir_id).cloned();
if let Some(Region::LateBound(_, def_id, _)) = def {
if let Some(node_id) = self.tcx.hir().as_local_node_id(def_id) {
// Ensure that the parent of the def is an item, not HRTB
Expand Down Expand Up @@ -1267,8 +1264,8 @@ fn extract_labels(ctxt: &mut LifetimeContext<'_, '_>, body: &hir::Body) {

fn compute_object_lifetime_defaults(
tcx: TyCtxt<'_, '_, '_>,
) -> NodeMap<Vec<ObjectLifetimeDefault>> {
let mut map = NodeMap::default();
) -> HirIdMap<Vec<ObjectLifetimeDefault>> {
let mut map = HirIdMap::default();
for item in tcx.hir().krate().items.values() {
match item.node {
hir::ItemKind::Struct(_, ref generics)
Expand Down Expand Up @@ -1312,7 +1309,7 @@ fn compute_object_lifetime_defaults(
tcx.sess.span_err(item.span, &object_lifetime_default_reprs);
}

map.insert(item.id, result);
map.insert(item.hir_id, result);
}
_ => {}
}
Expand Down Expand Up @@ -1711,7 +1708,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.iter()
.filter_map(|param| match param.kind {
GenericParamKind::Lifetime { .. } => {
if self.map.late_bound.contains(&param.id) {
if self.map.late_bound.contains(&param.hir_id) {
Some(Region::late(&self.tcx.hir(), param))
} else {
Some(Region::early(&self.tcx.hir(), &mut index, param))
Expand Down Expand Up @@ -1957,8 +1954,10 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
};

let map = &self.map;
let unsubst = if let Some(id) = self.tcx.hir().as_local_node_id(def_id) {
&map.object_lifetime_defaults[&id]
let hir = self.tcx.hir();
let unsubst = if let Some(node_id) = hir.as_local_node_id(def_id) {
let hir_id = hir.definitions().node_to_hir_id(node_id);
&map.object_lifetime_defaults[&hir_id]
} else {
let tcx = self.tcx;
self.xcrate_object_lifetime_defaults
Expand Down Expand Up @@ -2145,7 +2144,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
if let hir::TyKind::Rptr(lifetime_ref, ref mt) = inputs[0].node {
if let hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) = mt.ty.node {
if is_self_ty(path.def) {
if let Some(&lifetime) = self.map.defs.get(&lifetime_ref.id) {
if let Some(&lifetime) = self.map.defs.get(&lifetime_ref.hir_id) {
let scope = Scope::Elision {
elide: Elide::Exact(lifetime),
s: self.scope,
Expand Down Expand Up @@ -2264,7 +2263,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

fn visit_lifetime(&mut self, lifetime_ref: &hir::Lifetime) {
if let Some(&lifetime) = self.map.defs.get(&lifetime_ref.id) {
if let Some(&lifetime) = self.map.defs.get(&lifetime_ref.hir_id) {
match lifetime {
Region::LateBound(debruijn, _, _) | Region::LateBoundAnon(debruijn, _)
if debruijn < self.outer_index =>
Expand Down Expand Up @@ -2669,7 +2668,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
def,
self.tcx.sess.source_map().span_to_string(lifetime_ref.span)
);
self.map.defs.insert(lifetime_ref.id, def);
self.map.defs.insert(lifetime_ref.hir_id, def);

match def {
Region::LateBoundAnon(..) | Region::Static => {
Expand Down Expand Up @@ -2701,7 +2700,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
/// error (esp. around impl trait). In that case, we remove the
/// entry into `map.defs` so as not to confuse later code.
fn uninsert_lifetime_on_error(&mut self, lifetime_ref: &'tcx hir::Lifetime, bad_def: Region) {
let old_value = self.map.defs.remove(&lifetime_ref.id);
let old_value = self.map.defs.remove(&lifetime_ref.hir_id);
assert_eq!(old_value, Some(bad_def));
}
}
Expand Down Expand Up @@ -2793,7 +2792,7 @@ fn insert_late_bound_lifetimes(
param.id
);

let inserted = map.late_bound.insert(param.id);
let inserted = map.late_bound.insert(param.hir_id);
assert!(inserted, "visited lifetime {:?} twice", param.id);
}

Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ impl<'a, 'tcx, 'rcx> DocContext<'a, 'tcx, 'rcx> {

args.push(hir::GenericArg::Lifetime(hir::Lifetime {
id: ast::DUMMY_NODE_ID,
hir_id: hir::DUMMY_HIR_ID,
span: DUMMY_SP,
name: hir::LifetimeName::Param(name),
}));
Expand Down