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

Update 'or_fun_call' to ignore calls to len #4429

Merged
merged 4 commits into from
May 25, 2020

Conversation

jeremystucki
Copy link
Contributor

Resolves #1653

changelog: Update or_fun_call: Allow calls to len for Slice, Array & Vec.

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 24, 2019
@flip1995
Copy link
Member

flip1995 commented Sep 2, 2019

I'm not sure if this is really a FP, since the lint description matches the behavior 100%. with *_or the len() method is called every time, while with *_or_else the len() function is only called in the None/Err case. So even when it's just "a simple value copy", it has to be done, which still can result is perf difference (maybe?).

@flip1995
Copy link
Member

@bors r+

Thanks. Contrarily to my previous point, I now see that this is a clear improvement to the lint.

@bors
Copy link
Contributor

bors commented May 25, 2020

📌 Commit d9b8508 has been approved by flip1995

@bors
Copy link
Contributor

bors commented May 25, 2020

⌛ Testing commit d9b8508 with merge f7c486c...

bors added a commit that referenced this pull request May 25, 2020
Update 'or_fun_call' to ignore calls to len

Resolves #1653

changelog: Update `or_fun_call`: Allow calls to `len` for Slice, Array & Vec.
@bors
Copy link
Contributor

bors commented May 25, 2020

💔 Test failed - checks-action_test

@flip1995
Copy link
Member

@jeremystucki Can you update this PR to the latest rustc, so we can merge this?

@flip1995 flip1995 self-assigned this May 25, 2020
@flip1995
Copy link
Member

tests/ui/update-references.sh 'target/debug/test_build_base' 'or_fun_call.rs'

Please rebase this branch, to get rid of the merge commits.

@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented May 25, 2020

📌 Commit d9f5532 has been approved by flip1995

@bors
Copy link
Contributor

bors commented May 25, 2020

⌛ Testing commit d9f5532 with merge dd06d29...

@bors
Copy link
Contributor

bors commented May 25, 2020

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

@bors bors merged commit dd06d29 into rust-lang:master May 25, 2020
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.

unwrap_or_else false positive
4 participants