Skip to content

E0716: confusing reference to borrow of Stdin value when there's no obvious reference within StdinLock #85383

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
tlyu opened this issue May 16, 2021 · 8 comments · Fixed by #86799
Labels
A-borrow-checker Area: The borrow checker A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tlyu
Copy link
Contributor

tlyu commented May 16, 2021

Given the following code: (playground)

use std::io::{self, BufRead};
fn main() {
    let locked = io::stdin().lock();
    for line in locked.lines() {
        println!("{}", line.unwrap());
    }
}

The current output is:

   Compiling playground v0.0.1 (/playground)
error[E0716]: temporary value dropped while borrowed
 --> src/main.rs:3:18
  |
3 |     let locked = io::stdin().lock();
  |                  ^^^^^^^^^^^       - temporary value is freed at the end of this statement
  |                  |
  |                  creates a temporary which is freed while still in use
4 |     for line in locked.lines() {
  |                 ------ borrow later used here
  |
  = note: consider using a `let` binding to create a longer lived value

error: aborting due to previous error

For more information about this error, try `rustc --explain E0716`.
error: could not compile `playground`

To learn more, run the command again with --verbose.

I think it's typical for a newcomer to want to get a Lines iterator by chaining method calls like io::stdin().lock().lines(). There's no obvious reason why that shouldn't work. The examples in the std::io documentation do typically show io::stdin() being assigned to a variable prior to calling its .lock() method, but don't explain why that's necessary.

I think the "borrow" terminology is misleading, but there might not be a concise way to describe what's actually going on to the user. The "temporary value is freed at the end of this statement" is probably correct. The "creates a temporary which is freed while still in use" is probably more correctly "creates a temporary that is required to outlive (or live exactly as long as) the result of...", followed by a pointer to the .lock() method invocation, with text of "...this method call". Maybe it should also point out the '_ lifetime argument in the StdinLock<'_> return type.

I tried and failed to find a satisfactory explanation of what exactly StdinLock was borrowing from the temporary Stdin value produced by io::stdin(). When I looked through the library source code, the only references in StdinLock were those created by locking a Mutex, and the Mutex in question is static. I'm guessing the existence of a borrow is assumed rather than inferred from analyzing the function body of io::Stdin::lock(). The assumption might be the result of the anonymous lifetime argument on the output type StdinLock<'_> from io::Stdin::lock(), which imposes a requirement that the StdinLock doesn't outlive (or has the same lifetime as?) the Stdin value that is the receiver of lock().

The correct fix might be to change the locked stdin handles to all use 'static lifetimes, but I will probably file that as a separate issue.

@tlyu tlyu added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 16, 2021
@tlyu
Copy link
Contributor Author

tlyu commented May 16, 2021

@rustbot label +A-borrow-checker +A-lifetimes +D-confusing +D-incorrect +D-newcomer-roadblock

@rustbot rustbot added A-borrow-checker Area: The borrow checker A-lifetimes Area: Lifetimes / regions D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels May 16, 2021
@ElectronicRU
Copy link

IMO the intent is clear here. Without changing the API, why exactly is Rust unable to lift 'a &'static X to 'static & 'static X, if X also contains only 'static lifetimes? I don't think raw references have a meaningful dropper; borrowck is able to freely resolve any derived lifetimes on impls that don't depend on anything else to 'static; I'm pretty sure under current rules we should be able to substitute latter for earlier and get exactly the same behaviour.

@FabianWolff
Copy link
Contributor

This is due to lifetime elision. Stdin::lock() is declared as follows:

pub fn lock(&self) -> StdinLock<'_> { ... }

Because there is only one input lifetime (the elided lifetime of self), it is assigned to the output lifetime '_, which is why the lock can live at most as long as the Stdin temporary.

@tlyu
Copy link
Contributor Author

tlyu commented May 17, 2021

Because there is only one input lifetime (the elided lifetime of self), it is assigned to the output lifetime '_, which is why the lock can live at most as long as the Stdin temporary.

Thanks. I think this might be a bug in libstd (using StdinLock<'_> instead of StdinLock<'static> as the return type), but I'll file that separately.

What I think is a diagnostic inaccuracy is that the error message refers to a borrow, when, by inspection of the library source code, no borrow is taking place in StdinLock. (Unless I'm missing something?) What I think is happening is that the compiler is enforcing that StdinLock lives no longer than the temporary Stdin produced by io::stdin(), and is misleadingly calling that a borrow in the error message.

@tlyu
Copy link
Contributor Author

tlyu commented May 17, 2021

As a simplified example: (playground)

static NUM: i32 = 1;
struct A;
struct B<'a>(&'a i32);
impl A {
    fn get(&self) -> B<'_> {
        // Contains NO ref to self whatsoever!
        B(&NUM)
    }
}
fn a() -> A { A }
fn main() {
    let b = a().get();
    println!("{}", b.0.to_string());
}
error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:12:13
   |
12 |     let b = a().get();
   |             ^^^      - temporary value is freed at the end of this statement
   |             |
   |             creates a temporary which is freed while still in use
13 |     println!("{}", b.0.to_string());
   |                    --- borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

The error message refers to a borrow even when the output type of get() does not refer to self at all!

@estebank
Copy link
Contributor

@tlyu that's because of the elision rules: on the fn declaration you're implicitly telling the compiler that the lifetime of the self borrow matches the lifetime of the B type. That definition must be written fn get(&self) -> B<'static>.

@tlyu
Copy link
Contributor Author

tlyu commented May 18, 2021

@estebank Thanks. I think the error messages could use a little more detail about how something that the borrow checker sees as a borrow may be an artifact of lifetime constraints in a function signature rather than an actual borrow in the function body. It certainly surprised me the first time I ran into it. (Maybe only in the explanatory text in the compiler error index. It looks like E0597 could also benefit from that detail, because it's a similar case, and an example in E0716 produces E0597.)

@tlyu
Copy link
Contributor Author

tlyu commented May 18, 2021

Shorter example, probably better suited for an example in the compiler error index: (playground)

fn main() {
    static NUM: i32 = 17;
    fn foo() -> i32 { 42 }
    // output ref is totally unrelated to input
    fn bar(_: &i32) -> &i32 { &NUM }
    let x = bar(&foo());
    println!("{}", x);
}
error[E0716]: temporary value dropped while borrowed
 --> src/main.rs:6:18
  |
6 |     let x = bar(&foo());
  |                  ^^^^^ - temporary value is freed at the end of this statement
  |                  |
  |                  creates a temporary which is freed while still in use
7 |     println!("{}", x);
  |                    - borrow later used here
  |
  = note: consider using a `let` binding to create a longer lived value

@estebank estebank removed the D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. label May 19, 2021
@bors bors closed this as completed in a8b8558 Jul 3, 2021
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 A-lifetimes Area: Lifetimes / regions D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
5 participants