Skip to content

Conversation

y21
Copy link
Member

@y21 y21 commented Oct 10, 2023

Closes #11648

Detects the pattern &String::from_utf8(bytes.to_vec()).unwrap() and suggests core::str::from_utf8(bytes).unwrap(), which avoids the unnecessary intermediate allocation.

I decided to put this in the existing unnecessary_to_owned lint (rather than creating a new lint) for a few reasons:

  • we get to use some of its logic (for example, recognizing any of the functions in the to_owned family, e.g. to_vec)
  • the actual inefficient operation that can be avoided here is the call to .to_vec(), so this is in a way similar to the other cases caught by unnecessary_to_owned, just through a bunch of type conversions
  • we can make this more "generic" later and catch other cases, so imo it's best not to tie this lint specifically to the String type

changelog: [unnecessary_to_owned]: catch &String::from_utf8(bytes.to_vec()).unwrap() and suggest core::str::from_utf8(bytes).unwrap()

@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 10, 2023
@bors
Copy link
Contributor

bors commented Oct 21, 2023

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

@xFrednet
Copy link
Contributor

(I claimed a few PRs, let's give this one to the lottery)

r? clippy

@rustbot rustbot assigned Jarcho and unassigned giraffate Mar 31, 2024
@y21 y21 force-pushed the unnecessary_string_from_utf8 branch 2 times, most recently from 1412560 to 0449a36 Compare March 31, 2024 20:58
Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

Looks fine. Just one small nit.

@y21 y21 force-pushed the unnecessary_string_from_utf8 branch from 0449a36 to ed9ccf6 Compare June 19, 2024 22:10
@Jarcho
Copy link
Contributor

Jarcho commented Jul 5, 2024

Whoops. I thought I already merged this. @bors r+

@bors
Copy link
Contributor

bors commented Jul 5, 2024

📌 Commit ed9ccf6 has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 5, 2024

⌛ Testing commit ed9ccf6 with merge 885f97e...

@bors
Copy link
Contributor

bors commented Jul 5, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 885f97e to master...

@bors bors merged commit 885f97e into rust-lang:master Jul 5, 2024
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.

taking reference on String::from_utf8 result whose argument is converted from slice can be rewritten in borrowed version of function
6 participants