Skip to content

mem_replace: clean-up#17190

Merged
samueltardieu merged 6 commits into
rust-lang:masterfrom
ada4a:cleanup-mem-replace
Jun 9, 2026
Merged

mem_replace: clean-up#17190
samueltardieu merged 6 commits into
rust-lang:masterfrom
ada4a:cleanup-mem-replace

Conversation

@ada4a

@ada4a ada4a commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This ended up being bigger than I expected...

This PR mainly does the following:

  • Commit 2 expands on Attributes cleanup in tests [12/20] #17096 by cleaning up the attributes, and cleans up some more.
  • mem_replace.rs used to test mem_replace_{option_with_none,option_with_some,default}, which resulted in a rather big and messy file. Commits 4-6 split it into three.
  • Meanwhile, mem_replace_with_uninit was tested in a completely separate file, called repl_uninit (?!). Commit 3 renames it and cleans it up.

As always, see the individual commits for more details.

changelog: [mem_replace_option_with_some]: clean-up the lint message
changelog: [mem_replace_option_with_none]: clean-up the lint message
changelog: [mem_replace_with_uninit]: clean-up the lint message
changelog: [mem_replace_with_default]: split off the suggestion from the main message

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 8, 2026
@rustbot

rustbot commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

r? @samueltardieu

rustbot has assigned @samueltardieu.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 8 candidates
  • 8 candidates expanded to 8 candidates
  • Random selection from Jarcho, dswij, llogiq, samueltardieu

@rustbot rustbot added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Jun 8, 2026
@rustbot

This comment has been minimized.

Sign in to view
@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jun 8, 2026
@ada4a ada4a mentioned this pull request Jun 8, 2026
Merged
@ada4a

ada4a commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@samueltardieu I could alternatively just include the attribute clean-up in this PR -- that would have the benefit of making this PR more standalone, but would require you to rebase your PR on top of this one later. Which option would you prefer?

@github-actions

github-actions Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Lintcheck changes for 2ae2877

Lint Added Removed Changed
clippy::mem_replace_option_with_some 0 0 5
clippy::mem_replace_with_default 0 0 10

This comment will be updated if you push new changes

@ada4a ada4a mentioned this pull request Jun 8, 2026
Open
@samueltardieu

samueltardieu commented Jun 8, 2026

Copy link
Copy Markdown
Member

@samueltardieu I could alternatively just include the attribute clean-up in this PR -- that would have the benefit of making this PR more standalone, but would require you to rebase your PR on top of this one later. Which option would you prefer?

No preference. If you make this PR standalone, just ping me and I'll review it and rebase the other once merged.

@ada4a ada4a force-pushed the cleanup-mem-replace branch from 47cb899 to 99f6d5c Compare June 8, 2026 20:43
@rustbot

rustbot commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jun 8, 2026
@ada4a ada4a removed the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Jun 8, 2026
@ada4a ada4a marked this pull request as draft June 8, 2026 20:44
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 8, 2026
@ada4a ada4a force-pushed the cleanup-mem-replace branch from 99f6d5c to a12f9cf Compare June 8, 2026 20:45
@ada4a ada4a marked this pull request as ready for review June 8, 2026 20:45
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 8, 2026
@ada4a

ada4a commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

If you make this PR standalone, just ping me

Okay, did that. Ping-ping!

@ada4a

ada4a commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

huh, what is this repl_uninit... let me investigate a bit

@ada4a ada4a marked this pull request as draft June 8, 2026 20:50
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 8, 2026
@ada4a ada4a force-pushed the cleanup-mem-replace branch from a12f9cf to 09a9e13 Compare June 8, 2026 22:32
@ada4a ada4a marked this pull request as ready for review June 8, 2026 22:39
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 8, 2026
@ada4a
clean-up suggestion messages
1389f57 
- unify wording
- `mem_replace_with_default`: split off the suggestion message from the
  main message
- mention the method to use in the suggestion message as well -- the
  suggestion message should be understandable on its own, without the diff
@ada4a ada4a force-pushed the cleanup-mem-replace branch 3 times, most recently from abfe3f2 to b44613c Compare June 8, 2026 23:11
samueltardieu
samueltardieu requested changes Jun 9, 2026
View reviewed changes
Comment thread tests/ui/mem_replace_option_with_none_no_std.fixed Outdated
Comment thread tests/ui/mem_replace_option_with_none_no_std.fixed Outdated
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 9, 2026
@rustbot

rustbot commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

ada4a added 2 commits June 9, 2026 11:54
@ada4a
clean-up tests, first pass
ef6e711 
- remove unused attributes
- add `warn`s for _all_ the lints tested
- remove `fn main` from the no-std test, so that it's considered a lib
  and thus doesn't need the no-std boilerplate
- use shorthand error patterns
@ada4a
rename repl_uninit.rs to mem_replace_with_uninit.rs, clean-up
a78f667 
- Turns out to be a test for `mem_replace_with_uninit`
- Extract the one unfixable case into a separate file
- Clean-up attributes, now with comments!
@ada4a ada4a force-pushed the cleanup-mem-replace branch from b44613c to d9433e7 Compare June 9, 2026 10:08
ada4a added 3 commits June 9, 2026 12:13
@ada4a
extract mem_replace_option_with_none tests into their own file
a5b5cef 
Note that this doesn't migrate the no-std test for issue9824. The
motivation is that the no-std file is usually supposed to just test that
the basic functionality of the lint works in no-std, which boils down to
`std` being properly replaced with `core`. Bringing a reproducer for a
separate issue thus doesn't add much value.
@ada4a
extract mem_replace_option_with_some tests into their own file
d9825ad
@ada4a
rename the remains to mem_replace_with_default.rs
2ae2877
@ada4a ada4a force-pushed the cleanup-mem-replace branch from d9433e7 to 2ae2877 Compare June 9, 2026 10:15
@ada4a

ada4a commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Sorry for the second force-push, here's the complete diff: https://github.com/rust-lang/rust-clippy/compare/b44613c3e583e9fcfb1a8def8f38e2082dc16413..2ae28777f3ecde1311f3b39e72d5332bad430e1c

@samueltardieu samueltardieu added this pull request to the merge queue Jun 9, 2026
Merged via the queue into rust-lang:master with commit 75bff05 Jun 9, 2026
11 checks passed
@ada4a ada4a deleted the cleanup-mem-replace branch June 9, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samueltardieu samueltardieu samueltardieu approved these changes

Assignees

@samueltardieu samueltardieu

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ada4a @rustbot @samueltardieu