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

Lint on calling to_owned on a reference #78187

Open
Aaron1011 opened this issue Oct 21, 2020 · 4 comments
Open

Lint on calling to_owned on a reference #78187

Aaron1011 opened this issue Oct 21, 2020 · 4 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Aaron1011
Copy link
Member

The ToOwned trait has a blanket impl for T: Clone. Since references implement Clone, it's possible to call &SomeNonCloneType.to_owned() - this returns a new reference &SomeNonCloneType, not an owned SomeNonCloneType. This can result in a confusing error message:

struct Foo;

fn main() {
    let a = &Foo;
    let b: Foo = a.to_owned();
}
error[E0308]: mismatched types
 --> src/main.rs:5:18
  |
5 |     let b: Foo = a.to_owned();
  |            ---   ^^^^^^^^^^^^ expected struct `Foo`, found `&Foo`
  |            |
  |            expected due to this

Here, adding #[derive(Clone)] to Foo makes this code compile, but it's not at all obvious from looking at the error message.

We should lint on calling val.to_owned() or val.clone(), when val is a reference, and treated as the Self type (as opposed to being called on a reference to some non-reference type that implemented Clone). This is very likely a mistake, and in any case can be written more succinctly as *val

@Aaron1011 Aaron1011 added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Oct 21, 2020
@Aaron1011
Copy link
Member Author

We also might want to consider displaying additional information when an error involves a method with a generic return type. Something like:

note: a.to_owned()` returns `&Foo` because `a` has type `&Foo`, which implements `ToOwned`.

@Aaron1011
Copy link
Member Author

Aaron1011 commented Oct 21, 2020

More generally, this issue is caused by the fact that &Foo.method() can resolve to two different things:

  1. A call to a method implemented for Foo, with a receiver of &self.
  2. A call to a method implemented for &T, with an (autoref) receiver of &self.

Unfortunately, I don't think we can do very much about this in general. The method implemented on Foo may come from a blanket impl (e.g. impl<T: Clone> ToOwned for T), and may not apply due to some complicated trait bound not holding. Emitting a warning because a type doesn't implement a trait (e.g. Clone) seems like it could be even more confusing.

@JohnTitor JohnTitor added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 22, 2020
@omarabid
Copy link

@Aaron1011 I was confused by this multiple times. I don't think this is a linting problem. Actually, I don't understand how this doesn't result in an error. (in fact, if you don't enforce the type on your example (for b), the compiler will be happy to just carry on).

@Aaron1011
Copy link
Member Author

@omarabid Without the enforced type, to_owned will be called on a reference, since they implement ToOwned through the blanket impl for Clone types:

struct Foo;

fn print_type<T>(val: T) { println!("{}", std::any::type_name::<T>()) }

fn main() {
    let a = &Foo;
    let b = a.to_owned();
    print_type(b);
}

this prints &playground::Foo

@Dylan-DPC Dylan-DPC added C-bug Category: This is a bug. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants