Skip to content
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

write_mir_pretty no longer works for non local def_ids #81200

Closed
hermanventer opened this issue Jan 19, 2021 · 12 comments · Fixed by #81965
Closed

write_mir_pretty no longer works for non local def_ids #81200

hermanventer opened this issue Jan 19, 2021 · 12 comments · Fixed by #81965
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@hermanventer
Copy link

hermanventer commented Jan 19, 2021

The expect_local call here means that write_mir_pretty no longer works for non local def_ids. The ability to print out MIR for non local functions this way is critical for the MIRAI project.

Originally posted by @hermanventer in #78407 (comment)

@camelid camelid added the A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html label Jan 20, 2021
@RalfJung
Copy link
Member

Cc @oli-obk

@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jan 20, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Jan 20, 2021

I think what we can do is to check is_const_fn_raw for the const fn path, and use tcx.instance_mir(ty::InstanceDef::Item(def_id)) if it is not a const fn, as instance_mir already does the correct dispatching for anything that is not a const fn.

@Rustin170506
Copy link
Member

Can I working on this?

@hermanventer
Copy link
Author

That would be great. This is a blocker for me.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 2, 2021

It's all yours! You can use @rustbot claim to have the bot assign it to you

@Rustin170506
Copy link
Member

@rustbot claim

@oli-obk
Copy link
Contributor

oli-obk commented Feb 2, 2021

If you need any help, don't hesitate to ask here or ping me on zulip. I'll respond whenever I have time (which is a bit flaky this week, but you should definitely get a response once a day)

@Rustin170506 Rustin170506 removed their assignment Feb 7, 2021
@Rustin170506
Copy link
Member

Sorry, I haven't had time to finish it lately. Let's see if others can help.

@osa1
Copy link
Contributor

osa1 commented Feb 9, 2021

Do we have a reproducer for this issue? I have some idea of what needs to be done here, but I don't know how to check if it really works.

@hermanventer
Copy link
Author

The only way I know of how to do this is to write something that uses the compiler driver, see for example: https://github.com/rust-lang/rustc-dev-guide/tree/master/examples. That seems like quite a lot of trouble to go to. More ideal would be some kind of unit test, but I have no idea of how that may be accomplished.

Perhaps the easiest way to do an ad hoc test would be to build MIRAI with the latest nightly (I'd have to put up a branch that has been ported to it) and then run its tests.

@hermanventer
Copy link
Author

I've uploaded a draft PR to facebookexperimental/MIRAI#754. If you build that with a recent nightly and then run cargo test, you should see failures. If your fix is good, then all tests should pass.

@osa1
Copy link
Contributor

osa1 commented Feb 10, 2021

Thanks @hermanventer, that's very helpful. I was able to run the tests successfully using the master branch, but if I use my stage 1 build with your branch some of the tests fail, and the backtraces include write_mir_pretty so I'm guessing those failing tests are reproducers for this issue.

@estebank estebank added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 11, 2021
@bors bors closed this as completed in 2efde8c Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants