-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Expand NLL MIR dumps #129711
Expand NLL MIR dumps #129711
Conversation
Also the commit history is a bit tediously granular here. Would appreciate if you squashed this into actual significant pieces changed -- it doesn't help to know that you've separated a commit changing "it's" to "its" rather than just sneaking it into a commit lol clarification: not saying this needs to be 1 commit though; this definitely has a few steps that are worth separating. there's just a lot of tiny nit commits peppered in between that seem unncessary :3 |
Yeah that's true. I review per-commit and thus also tend to move unrelated commits so that they don't pollute the more important ones, not to focus attention to them. Here, it's too granular indeed. I've reworked the history, hopefully it's more to your liking. |
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.
cool ty
r=me when green |
@bors r=compiler-errors |
This comment was marked as resolved.
This comment was marked as resolved.
initially starting with `-Z mir-include-spans` because we want them in the NLL mir dump pass
We want to allow setting this on the CLI, override it only in MIR passes, and disable it altogether in mir-opt tests. The default value is "only for NLL MIR dumps", which is considered off for all intents and purposes, except for `rustc_borrowck` when an NLL MIR dump is requested.
explicitly disable `-Zmir-include-spans` in mir-opt tests This will override the NLL default of true, and keep the blessed dumps easier to work with.
Rebased to fix conflicts. @bors r=compiler-errors |
Expand NLL MIR dumps This PR is a first step to clean up and expand NLL MIR dumps: - by restoring the "mir-include-spans" comments which are useful for `-Zdump-mir=nll` - by adding the list of borrows to NLL MIR dumps, where they are introduced in the CFG and in which region Comments in MIR dumps were turned off in rust-lang#112346, but as shown in rust-lang#114652 they were still useful for us working with NLL MIR dumps. So this PR pulls `-Z mir-include-spans` into its own options struct, so that passes dumping MIR can override them if need be. The rest of the compiler is not affected, only the "nll" pass dumps have these comments enabled again. The CLI still has priority when specifying the flag, so that we can explicitly turn them off in the `mir-opt` tests to keep blessed dumps easier to work with (which was one of the points of rust-lang#112346). Then, as part of a couple steps to improve NLL/polonius MIR dumps and `.dot` visualizations, I've also added the list of borrows and where they're introduced. I'm doing all this to help debug some polonius scope issues in my prototype location-sensitive analysis :3. I'll probably add member constraints soon.
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129711 (Expand NLL MIR dumps) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129733 (Subtree update of `rust-analyzer`) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129785 (Miri subtree update) - rust-lang#129791 (mark joboet as on vacation) Failed merges: - rust-lang#129777 (Add `unreachable_pub`, round 4) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#129659 (const fn stability checking: also check declared language features) - rust-lang#129711 (Expand NLL MIR dumps) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129733 (Subtree update of `rust-analyzer`) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129785 (Miri subtree update) - rust-lang#129791 (mark joboet as on vacation) - rust-lang#129812 (interpret, codegen: tweak some comments and checks regarding Box with custom allocator) Failed merges: - rust-lang#129777 (Add `unreachable_pub`, round 4) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#129659 (const fn stability checking: also check declared language features) - rust-lang#129711 (Expand NLL MIR dumps) - rust-lang#129730 (f32 docs: define 'arithmetic' operations) - rust-lang#129733 (Subtree update of `rust-analyzer`) - rust-lang#129749 (llvm-wrapper: adapt for LLVM API changes) - rust-lang#129757 (Add a test for trait solver overflow in MIR inliner cycle detection) - rust-lang#129760 (Make the "detect-old-time" UI test more representative) - rust-lang#129767 (Remove `#[macro_use] extern crate tracing`, round 4) - rust-lang#129774 (Remove `#[macro_use] extern crate tracing` from rustdoc and rustfmt) - rust-lang#129785 (Miri subtree update) - rust-lang#129791 (mark joboet as on vacation) - rust-lang#129812 (interpret, codegen: tweak some comments and checks regarding Box with custom allocator) Failed merges: - rust-lang#129777 (Add `unreachable_pub`, round 4) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#129711 - lqd:nll-mir-dumps, r=compiler-errors Expand NLL MIR dumps This PR is a first step to clean up and expand NLL MIR dumps: - by restoring the "mir-include-spans" comments which are useful for `-Zdump-mir=nll` - by adding the list of borrows to NLL MIR dumps, where they are introduced in the CFG and in which region Comments in MIR dumps were turned off in rust-lang#112346, but as shown in rust-lang#114652 they were still useful for us working with NLL MIR dumps. So this PR pulls `-Z mir-include-spans` into its own options struct, so that passes dumping MIR can override them if need be. The rest of the compiler is not affected, only the "nll" pass dumps have these comments enabled again. The CLI still has priority when specifying the flag, so that we can explicitly turn them off in the `mir-opt` tests to keep blessed dumps easier to work with (which was one of the points of rust-lang#112346). Then, as part of a couple steps to improve NLL/polonius MIR dumps and `.dot` visualizations, I've also added the list of borrows and where they're introduced. I'm doing all this to help debug some polonius scope issues in my prototype location-sensitive analysis :3. I'll probably add member constraints soon.
This PR is a first step to clean up and expand NLL MIR dumps:
-Zdump-mir=nll
Comments in MIR dumps were turned off in #112346, but as shown in #114652 they were still useful for us working with NLL MIR dumps. So this PR pulls
-Z mir-include-spans
into its own options struct, so that passes dumping MIR can override them if need be. The rest of the compiler is not affected, only the "nll" pass dumps have these comments enabled again. The CLI still has priority when specifying the flag, so that we can explicitly turn them off in themir-opt
tests to keep blessed dumps easier to work with (which was one of the points of #112346).Then, as part of a couple steps to improve NLL/polonius MIR dumps and
.dot
visualizations, I've also added the list of borrows and where they're introduced. I'm doing all this to help debug some polonius scope issues in my prototype location-sensitive analysis :3. I'll probably add member constraints soon.