Skip to content

Commit

Permalink
Auto merge of #58915 - ljedrz:deprecate_nodeid_methods, r=Zoxc
Browse files Browse the repository at this point in the history 
HirIdification: almost there

The next iteration of HirIdification (#57578).

Replaces a bunch of `NodeId` method calls (mostly `as_local_node_id`) with `HirId` ones.

Removes `NodeId` from:
- [x] `PathSegment`
- [x] `PatKind`
- [x] `Destination` (replaces it with `HirId`)

In addition this PR also removes `Visitor::visit_def_mention`, which doesn't seem to be doing anything.
  • Loading branch information
@bors
bors committed Mar 8, 2019
2 parents b58a006 + 24fad4c commit 2a65cbe
Show file tree
Hide file tree
Showing 85 changed files with 438 additions and 488 deletions.
8 changes: 4 additions & 4 deletions src/librustc/cfg/construct.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,9 +571,9 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
match destination.target_id {
Ok(loop_id) => {
for b in &self.breakable_block_scopes {
if b.block_expr_id == self.tcx.hir().node_to_hir_id(loop_id).local_id {
if b.block_expr_id == loop_id.local_id {
let scope = region::Scope {
id: self.tcx.hir().node_to_hir_id(loop_id).local_id,
id: loop_id.local_id,
data: region::ScopeData::Node
};
return (scope, match scope_cf_kind {
Expand All @@ -583,9 +583,9 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
}
}
for l in &self.loop_scopes {
if l.loop_id == self.tcx.hir().node_to_hir_id(loop_id).local_id {
if l.loop_id == loop_id.local_id {
let scope = region::Scope {
id: self.tcx.hir().node_to_hir_id(loop_id).local_id,
id: loop_id.local_id,
data: region::ScopeData::Node
};
return (scope, match scope_cf_kind {
Expand Down
19 changes: 2 additions & 17 deletions src/librustc/hir/intravisit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
use syntax::ast::{Ident, Name, Attribute};
use syntax_pos::Span;
use crate::hir::*;
use crate::hir::def::Def;
use crate::hir::map::Map;
use super::itemlikevisit::DeepVisitor;

Expand Down Expand Up @@ -228,9 +227,6 @@ pub trait Visitor<'v> : Sized {
fn visit_id(&mut self, _hir_id: HirId) {
// Nothing to do.
}
fn visit_def_mention(&mut self, _def: Def) {
// Nothing to do.
}
fn visit_name(&mut self, _span: Span, _name: Name) {
// Nothing to do.
}
Expand Down Expand Up @@ -494,13 +490,10 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) {
visitor.visit_ty(typ);
visitor.visit_generics(type_parameters)
}
ItemKind::Existential(ExistTy {ref generics, ref bounds, impl_trait_fn}) => {
ItemKind::Existential(ExistTy { ref generics, ref bounds, impl_trait_fn: _ }) => {
visitor.visit_id(item.hir_id);
walk_generics(visitor, generics);
walk_list!(visitor, visit_param_bound, bounds);
if let Some(impl_trait_fn) = impl_trait_fn {
visitor.visit_def_mention(Def::Fn(impl_trait_fn))
}
}
ItemKind::Enum(ref enum_definition, ref type_parameters) => {
visitor.visit_generics(type_parameters);
Expand Down Expand Up @@ -640,7 +633,6 @@ pub fn walk_qpath<'v, V: Visitor<'v>>(visitor: &mut V, qpath: &'v QPath, id: Hir
}

pub fn walk_path<'v, V: Visitor<'v>>(visitor: &mut V, path: &'v Path) {
visitor.visit_def_mention(path.def);
for segment in &path.segments {
visitor.visit_path_segment(path.span, segment);
}
Expand Down Expand Up @@ -697,8 +689,7 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat) {
PatKind::Ref(ref subpattern, _) => {
visitor.visit_pat(subpattern)
}
PatKind::Binding(_, canonical_id, _hir_id, ident, ref optional_subpattern) => {
visitor.visit_def_mention(Def::Local(canonical_id));
PatKind::Binding(_, _hir_id, ident, ref optional_subpattern) => {
visitor.visit_ident(ident);
walk_list!(visitor, visit_pat, optional_subpattern);
}
Expand Down Expand Up @@ -1064,18 +1055,12 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
ExprKind::Break(ref destination, ref opt_expr) => {
if let Some(ref label) = destination.label {
visitor.visit_label(label);
if let Ok(node_id) = destination.target_id {
visitor.visit_def_mention(Def::Label(node_id))
}
}
walk_list!(visitor, visit_expr, opt_expr);
}
ExprKind::Continue(ref destination) => {
if let Some(ref label) = destination.label {
visitor.visit_label(label);
if let Ok(node_id) = destination.target_id {
visitor.visit_def_mention(Def::Label(node_id))
}
}
}
ExprKind::Ret(ref optional_expression) => {
Expand Down
25 changes: 11 additions & 14 deletions src/librustc/hir/lowering.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ impl<'a> LoweringContext<'a> {
let target_id = match destination {
Some((id, _)) => {
if let Def::Label(loop_id) = self.expect_full_def(id) {
Ok(self.lower_node_id(loop_id).node_id)
Ok(self.lower_node_id(loop_id).hir_id)
} else {
Err(hir::LoopIdError::UnresolvedLabel)
}
Expand All @@ -1077,7 +1077,7 @@ impl<'a> LoweringContext<'a> {
self.loop_scopes
.last()
.cloned()
.map(|id| Ok(self.lower_node_id(id).node_id))
.map(|id| Ok(self.lower_node_id(id).hir_id))
.unwrap_or(Err(hir::LoopIdError::OutsideLoopScope))
.into()
}
Expand Down Expand Up @@ -1932,7 +1932,6 @@ impl<'a> LoweringContext<'a> {

hir::PathSegment::new(
segment.ident,
Some(id.node_id),
Some(id.hir_id),
Some(def),
generic_args,
Expand Down Expand Up @@ -3276,10 +3275,8 @@ impl<'a> LoweringContext<'a> {
debug!("renumber_segment_ids(path = {:?})", path);
let mut path = path.clone();
for seg in path.segments.iter_mut() {
if seg.id.is_some() {
let next_id = self.next_id();
seg.id = Some(next_id.node_id);
seg.hir_id = Some(next_id.hir_id);
if seg.hir_id.is_some() {
seg.hir_id = Some(self.next_id().hir_id);
}
}
path
Expand Down Expand Up @@ -3682,11 +3679,10 @@ impl<'a> LoweringContext<'a> {
Some(Def::Local(id)) => id,
_ => p.id,
};
let hir_id = self.lower_node_id(canonical_id).hir_id;

hir::PatKind::Binding(
self.lower_binding_mode(binding_mode),
canonical_id,
hir_id,
self.lower_node_id(canonical_id).hir_id,
ident,
sub.as_ref().map(|x| self.lower_pat(x)),
)
Expand Down Expand Up @@ -4568,12 +4564,13 @@ impl<'a> LoweringContext<'a> {
let thin_attrs = ThinVec::from(attrs);
let catch_scope = self.catch_scopes.last().map(|x| *x);
let ret_expr = if let Some(catch_node) = catch_scope {
let target_id = Ok(self.lower_node_id(catch_node).hir_id);
P(self.expr(
e.span,
hir::ExprKind::Break(
hir::Destination {
label: None,
target_id: Ok(catch_node),
target_id,
},
Some(from_err_expr),
),
Expand Down Expand Up @@ -4988,7 +4985,7 @@ impl<'a> LoweringContext<'a> {
(
P(hir::Pat {
hir_id,
node: hir::PatKind::Binding(bm, node_id, hir_id, ident.with_span_pos(span), None),
node: hir::PatKind::Binding(bm, hir_id, ident.with_span_pos(span), None),
span,
}),
node_id
Expand Down Expand Up @@ -5024,8 +5021,8 @@ impl<'a> LoweringContext<'a> {


for seg in path.segments.iter_mut() {
if let Some(id) = seg.id {
seg.id = Some(self.lower_node_id(id).node_id);
if seg.hir_id.is_some() {
seg.hir_id = Some(self.next_id().hir_id);
}
}
path
Expand Down
78 changes: 32 additions & 46 deletions src/librustc/hir/map/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,10 @@ impl<'hir> Map<'hir> {
}

/// Given a body owner's id, returns the `BodyId` associated with it.
pub fn body_owned_by(&self, id: NodeId) -> BodyId {
self.maybe_body_owned_by(id).unwrap_or_else(|| {
span_bug!(self.span(id), "body_owned_by: {} has no associated body",
self.node_to_string(id));
pub fn body_owned_by(&self, id: HirId) -> BodyId {
self.maybe_body_owned_by_by_hir_id(id).unwrap_or_else(|| {
span_bug!(self.span_by_hir_id(id), "body_owned_by: {} has no associated body",
self.hir_to_string(id));
})
}

Expand Down Expand Up @@ -539,19 +539,19 @@ impl<'hir> Map<'hir> {
self.body_owner_kind(node_id)
}

pub fn ty_param_owner(&self, id: NodeId) -> NodeId {
match self.get(id) {
pub fn ty_param_owner(&self, id: HirId) -> HirId {
match self.get_by_hir_id(id) {
Node::Item(&Item { node: ItemKind::Trait(..), .. }) => id,
Node::GenericParam(_) => self.get_parent_node(id),
_ => bug!("ty_param_owner: {} not a type parameter", self.node_to_string(id))
Node::GenericParam(_) => self.get_parent_node_by_hir_id(id),
_ => bug!("ty_param_owner: {} not a type parameter", self.hir_to_string(id))
}
}

pub fn ty_param_name(&self, id: NodeId) -> Name {
match self.get(id) {
pub fn ty_param_name(&self, id: HirId) -> Name {
match self.get_by_hir_id(id) {
Node::Item(&Item { node: ItemKind::Trait(..), .. }) => keywords::SelfUpper.name(),
Node::GenericParam(param) => param.name.ident().name,
_ => bug!("ty_param_name: {} not a type parameter", self.node_to_string(id)),
_ => bug!("ty_param_name: {} not a type parameter", self.hir_to_string(id)),
}
}

Expand Down Expand Up @@ -618,11 +618,11 @@ impl<'hir> Map<'hir> {
}

for id in &module.trait_items {
visitor.visit_trait_item(self.expect_trait_item_by_hir_id(id.hir_id));
visitor.visit_trait_item(self.expect_trait_item(id.hir_id));
}

for id in &module.impl_items {
visitor.visit_impl_item(self.expect_impl_item_by_hir_id(id.hir_id));
visitor.visit_impl_item(self.expect_impl_item(id.hir_id));
}
}

Expand Down Expand Up @@ -929,66 +929,52 @@ impl<'hir> Map<'hir> {

// FIXME(@ljedrz): replace the NodeId variant
pub fn expect_item_by_hir_id(&self, id: HirId) -> &'hir Item {
let node_id = self.hir_to_node_id(id);
self.expect_item(node_id)
match self.find_by_hir_id(id) { // read recorded by `find`
Some(Node::Item(item)) => item,
_ => bug!("expected item, found {}", self.hir_to_string(id))
}
}

pub fn expect_impl_item(&self, id: NodeId) -> &'hir ImplItem {
match self.find(id) {
pub fn expect_impl_item(&self, id: HirId) -> &'hir ImplItem {
match self.find_by_hir_id(id) {
Some(Node::ImplItem(item)) => item,
_ => bug!("expected impl item, found {}", self.node_to_string(id))
_ => bug!("expected impl item, found {}", self.hir_to_string(id))
}
}

// FIXME(@ljedrz): replace the NodeId variant
pub fn expect_impl_item_by_hir_id(&self, id: HirId) -> &'hir ImplItem {
let node_id = self.hir_to_node_id(id);
self.expect_impl_item(node_id)
}

// FIXME(@ljedrz): replace the NodeId variant
pub fn expect_trait_item_by_hir_id(&self, id: HirId) -> &'hir TraitItem {
let node_id = self.hir_to_node_id(id);
self.expect_trait_item(node_id)
}

pub fn expect_trait_item(&self, id: NodeId) -> &'hir TraitItem {
match self.find(id) {
pub fn expect_trait_item(&self, id: HirId) -> &'hir TraitItem {
match self.find_by_hir_id(id) {
Some(Node::TraitItem(item)) => item,
_ => bug!("expected trait item, found {}", self.node_to_string(id))
_ => bug!("expected trait item, found {}", self.hir_to_string(id))
}
}

pub fn expect_variant_data(&self, id: HirId) -> &'hir VariantData {
let id = self.hir_to_node_id(id); // FIXME(@ljedrz): remove when possible

match self.find(id) {
match self.find_by_hir_id(id) {
Some(Node::Item(i)) => {
match i.node {
ItemKind::Struct(ref struct_def, _) |
ItemKind::Union(ref struct_def, _) => struct_def,
_ => bug!("struct ID bound to non-struct {}", self.node_to_string(id))
_ => bug!("struct ID bound to non-struct {}", self.hir_to_string(id))
}
}
Some(Node::StructCtor(data)) => data,
Some(Node::Variant(variant)) => &variant.node.data,
_ => bug!("expected struct or variant, found {}", self.node_to_string(id))
_ => bug!("expected struct or variant, found {}", self.hir_to_string(id))
}
}

pub fn expect_variant(&self, id: HirId) -> &'hir Variant {
let id = self.hir_to_node_id(id); // FIXME(@ljedrz): remove when possible

match self.find(id) {
match self.find_by_hir_id(id) {
Some(Node::Variant(variant)) => variant,
_ => bug!("expected variant, found {}", self.node_to_string(id)),
_ => bug!("expected variant, found {}", self.hir_to_string(id)),
}
}

pub fn expect_foreign_item(&self, id: NodeId) -> &'hir ForeignItem {
match self.find(id) {
pub fn expect_foreign_item(&self, id: HirId) -> &'hir ForeignItem {
match self.find_by_hir_id(id) {
Some(Node::ForeignItem(item)) => item,
_ => bug!("expected foreign item, found {}", self.node_to_string(id))
_ => bug!("expected foreign item, found {}", self.hir_to_string(id))
}
}

Expand Down Expand Up @@ -1016,7 +1002,7 @@ impl<'hir> Map<'hir> {
Node::Field(f) => f.ident.name,
Node::Lifetime(lt) => lt.name.ident().name,
Node::GenericParam(param) => param.name.ident().name,
Node::Binding(&Pat { node: PatKind::Binding(_, _, _, l, _), .. }) => l.name,
Node::Binding(&Pat { node: PatKind::Binding(_, _, l, _), .. }) => l.name,
Node::StructCtor(_) => self.name(self.get_parent(id)),
_ => bug!("no name for {}", self.node_to_string(id))
}
Expand Down
10 changes: 3 additions & 7 deletions src/librustc/hir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ pub struct PathSegment {
// therefore will not have 'jump to def' in IDEs, but otherwise will not be
// affected. (In general, we don't bother to get the defs for synthesized
// segments, only for segments which have come from the AST).
pub id: Option<NodeId>,
pub hir_id: Option<HirId>,
pub def: Option<Def>,

Expand All @@ -351,7 +350,6 @@ impl PathSegment {
pub fn from_ident(ident: Ident) -> PathSegment {
PathSegment {
ident,
id: None,
hir_id: None,
def: None,
infer_types: true,
Expand All @@ -361,15 +359,13 @@ impl PathSegment {

pub fn new(
ident: Ident,
id: Option<NodeId>,
hir_id: Option<HirId>,
def: Option<Def>,
args: GenericArgs,
infer_types: bool,
) -> Self {
PathSegment {
ident,
id,
hir_id,
def,
infer_types,
Expand Down Expand Up @@ -941,10 +937,10 @@ pub enum PatKind {
Wild,

/// A fresh binding `ref mut binding @ OPT_SUBPATTERN`.
/// The `NodeId` is the canonical ID for the variable being bound,
/// The `HirId` is the canonical ID for the variable being bound,
/// (e.g., in `Ok(x) | Err(x)`, both `x` use the same canonical ID),
/// which is the pattern ID of the first `x`.
Binding(BindingAnnotation, NodeId, HirId, Ident, Option<P<Pat>>),
Binding(BindingAnnotation, HirId, Ident, Option<P<Pat>>),

/// A struct or struct variant pattern (e.g., `Variant {x, y, ..}`).
/// The `bool` is `true` in the presence of a `..`.
Expand Down Expand Up @@ -1623,7 +1619,7 @@ pub struct Destination {

// These errors are caught and then reported during the diagnostics pass in
// librustc_passes/loops.rs
pub target_id: Result<NodeId, LoopIdError>,
pub target_id: Result<HirId, LoopIdError>,
}

#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
Expand Down
Loading

0 comments on commit 2a65cbe

Please sign in to comment.