-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Fix "consider borrowing" for else-if #141812
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
Conversation
nicely done! I cannot quite reproduce the case in which this is needed: But I'm a little worried it's not always correct. At the very least, there's a difference here (https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=ac067fdd4897e207ee42b9d9f9a433b8) in putting the reference inside or outside the block. Am I right that that's the case it's trying to catch? |
Indeed, you're right. call(&if true {
String::new()
} else {
String::new()
}) Would you suggest just limiting this improvement to if then? I can quickly code that up, is probably better |
I think the if suggestion is clearly wrong, any improvement is worth it. But I'd say the old block suggestion is more often correct. Let's limit it to that and I'll re-review @rustbot author |
Reminder, once the PR becomes ready for a review, use |
58d73b1
to
b1a1df2
Compare
@rustbot ready |
looking good! @bors r+ rollup |
…r=jdonszelmann Fix "consider borrowing" for else-if Fixes rust-lang#141810 When trying to suggest a borrow on a `if` or `block` expression, instead we now recurse into the `if` or `block`. The comments in the code should explain the goal of the new code. r? `@jdonszelmann`
Rollup of 6 pull requests Successful merges: - #141072 (Stabilize feature `result_flattening`) - #141215 (std: clarify Clone trait documentation about duplication semantics) - #141277 (Miri CI: test aarch64-apple-darwin in PRs instead of the x86_64 target) - #141521 (Add `const` support for float rounding methods) - #141812 (Fix "consider borrowing" for else-if) - #141832 (library: explain TOCTOU races in `fs::remove_dir_all`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #141812 - JonathanBrouwer:fix-else-if-help, r=jdonszelmann Fix "consider borrowing" for else-if Fixes #141810 When trying to suggest a borrow on a `if` or `block` expression, instead we now recurse into the `if` or `block`. The comments in the code should explain the goal of the new code. r? ``@jdonszelmann``
Rollup of 6 pull requests Successful merges: - rust-lang/rust#141072 (Stabilize feature `result_flattening`) - rust-lang/rust#141215 (std: clarify Clone trait documentation about duplication semantics) - rust-lang/rust#141277 (Miri CI: test aarch64-apple-darwin in PRs instead of the x86_64 target) - rust-lang/rust#141521 (Add `const` support for float rounding methods) - rust-lang/rust#141812 (Fix "consider borrowing" for else-if) - rust-lang/rust#141832 (library: explain TOCTOU races in `fs::remove_dir_all`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 6 pull requests Successful merges: - rust-lang#141072 (Stabilize feature `result_flattening`) - rust-lang#141215 (std: clarify Clone trait documentation about duplication semantics) - rust-lang#141277 (Miri CI: test aarch64-apple-darwin in PRs instead of the x86_64 target) - rust-lang#141521 (Add `const` support for float rounding methods) - rust-lang#141812 (Fix "consider borrowing" for else-if) - rust-lang#141832 (library: explain TOCTOU races in `fs::remove_dir_all`) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #141810
When trying to suggest a borrow on a
if
orblock
expression, instead we now recurse into theif
orblock
.The comments in the code should explain the goal of the new code.
r? @jdonszelmann