Skip to content

Commit

Permalink
Auto merge of #120206 - petrochenkov:somehir, r=<try>
Browse files Browse the repository at this point in the history
hir: Make sure all `HirId`s have corresponding HIR `Node`s

And then remove `tcx.opt_hir_node(hir_id)` in favor of `tcx.hir_node(hir_id)`.
  • Loading branch information
bors committed Jan 21, 2024
2 parents ef71f10 + f26f870 commit 21fc882
Show file tree
Hide file tree
Showing 52 changed files with 352 additions and 367 deletions.
39 changes: 37 additions & 2 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub(super) fn index_hir<'hir>(
tcx: TyCtxt<'hir>,
item: hir::OwnerNode<'hir>,
bodies: &SortedMap<ItemLocalId, &'hir Body<'hir>>,
) -> (IndexVec<ItemLocalId, Option<ParentedNode<'hir>>>, LocalDefIdMap<ItemLocalId>) {
) -> (IndexVec<ItemLocalId, ParentedNode<'hir>>, LocalDefIdMap<ItemLocalId>) {
let mut nodes = IndexVec::new();
// This node's parent should never be accessed: the owner's parent is computed by the
// hir_owner_parent query. Make it invalid (= ItemLocalId::MAX) to force an ICE whenever it is
Expand All @@ -54,7 +54,18 @@ pub(super) fn index_hir<'hir>(
OwnerNode::ForeignItem(item) => collector.visit_foreign_item(item),
};

(collector.nodes, collector.parenting)
let err = || {
let span = item.span();
tcx.dcx().span_delayed_bug(*span, "ID not encountered when visiting item HIR");
ParentedNode { parent: ItemLocalId::new(0), node: Node::Err(span) }
};
let nodes = collector
.nodes
.into_iter()
.map(|parented_node| parented_node.unwrap_or_else(err))
.collect();

(nodes, collector.parenting)
}

impl<'a, 'hir> NodeCollector<'a, 'hir> {
Expand Down Expand Up @@ -348,4 +359,28 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {

self.visit_nested_foreign_item(id);
}

fn visit_where_predicate(&mut self, predicate: &'hir WherePredicate<'hir>) {
match predicate {
WherePredicate::BoundPredicate(pred) => {
self.insert(pred.span, pred.hir_id, Node::WhereBoundPredicate(pred));
self.with_parent(pred.hir_id, |this| {
intravisit::walk_where_predicate(this, predicate)
})
}
_ => intravisit::walk_where_predicate(self, predicate),
}
}

fn visit_let_expr(&mut self, lex: &'hir Let<'hir>) {
self.insert(lex.span, lex.hir_id, Node::Let(lex));
self.with_parent(lex.hir_id, |this| intravisit::walk_let_expr(this, lex))
}

fn visit_array_length(&mut self, len: &'hir ArrayLen) {
match len {
ArrayLen::Infer(hir_id, span) => self.insert(*span, *hir_id, Node::ArrayLenInfer(span)),
ArrayLen::Body(..) => intravisit::walk_array_len(self, len),
}
}
}
121 changes: 57 additions & 64 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,66 +399,60 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
let typeck = self.infcx.tcx.typeck(self.mir_def_id());
let hir_id = hir.parent_id(expr.hir_id);
if let Some(parent) = self.infcx.tcx.opt_hir_node(hir_id) {
let (def_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent
&& let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind
&& let Some(def_id) = typeck.type_dependent_def_id(parent_expr.hir_id)
{
(def_id.as_local(), args, 1)
} else if let hir::Node::Expr(parent_expr) = parent
&& let hir::ExprKind::Call(call, args) = parent_expr.kind
&& let ty::FnDef(def_id, _) = typeck.node_type(call.hir_id).kind()
{
(def_id.as_local(), args, 0)
} else {
(None, &[][..], 0)
let parent = self.infcx.tcx.hir_node(hir_id);
let (def_id, args, offset) = if let hir::Node::Expr(parent_expr) = parent
&& let hir::ExprKind::MethodCall(_, _, args, _) = parent_expr.kind
&& let Some(def_id) = typeck.type_dependent_def_id(parent_expr.hir_id)
{
(def_id.as_local(), args, 1)
} else if let hir::Node::Expr(parent_expr) = parent
&& let hir::ExprKind::Call(call, args) = parent_expr.kind
&& let ty::FnDef(def_id, _) = typeck.node_type(call.hir_id).kind()
{
(def_id.as_local(), args, 0)
} else {
(None, &[][..], 0)
};
if let Some(def_id) = def_id
&& let node =
self.infcx.tcx.hir_node(self.infcx.tcx.local_def_id_to_hir_id(def_id))
&& let Some(fn_sig) = node.fn_sig()
&& let Some(ident) = node.ident()
&& let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
&& let Some(arg) = fn_sig.decl.inputs.get(pos + offset)
{
let mut span: MultiSpan = arg.span.into();
span.push_span_label(
arg.span,
"this parameter takes ownership of the value".to_string(),
);
let descr = match node.fn_kind() {
Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
Some(hir::intravisit::FnKind::Method(..)) => "method",
Some(hir::intravisit::FnKind::Closure) => "closure",
};
if let Some(def_id) = def_id
&& let Some(node) = self
.infcx
.tcx
.opt_hir_node(self.infcx.tcx.local_def_id_to_hir_id(def_id))
&& let Some(fn_sig) = node.fn_sig()
&& let Some(ident) = node.ident()
&& let Some(pos) = args.iter().position(|arg| arg.hir_id == expr.hir_id)
&& let Some(arg) = fn_sig.decl.inputs.get(pos + offset)
{
let mut span: MultiSpan = arg.span.into();
span.push_span_label(
arg.span,
"this parameter takes ownership of the value".to_string(),
);
let descr = match node.fn_kind() {
Some(hir::intravisit::FnKind::ItemFn(..)) | None => "function",
Some(hir::intravisit::FnKind::Method(..)) => "method",
Some(hir::intravisit::FnKind::Closure) => "closure",
};
span.push_span_label(ident.span, format!("in this {descr}"));
err.span_note(
span,
format!(
"consider changing this parameter type in {descr} `{ident}` to \
span.push_span_label(ident.span, format!("in this {descr}"));
err.span_note(
span,
format!(
"consider changing this parameter type in {descr} `{ident}` to \
borrow instead if owning the value isn't necessary",
),
);
}
let place = &self.move_data.move_paths[mpi].place;
let ty = place.ty(self.body, self.infcx.tcx).ty;
if let hir::Node::Expr(parent_expr) = parent
&& let hir::ExprKind::Call(call_expr, _) = parent_expr.kind
&& let hir::ExprKind::Path(hir::QPath::LangItem(
LangItem::IntoIterIntoIter,
_,
)) = call_expr.kind
{
// Do not suggest `.clone()` in a `for` loop, we already suggest borrowing.
} else if let UseSpans::FnSelfUse { kind: CallKind::Normal { .. }, .. } =
move_spans
{
// We already suggest cloning for these cases in `explain_captures`.
} else {
self.suggest_cloning(err, ty, expr, move_span);
}
),
);
}
let place = &self.move_data.move_paths[mpi].place;
let ty = place.ty(self.body, self.infcx.tcx).ty;
if let hir::Node::Expr(parent_expr) = parent
&& let hir::ExprKind::Call(call_expr, _) = parent_expr.kind
&& let hir::ExprKind::Path(hir::QPath::LangItem(LangItem::IntoIterIntoIter, _)) =
call_expr.kind
{
// Do not suggest `.clone()` in a `for` loop, we already suggest borrowing.
} else if let UseSpans::FnSelfUse { kind: CallKind::Normal { .. }, .. } = move_spans
{
// We already suggest cloning for these cases in `explain_captures`.
} else {
self.suggest_cloning(err, ty, expr, move_span);
}
}
if let Some(pat) = finder.pat {
Expand Down Expand Up @@ -1757,7 +1751,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
fn_decl: hir::FnDecl { inputs, .. },
..
}) = e.kind
&& let Some(hir::Node::Expr(body)) = self.tcx.opt_hir_node(body.hir_id)
&& let hir::Node::Expr(body) = self.tcx.hir_node(body.hir_id)
{
self.suggest_arg = "this: &Self".to_string();
if inputs.len() > 0 {
Expand Down Expand Up @@ -1823,11 +1817,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

if let Some(hir::Node::ImplItem(hir::ImplItem {
if let hir::Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(_fn_sig, body_id),
..
})) = self.infcx.tcx.opt_hir_node(self.mir_hir_id())
&& let Some(hir::Node::Expr(expr)) = self.infcx.tcx.opt_hir_node(body_id.hir_id)
}) = self.infcx.tcx.hir_node(self.mir_hir_id())
&& let hir::Node::Expr(expr) = self.infcx.tcx.hir_node(body_id.hir_id)
{
let mut finder = ExpressionFinder {
capture_span: *capture_kind_span,
Expand Down Expand Up @@ -2395,8 +2389,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let proper_span = proper_span.source_callsite();
if let Some(scope) = self.body.source_scopes.get(source_info.scope)
&& let ClearCrossCrate::Set(scope_data) = &scope.local_data
&& let Some(node) = self.infcx.tcx.opt_hir_node(scope_data.lint_root)
&& let Some(id) = node.body_id()
&& let Some(id) = self.infcx.tcx.hir_node(scope_data.lint_root).body_id()
&& let hir::ExprKind::Block(block, _) = self.infcx.tcx.hir().body(id).value.kind
{
for stmt in block.stmts {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
if let hir::ExprKind::Path(hir::QPath::Resolved(None, p)) = expr.kind
&& let [hir::PathSegment { ident, args: None, .. }] = p.segments
&& let hir::def::Res::Local(hir_id) = p.res
&& let Some(hir::Node::Pat(pat)) = tcx.opt_hir_node(hir_id)
&& let hir::Node::Pat(pat) = tcx.hir_node(hir_id)
{
err.span_label(pat.span, format!("binding `{ident}` declared here"));
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {

let upvar_hir_id = captured_place.get_root_variable();

if let Some(Node::Pat(pat)) = self.infcx.tcx.opt_hir_node(upvar_hir_id)
if let Node::Pat(pat) = self.infcx.tcx.hir_node(upvar_hir_id)
&& let hir::PatKind::Binding(hir::BindingAnnotation::NONE, _, upvar_ident, _) =
pat.kind
{
Expand Down Expand Up @@ -688,15 +688,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
break;
}
f_in_trait_opt.and_then(|f_in_trait| {
match self.infcx.tcx.opt_hir_node(f_in_trait) {
Some(Node::TraitItem(hir::TraitItem {
match self.infcx.tcx.hir_node(f_in_trait) {
Node::TraitItem(hir::TraitItem {
kind:
hir::TraitItemKind::Fn(
hir::FnSig { decl: hir::FnDecl { inputs, .. }, .. },
_,
),
..
})) => {
}) => {
let hir::Ty { span, .. } = inputs[local.index() - 1];
Some(span)
}
Expand Down Expand Up @@ -759,10 +759,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
//
// `let &b = a;` -> `let &(mut b) = a;`
if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(hir::Local {
&& let hir::Node::Local(hir::Local {
pat: hir::Pat { kind: hir::PatKind::Ref(_, _), .. },
..
})) = self.infcx.tcx.opt_hir_node(hir_id)
}) = self.infcx.tcx.hir_node(hir_id)
&& let Ok(name) =
self.infcx.tcx.sess.source_map().span_to_snippet(local_decl.source_info.span)
{
Expand Down Expand Up @@ -1206,7 +1206,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};

if let Some(hir_id) = hir_id
&& let Some(hir::Node::Local(local)) = self.infcx.tcx.opt_hir_node(hir_id)
&& let hir::Node::Local(local) = self.infcx.tcx.hir_node(hir_id)
{
let tables = self.infcx.tcx.typeck(def_id.as_local().unwrap());
if let Some(clone_trait) = self.infcx.tcx.lang_items().clone_trait()
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
if let Some(id) = placeholder.bound.kind.get_id()
&& let Some(placeholder_id) = id.as_local()
&& let gat_hir_id = self.infcx.tcx.local_def_id_to_hir_id(placeholder_id)
&& let Some(generics_impl) = hir.get_parent(gat_hir_id).generics()
&& let Some(generics_impl) =
hir.get_parent(hir.parent_id(gat_hir_id)).generics()
{
Some((gat_hir_id, generics_impl))
} else {
Expand Down
35 changes: 20 additions & 15 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,17 +835,16 @@ pub struct OwnerNodes<'tcx> {
// The zeroth node's parent should never be accessed: the owner's parent is computed by the
// hir_owner_parent query. It is set to `ItemLocalId::INVALID` to force an ICE if accidentally
// used.
pub nodes: IndexVec<ItemLocalId, Option<ParentedNode<'tcx>>>,
pub nodes: IndexVec<ItemLocalId, ParentedNode<'tcx>>,
/// Content of local bodies.
pub bodies: SortedMap<ItemLocalId, &'tcx Body<'tcx>>,
}

impl<'tcx> OwnerNodes<'tcx> {
pub fn node(&self) -> OwnerNode<'tcx> {
use rustc_index::Idx;
let node = self.nodes[ItemLocalId::new(0)].as_ref().unwrap().node;
let node = node.as_owner().unwrap(); // Indexing must ensure it is an OwnerNode.
node
// Indexing must ensure it is an OwnerNode.
self.nodes[ItemLocalId::new(0)].node.as_owner().unwrap()
}
}

Expand All @@ -860,9 +859,7 @@ impl fmt::Debug for OwnerNodes<'_> {
.nodes
.iter_enumerated()
.map(|(id, parented_node)| {
let parented_node = parented_node.as_ref().map(|node| node.parent);

debug_fn(move |f| write!(f, "({id:?}, {parented_node:?})"))
debug_fn(move |f| write!(f, "({id:?}, {:?})", parented_node.parent))
})
.collect::<Vec<_>>(),
)
Expand Down Expand Up @@ -3356,13 +3353,14 @@ impl<'hir> OwnerNode<'hir> {
}
}

pub fn span(&self) -> Span {
#[allow(rustc::pass_by_value)]
pub fn span(&self) -> &'hir Span {
match self {
OwnerNode::Item(Item { span, .. })
| OwnerNode::ForeignItem(ForeignItem { span, .. })
| OwnerNode::ImplItem(ImplItem { span, .. })
| OwnerNode::TraitItem(TraitItem { span, .. }) => *span,
OwnerNode::Crate(Mod { spans: ModSpans { inner_span, .. }, .. }) => *inner_span,
| OwnerNode::TraitItem(TraitItem { span, .. }) => span,
OwnerNode::Crate(Mod { spans: ModSpans { inner_span, .. }, .. }) => inner_span,
}
}

Expand Down Expand Up @@ -3491,17 +3489,20 @@ pub enum Node<'hir> {
Arm(&'hir Arm<'hir>),
Block(&'hir Block<'hir>),
Local(&'hir Local<'hir>),

/// `Ctor` refers to the constructor of an enum variant or struct. Only tuple or unit variants
/// with synthesized constructors.
Ctor(&'hir VariantData<'hir>),

Lifetime(&'hir Lifetime),
GenericParam(&'hir GenericParam<'hir>),

Crate(&'hir Mod<'hir>),

Infer(&'hir InferArg),
WhereBoundPredicate(&'hir WhereBoundPredicate<'hir>),
Let(&'hir Let<'hir>),
// Span by reference to minimize `Node`'s size
#[allow(rustc::pass_by_value)]
ArrayLenInfer(&'hir Span),
#[allow(rustc::pass_by_value)]
Err(&'hir Span),
}

impl<'hir> Node<'hir> {
Expand Down Expand Up @@ -3546,7 +3547,11 @@ impl<'hir> Node<'hir> {
| Node::Crate(..)
| Node::Ty(..)
| Node::TraitRef(..)
| Node::Infer(..) => None,
| Node::Infer(..)
| Node::WhereBoundPredicate(..)
| Node::Let(..)
| Node::ArrayLenInfer(..)
| Node::Err(..) => None,
}
}

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ impl<'a> FnKind<'a> {

/// An abstract representation of the HIR `rustc_middle::hir::map::Map`.
pub trait Map<'hir> {
/// Retrieves the `Node` corresponding to `id`, returning `None` if cannot be found.
fn find(&self, hir_id: HirId) -> Option<Node<'hir>>;
/// Retrieves the `Node` corresponding to `id`.
fn hir_node(&self, hir_id: HirId) -> Node<'hir>;
fn body(&self, id: BodyId) -> &'hir Body<'hir>;
fn item(&self, id: ItemId) -> &'hir Item<'hir>;
fn trait_item(&self, id: TraitItemId) -> &'hir TraitItem<'hir>;
Expand All @@ -119,7 +119,7 @@ pub trait Map<'hir> {

// Used when no map is actually available, forcing manual implementation of nested visitors.
impl<'hir> Map<'hir> for ! {
fn find(&self, _: HirId) -> Option<Node<'hir>> {
fn hir_node(&self, _: HirId) -> Node<'hir> {
*self;
}
fn body(&self, _: BodyId) -> &'hir Body<'hir> {
Expand Down
Loading

0 comments on commit 21fc882

Please sign in to comment.