-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Add a method to dump MIR in the middle of MIR building #144780
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @petrochenkov. Use |
I used this while debugging a loop match issue with @folkertdev. I figured it could be useful for others too in the future. |
self.coroutine.clone(), | ||
None, | ||
); | ||
//body.coverage_info_hi = self.coverage_info.clone().map(|b| b.into_done()); |
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.
The coverage info builder is not Clone and I'm not sure if we should implement Clone for it anyway..
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 think it's fine to implement Clone for this, but maybe the people who implemented coverage think differently? cc @Zalathar
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.
Should be fine I think.
I have some other half-formed ideas about how to get the same result, but for this PR I don't mind just slapping #[derive(Clone)]
on a few data structures.
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've posted a draft PR at #145141 that shows how to avoid cloning the builder itself, by making the build method not consume self.
Feel free to incorporate it into this PR if you want, or we can merge it separately, whatever's more convenient for you.
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've added something like your PR except as a separate method to avoid extra allocations when not doing a dump.
r? mir |
This makes it easier to debug issues with MIR building by inserting dump_for_debugging calls around the suspected code responsible for the bad MIR.
2978c66
to
1662a75
Compare
Some changes occurred in coverage instrumentation. cc @Zalathar |
1662a75
to
7287b93
Compare
This comment has been minimized.
This comment has been minimized.
7287b93
to
695473a
Compare
This makes it easier to debug issues with MIR building by inserting dump_for_debugging calls around the suspected code responsible for the bad MIR.