-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Rename hir::Map::{get_,find_}parent_node
to hir::Map::{,opt_}parent_id
, and add hir::Map::{get,find}_parent
#106403
Conversation
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
Well I did something wrong @rustbot author |
0a57f5d
to
b4f6ee9
Compare
compiler/rustc_hir/src/hir.rs
Outdated
@@ -3456,7 +3456,7 @@ impl<'hir> Node<'hir> { | |||
/// ```ignore (illustrative) | |||
/// ctor | |||
/// .ctor_hir_id() | |||
/// .and_then(|ctor_id| tcx.hir().find(tcx.hir().get_parent_node(ctor_id))) | |||
/// .and_then(|ctor_id| tcx.hir().find_parent_node(ctor_id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a glance, this seems incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you be more specific?
get_parent_node(_: HirId) -> HirId
was renamed to parent_id(_: HirId) -> HirId
, and a new find_parent_node(_: HirId) -> Optiion<Node>
was introduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't you introduce find_parent
instead of find_parent_node
?
r=me with the comment and PR title corrected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait -- what modification to the PR title do you want? Also mentioning adding {find,get}_parent
?
edit: just did that.
b4f6ee9
to
59c9517
Compare
hir::Map::{get_,find_}parent_node
to hir::Map::{,opt_}parent_id
hir::Map::{get_,find_}parent_node
to hir::Map::{,opt_}parent_id
, and add hir::Map::{get,find}_parent
@bors r=cjgillot |
📌 Commit 59c95178848867f7cfca35dc8ddf2a217ef641f6 has been approved by It is now in the queue for this repository. |
☔ The latest upstream changes (presumably #105752) made this pull request unmergeable. Please resolve the merge conflicts. |
59c9517
to
b1b19bd
Compare
@bors r=cjgillot |
…, r=cjgillot Rename `hir::Map::{get_,find_}parent_node` to `hir::Map::{,opt_}parent_id`, and add `hir::Map::{get,find}_parent` The `hir::Map::get_parent_node` function doesn't return a `Node`, and I think that's quite confusing. Let's rename it to something that sounds more like something that gets the parent hir id => `hir::Map::parent_id`. Same with `find_parent_node` => `opt_parent_id`. Also, combine `hir.get(hir.parent_id(hir_id))` and similar `hir.find(hir.parent_id(hir_id))` function into new functions that actually retrieve the parent node in one call. This last commit is the only one that might need to be looked at closely.
This is bitrotty @bors p=1 |
…mpiler-errors Rollup of 6 pull requests Successful merges: - rust-lang#105846 (Account for return-position `impl Trait` in trait in `opt_suggest_box_span`) - rust-lang#106385 (Split `-Zchalk` flag into `-Ztrait-solver=(classic|chalk|next)` flag) - rust-lang#106403 (Rename `hir::Map::{get_,find_}parent_node` to `hir::Map::{,opt_}parent_id`, and add `hir::Map::{get,find}_parent`) - rust-lang#106462 (rustdoc: remove unnecessary wrapper around sidebar and mobile logos) - rust-lang#106464 (Update Fuchsia walkthrough with new configs) - rust-lang#106478 (Tweak wording of fn call with wrong number of args) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
The
hir::Map::get_parent_node
function doesn't return aNode
, and I think that's quite confusing. Let's rename it to something that sounds more like something that gets the parent hir id =>hir::Map::parent_id
. Same withfind_parent_node
=>opt_parent_id
.Also, combine
hir.get(hir.parent_id(hir_id))
and similarhir.find(hir.parent_id(hir_id))
function into new functions that actually retrieve the parent node in one call. This last commit is the only one that might need to be looked at closely.