-
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
Add and use expect methods to hir. #107125
Add and use expect methods to hir. #107125
Conversation
compiler/rustc_hir/src/hir.rs
Outdated
/// Expect an [`ItemKind::ExternCrate`] or panic. | ||
#[track_caller] | ||
pub fn expect_extern_crate(&self) -> Option<Symbol> { | ||
let ItemKind::ExternCrate(s) = self.kind else { unreachable!() }; |
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.
these should probably be bug!
s
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.
even more ideally, they'd say "expected an extern crate, got {:?}
" or something -- makes it much easier at a glance (or a stack trace submitted on an issue) to know what's up
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 don't think bug!
s are available at this point in the crate graph :(
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.
nooooooo
ok well can u at least improve the panic message?
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.
Yes, I'll improve the messages (not today tho, I'm tired and tired)
This comment has been minimized.
This comment has been minimized.
@rustbot ready |
#[track_caller] | ||
fn expect_failed(&self, expected: &'static str) -> ! { | ||
panic!("expected {expected} node, found {self:?}") | ||
} |
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.
This file is way too big and should be split apart into like defs, helpers, expects etc. at some point :(
But it's fine for this PR
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.
yeah, a lot of files are like this :(
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.
:(
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.
Looks good, looking forward to follow up PRs using it in more places :)
@bors r+ |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#107125 (Add and use expect methods to hir.) - rust-lang#107172 (Reimplement NormalizeArrayLen based on SsaLocals) - rust-lang#107177 (Keep all theme-updating logic together) - rust-lang#107424 (Make Vec::clone_from and slice::clone_into share the same code) - rust-lang#107455 (use a more descriptive name) - rust-lang#107465 (`has_allow_dead_code_or_lang_attr` micro refactor) - rust-lang#107469 (Change turbofish context link to an archive link) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
The future has come.
r? @Nilstrieb
tbh I'm not even sure if it's worth it