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

alloc: Add Borrow impls for &String, &mut String, &Vec, and &mut Vec #45808

Closed
wants to merge 1 commit into from

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Nov 6, 2017

This patch addresses a small papercut, where the auto-ref/deref
does not work well with generic types. One small example of this
is:

use std::borrow::Borrow;

fn foo<T: Borrow<[usize]>>(x: T) {
    println!("{:?}", x.borrow());
}

fn main() {
    let x = vec![1, 2, 3];
    foo(&x);
}

Without this patch, rust will error with:

error[E0277]: the trait bound `&std::vec::Vec<{integer}>: std::borrow::Borrow<[usize]>` is not satisfied
 --> foo.rs:9:5
  |
9 |     foo(&x);
  |     ^^^ the trait `std::borrow::Borrow<[usize]>` is not implemented for `&std::vec::Vec<{integer}>`
  |
  = help: the following implementations were found:
            <std::vec::Vec<T> as std::borrow::Borrow<[T]>>
  = note: required by `foo`

error: aborting due to previous error

This forces users to use x.as_slice() to get the code to compile.
This patch then implements the following to cut down on unnecessary
line noise:

  • Borrow<str> for &String, &mut String
  • BorrowMut<str> for String and &mut String
  • Borrow<[T]> for &Vec<T> and &mut Vec<T>
  • BorrowMut<[T]> for &mut Vec<T>

@rust-highfive
Copy link
Collaborator

r? @dtolnay

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

This patch addresses a small papercut, where the auto-ref/deref
does not work well with generic types. One small example of this
is:

```rust
use std::borrow::Borrow;

fn foo<T: Borrow<[usize]>>(x: T) {
    println!("{:?}", x.borrow());
}

fn main() {
    let x = vec![1, 2, 3];
    foo(&x);
}
```

Without this patch, rust will error with:

```
error[E0277]: the trait bound `&std::vec::Vec<{integer}>: std::borrow::Borrow<[usize]>` is not satisfied
 --> foo.rs:9:5
  |
9 |     foo(&x);
  |     ^^^ the trait `std::borrow::Borrow<[usize]>` is not implemented for `&std::vec::Vec<{integer}>`
  |
  = help: the following implementations were found:
            <std::vec::Vec<T> as std::borrow::Borrow<[T]>>
  = note: required by `foo`

error: aborting due to previous error
```

This forces users to use `x.as_slice()` to get the code to compile.
This patch then implements the following to cut down on unnecessary
line noise:

* `Borrow<str>` for `&String`, `&mut String`
* `BorrowMut<str>` for `String` and `&mut String`
* `Borrow<[T]>` for `&Vec<T>` and `&mut Vec<T>`
* `BorrowMut<[T]>` for `&mut Vec<T>`
@erickt
Copy link
Contributor Author

erickt commented Nov 6, 2017

This may be made redundant by one of the ergonomics experiments, specifically the ones addressing RFC 2147.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Is BorrowMut<str> for String ever useful? I think the only way to use &mut str is make_ascii_uppercase.

For the others, is this something specific to &String and &Vec or do we ideally want it for everything else, &Cow, &Box, &Rc, &PathBuf, &CString, &[T; N], etc?

I would be inclined to avoid jumping the gun and give the ergonomics folks a chance to address this in a better way if possible.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 7, 2017
@erickt
Copy link
Contributor Author

erickt commented Nov 9, 2017

Yeah, this might be a little too much of a papercut. I wrote this a while ago, and I'm having trouble tracking down why I really wanted this :) I also havne't found many examples where this would clean things up, so I'll close it, and hopefully the ergonomics folks will find a more all-encompassing solution.

@erickt erickt closed this Nov 9, 2017
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.

4 participants