Teach rustc --emit=mir by shepmaster · Pull Request #39891 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation30 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
I'm opening this PR to discuss:
- Is this a good idea?
- Is this a good implementation?
I'm sure people will have opinions on both points!
This spawned from #31847 (comment), so I figured a prototype implementation could help provide a seed to talk about.
(rust_highfive has picked a reviewer for you, use r? to override)
So this seems to fall under a broad category of options that we currently confine to -Z but which I think we may want to expose on stable. I'd classify these as "debugging" or "educational" options -- these would include things like the pretty-printer, dumping MIR, profiling information, or other ways to view internal compiler state. The common thread here is that the output would be completely unstable and subject to change -- that is, it is meant for human viewers only (e.g., like diagnostics output).
I sort of suspect that such a family of options should have their own header (rather like -C), and not be conflated with things like --emit, which are used to produce (primarily, anyway) output that can be fed into other tools.
I'd classify these as "debugging" or "educational" options
This seems like a reasonable categorization.
the output would be completely unstable and subject to change
Absolutely.
I sort of suspect that such a family of options should have their own header (rather like
-C)
This makes sense to me
not be conflated with things like
--emit, which are used to produce (primarily, anyway) output that can be fed into other tools.
I suspect I fall into the target group of that "primarily" 😇 . My personal stake is that I'd like to have a stable and unified way of accessing the final MIR, ASM, and LLVM-IR for the playground. In that context, all three of these are debugging/educational, often for the purposes of "how does that look when optimized?".
We could probably find a better name for --pretty (or rather, --unpretty).
In fact, we have --print which can output a dozen useful strings.
What if we gave that -P and also used it for pretty-printing? That is, -P pretty-source or something.
@eddyb seems reasonable to me. I'm also not opposed to using --emit for this purpose, I guess, but it does seem like there is a definite distinction between "object code" and "MIR" or "pretty-printed HIR".
So I was looking into --print. The current list of things to print seems (to me) to be targeting tool consumption, right?
--print [crate-name|file-names|sysroot|cfg|target-list|target-cpus|target-features|relocation-models|code-models|target-spec-json]
Comma separated list of compiler information to print
Is that then the right option? Does it matter if we mix "tool"-targeting things from "human"-targeting things under one option? It feels like it will encourage misuse, but perhaps not.
FWIW --print is generally "this goes to stdout" where --emit is "this goes to a file", and this seems like it's intending to be the latter?
@alexcrichton I definitely agree --print is better than --emit. Just wondering if it's the right thing to use, in terms of discouraging people from casually thinking the output is stable. Maybe it's good enough.
Or maybe those things aren't especially stable either?
I'd personally be ok with a giant header comment saying "THIS OUTPUT IS NOT STABLE IT'S JUST MEANT FOR DEBUGGING, IT WILL CHANGE WITHOUT NOTIFICATION" or something like that.
@alexcrichton where would such a comment go? Would it apply to all the things that --print uses?
Right now the full list shows up in --help. I was thinking maybe it should not, and we should instead have some way to get the list. This list could then make clear (perhaps with an asterix) those items where the output format is not, and will likely never be, stable. But at the end of the day I kind of just feel is has to be written somewhere that is not TOO obscure, and it's fine. Which I think is your point.
Oh sorry I misread what you preferred. I assumed that you preferred --emit (like me) in which case such a comment would go in the file.
If you prefer --print then yeah there's not really a place for such a comment.
@alexcrichton oh, like, put it in the output itself? That's...kind of a good idea. =)
Just add something like to the front:
// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
Regarding --emit vs --print I don't honestly care that much.
Relevant to the compiler team, which will review and decide on the PR/issue.
label
@rfcbot fcp merge
I propose we adopt @alexcrichton's suggestion here, and support --emit mir (and other such unstable-y things) but we prefix the output with some comments warning people from relying on any particular detail of the format. We can do the same with the pretty printer, etc. (Feel free to object if you prefer something other than --emit or have any other concerns. I'm just trying to move things along here.)
In the final comment period and will be merged soon unless new substantive objections are raised.
label
🔔 This is now entering its final comment period, as per the review above. 🔔
☔ The latest upstream changes (presumably #39628) made this pull request unmergeable. Please resolve the merge conflicts.
Nothing scarier than your little ol' PR getting a merge conflict from a hulking +2,797 −3,169 PR. Thankfully, it didn't seem to have any actual conflicts...
To prevent tools from using this you could put spaces/dots/underscores at random places.
@bjorn3 Any half-decent lexer can remove extra spaces, and if it impedes a lexer, it also does humans.
@nikomatsakis thank you for the heads up. The build is green again.
Mm I'm going to r+ even though the FCP has not yet expired. I think we've had enough time here.
📌 Commit 4ddedf7 has been approved by nikomatsakis
frewsxcv added a commit to frewsxcv/rust that referenced this pull request
Teach rustc --emit=mir
I'm opening this PR to discuss:
- Is this a good idea?
- Is this a good implementation?
I'm sure people will have opinions on both points!
This spawned from rust-lang#31847 (comment), so I figured a prototype implementation could help provide a seed to talk about.
bors added a commit that referenced this pull request
nateozem pushed a commit to nateozem/rust that referenced this pull request
…n doc.
This is added because 'rustc' can now generate MIR (referencing to "Teach rustc --emit=mir rust-lang#39891").
frewsxcv added a commit to frewsxcv/rust that referenced this pull request
add 'mir' to rustc help menu and man doc
add 'mir' to '--emit' flag list for 'rustc'. This is added because 'rustc' can now generate MIR (referencing to "Teach rustc --emit=mir rust-lang#39891").
Labels
In the final comment period and will be merged soon unless new substantive objections are raised.
Relevant to the compiler team, which will review and decide on the PR/issue.