Skip to content
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

Remove unnecessary res field in for_each_expr visitors #13156

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

y21
Copy link
Member

@y21 y21 commented Jul 25, 2024

Small refactor in the for_each_expr* visitors. This should not change anything functionally.

Instead of storing the final value Option<B> in the visitor and setting it to Some when we get a ControlFlow::Break(B) from the closure, we can just directly return it from the visitor itself now that visitors support that.

cc #12829 and #12830 (comment)

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2024

r? @xFrednet

rustbot has assigned @xFrednet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 25, 2024
@y21
Copy link
Member Author

y21 commented Jul 25, 2024

Lintcheck seems to agree. Very nice to have for these kinds of refactors

@xFrednet
Copy link
Member

LGTM, nice refactoring!

Lintcheck seems to agree. Very nice to have for these kinds of refactors

And yes, it is, it's insane how quickly it has become part of my workflow :3

cc: @blyxyas because of #12829 in case you're interested


Roses are red, Continue(())
Violets are blue, Continue(())
The visitor is visiting, Continue(())
Until now Break(())

@bors
Copy link
Contributor

bors commented Jul 25, 2024

📌 Commit de1e163 has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 25, 2024

⌛ Testing commit de1e163 with merge c7cfe7f...

@bors
Copy link
Contributor

bors commented Jul 25, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing c7cfe7f to master...

@bors bors merged commit c7cfe7f into rust-lang:master Jul 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants