-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Show absolute line numbers if span is outside relative span #99908
Conversation
src/test/mir-opt/const_promotion_extern_static.BAR.PromoteTemps.diff
Outdated
Show resolved
Hide resolved
let mut _0: (); // return place in scope 0 at $DIR/generator-storage-dead-unwind.rs:+0:19: +0:19 | ||
let _3: Foo; // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+1:13: +1:14 | ||
let _5: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+3:9: +3:14 | ||
let mut _6: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+3:9: +3:14 | ||
let _7: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+4:9: +4:16 | ||
let mut _8: Foo; // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+4:14: +4:15 | ||
let _9: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+5:9: +5:16 | ||
let mut _10: Bar; // in scope 0 at $DIR/generator-storage-dead-unwind.rs:+5:14: +5:15 | ||
let mut _0: (); // return place in scope 0 at $DIR/generator-storage-dead-unwind.rs:22:19: 22:19 | ||
let _3: Foo; // in scope 0 at $DIR/generator-storage-dead-unwind.rs:23:13: 23:14 | ||
let _5: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:25:9: 25:14 | ||
let mut _6: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:25:9: 25:14 | ||
let _7: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:26:9: 26:16 | ||
let mut _8: Foo; // in scope 0 at $DIR/generator-storage-dead-unwind.rs:26:14: 26:15 | ||
let _9: (); // in scope 0 at $DIR/generator-storage-dead-unwind.rs:27:9: 27:16 | ||
let mut _10: Bar; // in scope 0 at $DIR/generator-storage-dead-unwind.rs:27:14: 27:15 |
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.
Same thing for closures, their main span only points to the ||
at the start
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.
Doing it for consts and statics was fairly simple. But also doing it for closures not only caused big diffs in the ui tests, but also regressed a borrowck diagnostic in src/test/ui/borrowck/issue-93093.rs
where it didn't suggest making the parameter of an async function &mut self
anymore because the changes in the spans somehow affected it. Unless you have a quick idea, I would prefer not doing that in this PR, and staying with consts and statics for now.
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #100087) made this pull request unmergeable. Please resolve the merge conflicts. |
This is a little bitrotty, I'll reroll to get it through r? mir-opt |
That doesn't work either, I guess I'll just assign someone directly |
📌 Commit ab883c4b51b557855d13498dd184112bf1c3215d has been approved by It is now in the queue for this repository. |
⌛ Testing commit ab883c4b51b557855d13498dd184112bf1c3215d with merge 5003bd390ff07eed19b30f0c05c43aced8d0200f... |
💔 Test failed - checks-actions |
This comment has been minimized.
This comment has been minimized.
Huh, interesting, that's not even a 32-bit or profiling test. I'll try to fix it. |
In the MIR pretty printing, it can sometimes happen that the span of the statement is outside the span of the body (for example through inlining). In this case, don't display a relative span but an absolute span. This will make the mir-opt-tests a little more prone to diffs again, but the impact should be small.
ab883c4
to
5d7ce21
Compare
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (e0dc8d7): 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.
Footnotes |
In the MIR pretty printing, it can sometimes happen that the span of the statement is outside the span of the body (for example through inlining). In this case, don't display a relative span but an absolute span. This will make the mir-opt-tests a little more prone to diffs again, but the impact should be small.
Fixes #99854
r? @oli-obk