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

Replace all move|| with move || #126631

Closed
wants to merge 1 commit into from

Conversation

Vonr
Copy link
Contributor

@Vonr Vonr commented Jun 18, 2024

There were about 134 instances of move|| across 91 files in the repo.

I have replaced all such instances with move || instead as it better adheres to modern formatting standards.

Ideally, we would have this automated by rustfmt or some other tool, but I do not have the time to implement such a feature or tool. Nonetheless, I would encourage any effort invested into such a tool or feature.

Tests have been run and had their outputs replaced with --bless, I have manually reviewed all the changes to make sure nothing strange was blessed - but I may have made mistakes.

Adheres to modern formatting standards
@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 18, 2024
@Noratrieb
Copy link
Member

Noratrieb commented Jun 18, 2024

oh, UI tests, I missed that you mentioned those, sorry. can you undo the changes to the UI tests? just compiler and library is enough, we don't really format ui tests and I don't think changing them here helps

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we should probably not be mass-changing UI tests unless there's a particularly good reason to do so. There's little value to formatting test artifacts like this, and there's added risk to accidentally changing the intention of the tests unless we understand what the test is actually doing.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2024
Vonr added a commit to Vonr/rust that referenced this pull request Jun 18, 2024
@Vonr
Copy link
Contributor Author

Vonr commented Jun 18, 2024

I have managed to royally screw my local copy of the repo, so I'll do the changes in a new branch and send a new PR #126632

@Vonr Vonr closed this Jun 18, 2024
@Vonr Vonr deleted the fix/moving-closure-formatting branch June 18, 2024 15:37
@Vonr Vonr restored the fix/moving-closure-formatting branch June 18, 2024 15:50
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 18, 2024
…-v2, r=Nilstrieb

Replace `move||` with `move ||`

Edit from rust-lang#126631 to revert changes in `tests/ui`.

There are 18 instances of `move||` across 6 files in the repo:
- `compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs`
- `library/core/src/sync/atomic.rs`
- `library/std/src/sync/condvar.rs`
- `library/std/src/sync/mpsc/mod.rs`
- `library/std/src/sync/barrier.rs`
- `library/std/src/thread/local.rs`

I have replaced all such instances with `move ||` instead as it better adheres to modern formatting standards.

Ideally, we would have this automated by rustfmt or some other tool, but I do not have the time to implement such a feature or tool.
Nonetheless, I would encourage any effort invested into such a tool or feature.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 18, 2024
…-v2, r=Nilstrieb

Replace `move||` with `move ||`

Edit from rust-lang#126631 to revert changes in `tests/ui`.

There are 18 instances of `move||` across 6 files in the repo:
- `compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs`
- `library/core/src/sync/atomic.rs`
- `library/std/src/sync/condvar.rs`
- `library/std/src/sync/mpsc/mod.rs`
- `library/std/src/sync/barrier.rs`
- `library/std/src/thread/local.rs`

I have replaced all such instances with `move ||` instead as it better adheres to modern formatting standards.

Ideally, we would have this automated by rustfmt or some other tool, but I do not have the time to implement such a feature or tool.
Nonetheless, I would encourage any effort invested into such a tool or feature.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 19, 2024
…-v2, r=Nilstrieb

Replace `move||` with `move ||`

Edit from rust-lang#126631 to revert changes in `tests/ui`.

There are 18 instances of `move||` across 6 files in the repo:
- `compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs`
- `library/core/src/sync/atomic.rs`
- `library/std/src/sync/condvar.rs`
- `library/std/src/sync/mpsc/mod.rs`
- `library/std/src/sync/barrier.rs`
- `library/std/src/thread/local.rs`

I have replaced all such instances with `move ||` instead as it better adheres to modern formatting standards.

Ideally, we would have this automated by rustfmt or some other tool, but I do not have the time to implement such a feature or tool.
Nonetheless, I would encourage any effort invested into such a tool or feature.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 19, 2024
…-v2, r=Nilstrieb

Replace `move||` with `move ||`

Edit from rust-lang#126631 to revert changes in `tests/ui`.

There are 18 instances of `move||` across 6 files in the repo:
- `compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs`
- `library/core/src/sync/atomic.rs`
- `library/std/src/sync/condvar.rs`
- `library/std/src/sync/mpsc/mod.rs`
- `library/std/src/sync/barrier.rs`
- `library/std/src/thread/local.rs`

I have replaced all such instances with `move ||` instead as it better adheres to modern formatting standards.

Ideally, we would have this automated by rustfmt or some other tool, but I do not have the time to implement such a feature or tool.
Nonetheless, I would encourage any effort invested into such a tool or feature.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2024
Rollup merge of rust-lang#126632 - Vonr:fix/moving-closure-formatting-v2, r=Nilstrieb

Replace `move||` with `move ||`

Edit from rust-lang#126631 to revert changes in `tests/ui`.

There are 18 instances of `move||` across 6 files in the repo:
- `compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs`
- `library/core/src/sync/atomic.rs`
- `library/std/src/sync/condvar.rs`
- `library/std/src/sync/mpsc/mod.rs`
- `library/std/src/sync/barrier.rs`
- `library/std/src/thread/local.rs`

I have replaced all such instances with `move ||` instead as it better adheres to modern formatting standards.

Ideally, we would have this automated by rustfmt or some other tool, but I do not have the time to implement such a feature or tool.
Nonetheless, I would encourage any effort invested into such a tool or feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants