-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
HirId-ify hir::BodyId #58167
HirId-ify hir::BodyId #58167
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
r? @Zoxc |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@Zoxc I've looked at stack traces, but I'm not too savvy with codegen; any suggestions on how I could tackle this? |
Mind posting a stack trace? =P |
This comment has been minimized.
This comment has been minimized.
body: &'gcx hir::Body, | ||
span: Span, | ||
) { | ||
// When we enter a function, we can derive | ||
debug!("visit_fn_body(id={})", id); | ||
debug!("visit_fn_body(id={:?})", 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.
Maybe you should add a Display impl for HirId?
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.
I'd probably prefer to do it in a separate PR, but sure, why not; how would you suggest it should look like?
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.
You can probably just call the Debug
impl for now, and then just revert the changes to use {:?}
I've looked over this and didn't spot anything else that is wrong. |
48cbfc1
to
342cc4d
Compare
@Zoxc Since now the extra |
@ljedrz Just rebase them to remove the commit. I like to use |
342cc4d
to
07cc606
Compare
Rebased. |
@bors r+ |
📌 Commit 07cc606574052298394f41387a18d3912fc4599b has been approved by |
☔ The latest upstream changes (presumably #58254) made this pull request unmergeable. Please resolve the merge conflicts. |
07cc606
to
221b1c3
Compare
Rebased. |
@bors r+ |
📌 Commit 221b1c3 has been approved by |
📌 Commit 932d7e40f7b54d302dd3695f8324663912654d11 has been approved by |
☔ The latest upstream changes (presumably #58341) made this pull request unmergeable. Please resolve the merge conflicts. |
932d7e4
to
eac43cc
Compare
Rebased. |
@bors r+ |
📌 Commit eac43cc has been approved by |
HirId-ify hir::BodyId Another step towards rust-lang#57578.
HirId-ify hir::BodyId Another step towards rust-lang#57578.
HirId-ify hir::BodyId Another step towards rust-lang#57578.
Rollup of 10 pull requests Successful merges: - #58110 (libpanic_unwind => 2018) - #58167 (HirId-ify hir::BodyId) - #58202 (Ignore future deprecations in #[deprecated]) - #58272 (Cut down on number formating code size) - #58276 (Improve the error messages for missing stability attributes) - #58354 (Fix ICE and invalid filenames in MIR printing code) - #58381 (Only suggest imports if not imported.) - #58386 (Fix #54242) - #58400 (Fix rustc_driver swallowing errors when compilation is stopped) - #58420 (target/uefi: clarify documentation) Failed merges: r? @ghost
Fix breakage due to rust-lang/rust#58167
It is now a newtype'd HirId, not NodeId. See rust-lang/rust#58167.
It is now a newtype'd HirId, not NodeId. See rust-lang/rust#58167.
It is now a newtype'd HirId, not NodeId. See rust-lang/rust#58167.
Another step towards #57578.