Make TerminatorKind::Call::func Spanned and remove ::fn_span
#120114
Conversation
To get structural symmetry between `TerminatorKind::Call::func` and `TerminatorKind::Call::args`, make `func` `Spanned`, since `args` already is `Spanned`. This way we can remove the separate `fn_span` field. This is a separate commit to make review easy. This commit does not build. The next commit makes no semantic changes, it only makes the code compile again.
To make review easier, this commit does not make any semantic changes. It only makes the previous commit build.
|
(rustbot has picked a reviewer for you, use r? to override) |
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Make `TerminatorKind::Call::func` `Spanned` and remove `::fn_span` To get structural symmetry between `TerminatorKind::Call::func` and `TerminatorKind::Call::args`, make `func` `Spanned`, since `args` already is `Spanned`. This way we can remove the separate `fn_span` field. Doing this was briefly discussed [here](rust-lang#116520 (comment)). For easier review, this PR is split into two commits. The first commit is the actual change. The second commit just makes the code build again and is very repetitive. Can someone start a perf run of this change please?
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (0907f12): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never 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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 666.403s -> 665.929s (-0.07%) |
| func: Operand<'tcx>, | ||
| /// The function that’s being called, including the `Span` of the | ||
| /// function, without the dot and receiver | ||
| /// e.g. `foo(a, b)` in `x.foo(a, b)` |
There was a problem hiding this comment.
Now that we have separate spans for the arguments, wouldn't it make a lot more sense to make this just the foo? That's what I would expect from a Spanned<Operand> -- that the span only covers that operand.
What this PR does is confusing.
There was a problem hiding this comment.
Hmm, good point. Since that is not as straightforward, I'll leave that for the future and go ahead and close this.
Thanks for taking a look!
To get structural symmetry between
TerminatorKind::Call::funcandTerminatorKind::Call::args, makefuncSpanned, sinceargsalready isSpanned. This way we can remove the separatefn_spanfield.Doing this was briefly discussed here.
For easier review, this PR is split into two commits. The first commit is the actual change. The second commit just makes the code build again and is very repetitive.
Can someone start a perf run of this change please?