Skip to content

Misleading diagnostics when owning Option is used instead of an Option with a reference #43861

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

Closed
kornelski opened this issue Aug 14, 2017 · 3 comments
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@kornelski
Copy link
Contributor

kornelski commented Aug 14, 2017

There's an unfortunate effect of type-checking done before borrow checking, when the root of the type error is due to use of owned value instead of a reference.

Specifically, common uses of .map()/.and_then()/if let Some() on &Option<T: !Copy> are likely to fail with a misleading error.

When the inner value is used as if it was a reference it causes an error that is technically correct, but ultimately unhelpful and misleading:

= note: expected type &Foo
found type Foo
= help: try with &arg

This hint is wrong, because actually using &arg gives the dreaded:

cannot move out of borrowed content

and this error doesn't have any suggestion how to proceed, so it's a scary dead-end for someone struggling with the borrow checker.

The correct solution is to suggest calling .as_ref() on Option. Here's an example:

struct Foo;
fn takes_ref(_: &Foo) {}

fn main() {
  let ref opt = Some(Foo);

  opt.map(|arg| takes_ref(arg));
  // ^ suggests: opt.map(|arg| take_ref(&arg));

  opt.as_ref().map(|arg| takes_ref(arg));
  // ^ this should be done instead
}

I find it to be a big problem, because in Rust it's easy to end up with &Option<T>. To a novice it's not obvious that it can be converted to Option<&T> that works, and Rust doesn't suggest that solution.

Due to borrow checker depending on type-checking it's probably impossible to improve this diagnostic in general case, but I suggest at least special-casing it for Option (suggesting .as_ref()), since it's such a common pitfall.

@carols10cents carols10cents added A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. labels Aug 14, 2017
@scottmcm
Copy link
Member

Anecdotally, borrowing problems with options that are fixed with .as_ref() is the most common issue I see asked about in #rust-beginners.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 15, 2017

Could the borrow checker still be run even in the presence of type errors? It could simply not borrow check any code with type errors, that way the outer call of map on a reference to an option would fail borrow check additionally to the typeck error in the arguments of map

bors added a commit that referenced this issue May 30, 2018
Suggest using `as_ref` on some borrow errors [hack]

When encountering the following code:

```rust
struct Foo;
fn takes_ref(_: &Foo) {}
let ref opt = Some(Foo);

opt.map(|arg| takes_ref(arg));
```

Suggest using `opt.as_ref().map(|arg| takes_ref(arg));` instead.

This is a stop gap solution until we expand typeck to deal with these
cases in a more graceful way.

#43861
@estebank
Copy link
Contributor

Current output:

error[E0308]: mismatched types
 --> file2.rs:7:27
  |
7 |   opt.map(|arg| takes_ref(arg));
  |       ---                 ^^^ expected &Foo, found struct `Foo`
  |       |
  |       help: consider using `as_ref` instead: `as_ref().map`
  |
  = note: expected type `&Foo`
             found type `Foo`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. 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

5 participants