Skip to content

Commit 26c50d4

Browse files
authored
Unrolled build for #154646
Rollup merge of #154646 - m4rch3n1ng:144792-cow-diag, r=davidtwco Add suggestion to `.to_owned()` used on `Cow` when borrowing fixes #144792 supersedes #144925 with the review comments addressed the tests suggested from @Kivooeo from #144925 (comment) didn't work entirely, because these tests failed due to error `[E0308]` mismatched types, which actually already provides a suggestion, that actually makes the code compile: ``` $ cargo check error[E0308]: mismatched types --> src/main.rs:3:5 | 1 | fn test_cow_suggestion() -> String { | ------ expected `std::string::String` because of return type 2 | let os_string = std::ffi::OsString::from("test"); 3 | os_string.to_string_lossy().to_owned() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `String`, found `Cow<'_, str>` | = note: expected struct `std::string::String` found enum `std::borrow::Cow<'_, str>` help: try using a conversion method | 3 | os_string.to_string_lossy().to_owned().to_string() | ++++++++++++ ``` now this suggestion is of course not good or efficient code, but via clippy with `-Wclippy::nursery` lint group you can actually get to the correct code, so i don't think this is too much of an issue: <details> <summary>the clippy suggestions</summary> ``` $ cargo clippy -- -Wclippy::nursery warning: this `to_owned` call clones the `Cow<'_, str>` itself and does not cause its contents to become owned --> src/main.rs:3:5 | 3 | os_string.to_string_lossy().to_owned().to_string() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/beta/index.html#suspicious_to_owned = note: `#[warn(clippy::suspicious_to_owned)]` on by default help: depending on intent, either make the `Cow` an `Owned` variant | 3 | os_string.to_string_lossy().into_owned().to_string() | ++ help: or clone the `Cow` itself | 3 - os_string.to_string_lossy().to_owned().to_string() 3 + os_string.to_string_lossy().clone().to_string() | $ # apply first suggestion $ cargo c -- -Wclippy::nursery warning: redundant clone --> src/main.rs:3:45 | 3 | os_string.to_string_lossy().into_owned().to_string() | ^^^^^^^^^^^^ help: remove this | note: this value is dropped without further use --> src/main.rs:3:5 | 3 | os_string.to_string_lossy().into_owned().to_string() | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = help: for further information visit https://rust-lang.github.io/rust-clippy/beta/index.html#redundant_clone = note: `-W clippy::redundant-clone` implied by `-W clippy::nursery` = help: to override `-W clippy::nursery` add `#[allow(clippy::redundant_clone)]` ``` </details> the actual error that we are looking for is error `[E0515]`, cannot return value referencing local variable, which was only present in the original issue due to automatic type inference assuming you want to return a cloned `Cow<'_, str>`, which is of course not possible. this is why i took the original test functions and turned them into closures, where it's less obvious that it's trying to return the wrong type. r? davidtwco (because you reviewed the last attempt) (also, let me know if i should squash this down to one commit and add myself as the co-contributor)
2 parents b6100cc + 41f7c2d commit 26c50d4

5 files changed

Lines changed: 85 additions & 1 deletion

File tree

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3538,6 +3538,24 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
35383538
Applicability::MaybeIncorrect,
35393539
);
35403540
}
3541+
3542+
if let Some(cow_did) = tcx.get_diagnostic_item(sym::Cow)
3543+
&& let ty::Adt(adt_def, _) = return_ty.kind()
3544+
&& adt_def.did() == cow_did
3545+
{
3546+
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span) {
3547+
if let Some(pos) = snippet.rfind(".to_owned") {
3548+
let byte_pos = BytePos(pos as u32 + 1u32);
3549+
let to_owned_span = return_span.with_hi(return_span.lo() + byte_pos);
3550+
err.span_suggestion_short(
3551+
to_owned_span.shrink_to_hi(),
3552+
"try using `.into_owned()` if you meant to convert a `Cow<'_, T>` to an owned `T`",
3553+
"in",
3554+
Applicability::MaybeIncorrect,
3555+
);
3556+
}
3557+
}
3558+
}
35413559
}
35423560

35433561
Err(err)

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ symbols! {
196196
Continue,
197197
ControlFlow,
198198
Copy,
199+
Cow,
199200
Debug,
200201
Default,
201202
Deref,

src/tools/clippy/clippy_utils/src/sym.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ generate! {
5050
Cargo_toml: "Cargo.toml",
5151
Child,
5252
Command,
53-
Cow,
5453
Current,
5554
DOUBLE_QUOTE: "\"",
5655
DebugStruct,
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//! Regression test for: https://github.com/rust-lang/rust/issues/144792
2+
3+
fn main() {
4+
let _ = || {
5+
let os_string = std::ffi::OsString::from("test");
6+
os_string.to_string_lossy().to_owned()
7+
//~^ ERROR: cannot return value referencing local variable `os_string` [E0515]
8+
};
9+
10+
let _ = || {
11+
let s = "hello".to_owned();
12+
let cow = std::borrow::Cow::from(&s);
13+
cow.to_owned()
14+
//~^ ERROR: cannot return value referencing local variable `s` [E0515]
15+
};
16+
17+
let _ = || {
18+
let bytes = b"hello".to_owned();
19+
let cow = std::borrow::Cow::from(&bytes[..]);
20+
cow.to_owned()
21+
//~^ ERROR: cannot return value referencing local variable `bytes` [E0515]
22+
};
23+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
error[E0515]: cannot return value referencing local variable `os_string`
2+
--> $DIR/cow-into-owned-suggestion.rs:6:9
3+
|
4+
LL | os_string.to_string_lossy().to_owned()
5+
| ---------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
| |
7+
| returns a value referencing data owned by the current function
8+
| `os_string` is borrowed here
9+
|
10+
help: try using `.into_owned()` if you meant to convert a `Cow<'_, T>` to an owned `T`
11+
|
12+
LL | os_string.to_string_lossy().into_owned()
13+
| ++
14+
15+
error[E0515]: cannot return value referencing local variable `s`
16+
--> $DIR/cow-into-owned-suggestion.rs:13:9
17+
|
18+
LL | let cow = std::borrow::Cow::from(&s);
19+
| -- `s` is borrowed here
20+
LL | cow.to_owned()
21+
| ^^^^^^^^^^^^^^ returns a value referencing data owned by the current function
22+
|
23+
help: try using `.into_owned()` if you meant to convert a `Cow<'_, T>` to an owned `T`
24+
|
25+
LL | cow.into_owned()
26+
| ++
27+
28+
error[E0515]: cannot return value referencing local variable `bytes`
29+
--> $DIR/cow-into-owned-suggestion.rs:20:9
30+
|
31+
LL | let cow = std::borrow::Cow::from(&bytes[..]);
32+
| ----- `bytes` is borrowed here
33+
LL | cow.to_owned()
34+
| ^^^^^^^^^^^^^^ returns a value referencing data owned by the current function
35+
|
36+
help: try using `.into_owned()` if you meant to convert a `Cow<'_, T>` to an owned `T`
37+
|
38+
LL | cow.into_owned()
39+
| ++
40+
41+
error: aborting due to 3 previous errors
42+
43+
For more information about this error, try `rustc --explain E0515`.

0 commit comments

Comments
 (0)