Skip to content

Simplify switch sources #136959

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 5 commits into from
Feb 18, 2025
Merged

Simplify switch sources #136959

merged 5 commits into from
Feb 18, 2025

Conversation

nnethercote
Copy link
Contributor

SwitchSources and the code around it can be simplified.

r? @tmiasko

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 13, 2025
@nnethercote
Copy link
Contributor Author

As per the "useful comment" in the second last commit, SwitchSources is never actually instantiated right now, but I figure it's still good to simplify code and improve documentation, if only so some future person doesn't spend time working this stuff out the way I did.

@tmiasko
Copy link
Contributor

tmiasko commented Feb 13, 2025

SwitchSources is a hashmap containing SmallVec<[Option<u128>; 1]> elements. Each SmallVec has a predictable form: N Some elements followed by a single None element.

Elements from a particular SmallVec describe switch values with the same target block. The None element will be present only in a SmallVec describing a target of an otherwise branch.

@nnethercote nnethercote force-pushed the simplify-SwitchSources branch from c2720aa to d53670f Compare February 14, 2025 03:38
@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2025

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @vakaras

@nnethercote
Copy link
Contributor Author

The None element will be present only in a SmallVec describing a target of an otherwise branch.

Hmm, good point. I misunderstood that part.

I have removed some commits and added some new ones. This code is a bit tricky and I think it's good to make it simpler and clearer.

@bors
Copy link
Collaborator

bors commented Feb 14, 2025

☔ The latest upstream changes (presumably #137030) made this pull request unmergeable. Please resolve the merge conflicts.

@nnethercote
Refactor apply_effects_in_block.
1300310 
Very minor changes that will make the next few commits easier to follow.
@nnethercote
Add a useful comment.
23dbff8
@nnethercote
Add SwitchTargetValue.
8403d39 
This is much clearer than `Option<u128>`.
@nnethercote
Update and clarify the comment on SwitchTargets.
db1ca60
@nnethercote
Remove SwitchIntTarget.
3b81d9d 
It's only passed to `Analysis::apply_switch_int_edge_effect`, and the
existing impls of that method only use the `value` field. So pass that
instead.
@nnethercote nnethercote force-pushed the simplify-SwitchSources branch from d53670f to 3b81d9d Compare February 16, 2025 23:04
@nnethercote
Copy link
Contributor Author

I have addressed the comments, so this code is ready for final review, thanks.

@tmiasko
Copy link
Contributor

tmiasko commented Feb 17, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 17, 2025

📌 Commit 3b81d9d has been approved by tmiasko

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 17, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 17, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@bors
Auto merge of rust-lang#137176 - matthiaskrgr:rollup-eht05gr, r=matth…
de91711 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#136959 (Simplify switch sources)
 - rust-lang#137020 (Pass vendored sources from bootstrap to generate-copyright)
 - rust-lang#137073 (boostrap: skip no_std targets in Std doc step)
 - rust-lang#137165 (Use `tell` for `<File as Seek>::stream_position`)
 - rust-lang#137166 (Update default loongarch code model in docs)
 - rust-lang#137168 (correct comment)
 - rust-lang#137169 (CI: rfl: move job forward to Linux v6.14-rc3)
 - rust-lang#137170 (Allow configuring jemalloc per target)
 - rust-lang#137173 (Subtree update of `rust-analyzer`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 005de38 into rust-lang:master Feb 18, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 18, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2025
@rust-timer
Unrolled build for rust-lang#136959
18a3746 
Rollup merge of rust-lang#136959 - nnethercote:simplify-SwitchSources, r=tmiasko

Simplify switch sources

`SwitchSources` and the code around it can be simplified.

r? `@tmiasko`
@nnethercote nnethercote deleted the simplify-SwitchSources branch February 18, 2025 03:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@tmiasko tmiasko tmiasko left review comments

Assignees

@tmiasko tmiasko

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nnethercote @tmiasko @rustbot @bors @RalfJung