-
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
Fix crash due to CrateItem::kind()
not handling constructors
#119135
Conversation
r? @ouz-a (rustbot has picked a reviewer for you, use r? to override) |
This PR changes Stable MIR cc @oli-obk, @celinval, @spastorino, @ouz-a |
compiler/stable_mir/src/mir/mono.rs
Outdated
@@ -142,6 +147,7 @@ impl Debug for Instance { | |||
f.debug_struct("Instance") | |||
.field("kind", &self.kind) | |||
.field("def", &self.mangled_name()) | |||
.field("args", &self.args()) |
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.
In the future, please pull unrelated changes at least into a separate commit, but ideally into a separate PR. This has nothing to do with what's mentioned in the title of the PR, which is a one-liner 😓
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.
Fair. Sorry about that. I wasn't sure if you had a preference.
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.
No worries, just surprising to see so many changes when I expected a one-liner 😆
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 created #119141 for that, and I'll remove it from here.
b9db38f
to
1eef17e
Compare
Actually, sorry for the back and forth. I looked a bit deeper -- I don't think that unit structs have a body at all. Maybe let's leave this |
Maybe it would make sense if I understood a bit about how you encountered this crash -- if you got it from a |
Just to make sure I understand, To provide more context, I added a test that crashes without this fix, but succeeds after the fix. The test was crashing with the following 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.
Hmmm... maybe we should just make a new ItemKind::Ctor(CtorKind::{Fn, Const})
?
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.
r=me when ci is green
@bors r+ rollup |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#118691 (Add check for possible CStr literals in pre-2021) - rust-lang#118973 (rustc_codegen_ssa: Don't drop `IncorrectCguReuseType` , make `rustc_expected_cgu_reuse` attr work) - rust-lang#119071 (-Znext-solver: adapt overflow rules to avoid breakage) - rust-lang#119089 (effects: fix a comment) - rust-lang#119094 (Add function ABI and type layout to StableMIR) - rust-lang#119102 (Add arm-none-eabi and armv7r-none-eabi platform-support documentation.) - rust-lang#119107 (subtype_predicate: remove unnecessary probe) Failed merges: - rust-lang#119135 (Fix crash due to `CrateItem::kind()` not handling constructors) - rust-lang#119141 (Add method to get instance instantiation arguments) r? `@ghost` `@rustbot` modify labels: rollup
☔ The latest upstream changes (presumably #119156) made this pull request unmergeable. Please resolve the merge conflicts. |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
Change how we classify item kind for DefKind::Ctor
79ff316
to
7ab38b8
Compare
@rustbot ready |
@bors r+ |
…to use StableMIR (#2959) We still need internal APIs to handle things like: - **FnAbi:** Support is pending this PR to be merged: rust-lang/rust#119094 - **Filter non-function items:** Also pending a fix on the rust side: rust-lang/rust#119135 - **Checking for reachable functions:** This is not needed in theory, but std lib build fails since it unncovers an existing issue in Kani with a function that was not previously included. I'll create an issue for that. It's unrelated to this change though. - **Function attribute handling** - **Span error** Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#119135 (Fix crash due to `CrateItem::kind()` not handling constructors) - rust-lang#119141 (Add method to get instance instantiation arguments) - rust-lang#119145 (Give `VariantData::Struct` named fields, to clairfy `recovered`.) - rust-lang#119167 (E0761: module directory has .rs suffix) - rust-lang#119168 (resolve: Stop feeding visibilities for import list stems) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#119135 - celinval:smir-small-changes, r=compiler-errors Fix crash due to `CrateItem::kind()` not handling constructors Also add a method to get the instance instantiation arguments, and include that information in the instance debug.
Also add a method to get the instance instantiation arguments, and include that information in the instance debug.