#![custom_mir]: Various improvements
#104975
Conversation
rustbot
added
S-waiting-on-review
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
JakobDegen
force-pushed
the
custom_mir_let
branch
from
0d2d784
to
58f39b3
Compare
JakobDegen
force-pushed
the
custom_mir_let
branch
from
58f39b3
to
885250b
Compare
JakobDegen
changed the title
let statements in custom mir#![custom_mir]: Various improvements
| if self.is_custom_mir { | ||
| kind | ||
| } else { | ||
| ExprKind::Deref { | ||
| arg: self.thir.exprs.push(Expr { | ||
| ty, | ||
| temp_lifetime, | ||
| span: expr.span, | ||
| kind, | ||
| }), | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't like this too much, can we revert it once &Static(Name) is a thing?
There was a problem hiding this comment.
Yeah, actually, I can just revert it immediately. I can manually parse and ignore the deref instead
There was a problem hiding this comment.
Reverted the changes to this part of the code and pushed an additional commit that has the alternative to this. Should be much better, not sure why I didn't do this in the first place
JakobDegen
force-pushed
the
custom_mir_let
branch
from
885250b
to
30e0f8a
Compare
|
@bors delegate+ r=me with the new commit squashed into its parent |
|
✌️ @JakobDegen can now approve this pull request |
|
☔ The latest upstream changes (presumably #105012) made this pull request unmergeable. Please resolve the merge conflicts. |
JakobDegen
force-pushed
the
custom_mir_let
branch
from
30e0f8a
to
5a34dbf
Compare
|
@bors r=oli-obk |
bors
added
S-waiting-on-bors
`#![custom_mir]`: Various improvements
This PR makes a bunch of improvements to `#![custom_mir]`. Ideally this would be 4 PRs, one for each commit, but those would take forever to get merged and be a pain to juggle. Should still be reviewed one commit at a time though.
### Commit 1: Support arbitrary `let`
Before this change, all locals used in the body need to be declared at the top of the `mir!` invocation, which is rather annoying. We attempt to change that.
Unfortunately, we still have the requirement that the output of the `mir!` macro must resolve, typecheck, etc. Because of that, we can't just accept this in the THIR -> MIR parser because something like
```rust
{
let x = 0;
Goto(other)
}
other = {
RET = x;
Return()
}
```
will fail to resolve. Instead, the implementation does macro shenanigans to find the let declarations and extract them as part of the `mir!` macro. That *works*, but it is fairly complicated and degrades debuginfo by quite a bit. Specifically, the spans for any statements and declarations that are affected by this are completely wrong. My guess is that this is a net improvement though.
One way to recover some of the debuginfo would be to not support type annotations in the `let` statements, which would allow us to parse like `let $stmt:stmt`. That seems quite surprising though.
### Commit 2: Parse consts
Reuses most of the const parsing from regular Mir building for building custom mir
### Commit 3: Parse statics
Statics are slightly weird because the Mir primitive associated with them is a reference/pointer to them, so this is factored out separately.
### Commit 4: Fix some spans
A bunch of the spans were non-ideal, so we adjust them to be much more helpful.
r? `@oli-obk`
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (9c0bc30): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
|
This PR makes a bunch of improvements to
#![custom_mir]. Ideally this would be 4 PRs, one for each commit, but those would take forever to get merged and be a pain to juggle. Should still be reviewed one commit at a time though.Commit 1: Support arbitrary
letBefore this change, all locals used in the body need to be declared at the top of the
mir!invocation, which is rather annoying. We attempt to change that.Unfortunately, we still have the requirement that the output of the
mir!macro must resolve, typecheck, etc. Because of that, we can't just accept this in the THIR -> MIR parser because something likewill fail to resolve. Instead, the implementation does macro shenanigans to find the let declarations and extract them as part of the
mir!macro. That works, but it is fairly complicated and degrades debuginfo by quite a bit. Specifically, the spans for any statements and declarations that are affected by this are completely wrong. My guess is that this is a net improvement though.One way to recover some of the debuginfo would be to not support type annotations in the
letstatements, which would allow us to parse likelet $stmt:stmt. That seems quite surprising though.Commit 2: Parse consts
Reuses most of the const parsing from regular Mir building for building custom mir
Commit 3: Parse statics
Statics are slightly weird because the Mir primitive associated with them is a reference/pointer to them, so this is factored out separately.
Commit 4: Fix some spans
A bunch of the spans were non-ideal, so we adjust them to be much more helpful.
r? @oli-obk