Skip to content

Use mem::take instead of mem::replace when applicable#4881

Merged
bors merged 8 commits into
rust-lang:masterfrom
krishna-veerareddy:issue-4871-use-mem-take
Jan 4, 2020
Merged

Use mem::take instead of mem::replace when applicable#4881
bors merged 8 commits into
rust-lang:masterfrom
krishna-veerareddy:issue-4871-use-mem-take

Conversation

@krishna-veerareddy

@krishna-veerareddy krishna-veerareddy commented Dec 4, 2019

Copy link
Copy Markdown
Contributor

std::mem::take can be used to replace a value of type T with T::default() instead of std::mem::replace.

Fixes issue #4871

changelog: Added lint for [mem_replace_with_default]

@krishna-veerareddy krishna-veerareddy changed the title Use mem::take instead of mem::replace when applicable [WIP] Use mem::take instead of mem::replace when applicable Dec 4, 2019
@krishna-veerareddy krishna-veerareddy changed the title [WIP] Use mem::take instead of mem::replace when applicable Use mem::take instead of mem::replace when applicable Dec 4, 2019
@flip1995 flip1995 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties A-lint Area: New lints labels Dec 5, 2019
@krishna-veerareddy krishna-veerareddy force-pushed the issue-4871-use-mem-take branch 2 times, most recently from 0f6b46c to 38bb08d Compare December 8, 2019 03:10
Comment thread tests/ui/mem_replace.rs
Comment thread tests/ui/mem_replace.rs
Comment thread tests/ui/mem_replace.rs
@krishna-veerareddy

Copy link
Copy Markdown
Contributor Author

@lzutao Hey I added checks so that it doesn't lint within macros. Also added some macro test cases to prevent regressions in the future.

Comment thread clippy_lints/src/mem_replace.rs
@bors

bors commented Dec 21, 2019

Copy link
Copy Markdown
Contributor

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

@flip1995 flip1995 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Linting in internal macros would be good though. On how to test this see my comment #4881 (comment)

Comment thread clippy_lints/src/mem_replace.rs Outdated
@flip1995

Copy link
Copy Markdown
Member

And sorry for the late reply, I was on vacation.

Comment thread clippy_lints/src/mem_replace.rs Outdated
Comment thread clippy_lints/src/mem_replace.rs Outdated
Comment thread clippy_lints/src/mem_replace.rs Outdated
Comment thread clippy_lints/src/mem_replace.rs Outdated
Comment thread clippy_lints/src/mem_replace.rs Outdated
@tesuji

tesuji commented Dec 22, 2019

Copy link
Copy Markdown
Contributor

If you change

-            if func_args.len() == 2;
+            if let [dest, src] = &**func_args;

@krishna-veerareddy

This comment has been minimized.

@krishna-veerareddy krishna-veerareddy force-pushed the issue-4871-use-mem-take branch 2 times, most recently from 8d1f0fc to 5d8366f Compare December 24, 2019 16:01
Comment thread tests/ui/mem_replace_macro.rs Outdated
@flip1995

Copy link
Copy Markdown
Member

@bors r+

Thanks!

@bors

bors commented Dec 28, 2019

Copy link
Copy Markdown
Contributor

📌 Commit aca6815 has been approved by flip1995

@bors

bors commented Dec 28, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit aca6815 with merge 31eb751...

bors added a commit that referenced this pull request Dec 28, 2019
…lip1995

Use `mem::take` instead of `mem::replace` when applicable

`std::mem::take` can be used to replace a value of type `T` with `T::default()` instead of `std::mem::replace`.

Fixes issue #4871

changelog: Added lint for [`mem_replace_with_default`]
@bors

bors commented Dec 28, 2019

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-travis

@flip1995

Copy link
Copy Markdown
Member

Expr -> Expr<'_>

@bors

bors commented Dec 30, 2019

Copy link
Copy Markdown
Contributor

💔 Test failed - status-appveyor

@krishna-veerareddy

krishna-veerareddy commented Dec 30, 2019

Copy link
Copy Markdown
Contributor Author

@flip1995 @phansch Hey there was another rustup fix required so rebased again.

@krishna-veerareddy

Copy link
Copy Markdown
Contributor Author

@phansch @flip1995 Rebased after rustup. Please review when you have some time. Thanks!

@phansch

phansch commented Jan 4, 2020

Copy link
Copy Markdown
Contributor

@bors r=flip1995

@bors

bors commented Jan 4, 2020

Copy link
Copy Markdown
Contributor

📌 Commit 42e4595 has been approved by flip1995

@bors

bors commented Jan 4, 2020

Copy link
Copy Markdown
Contributor

⌛ Testing commit 42e4595 with merge fa9b85d...

bors added a commit that referenced this pull request Jan 4, 2020
…lip1995

Use `mem::take` instead of `mem::replace` when applicable

`std::mem::take` can be used to replace a value of type `T` with `T::default()` instead of `std::mem::replace`.

Fixes issue #4871

changelog: Added lint for [`mem_replace_with_default`]
@bors

bors commented Jan 4, 2020

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing fa9b85d to master...

@bors bors merged commit 42e4595 into rust-lang:master Jan 4, 2020
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 8, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 8, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 8, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 8, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 9, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 9, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a added a commit to ada4a/rust-clippy that referenced this pull request Jun 9, 2026
This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the regular test files --
both the std and the no-std one.

[^1]: rust-lang#4881 (comment)
ada4a pushed a commit to ada4a/rust-clippy that referenced this pull request Jun 9, 2026
…17191)

This was originally not done because emitting suggestions in macros used
to not work well.[^1] That's fortunately no longer the case.

This allows folding `mem_replace_with_default_macro` into the other test
files -- both the regular and the no-std one.

changelog: [`mem_replace_with_default`]: also emit inside macros

[^1]:
rust-lang#4881 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-lint Area: New lints 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.

5 participants