Tracking Issue for Stdin::lines forwarder method

[tlyu]

Feature gate: #![feature(stdin_forwarders)]

This is a tracking issue for adding new methods Stdin::lines and Stdin::split that will forward to the corresponding methods on the BufRead implementation of StdinLock. This proposal is related to #86845, and further reduces the obstacles for beginners to write simple interactive terminal programs.

Especially for beginners, reading a sequence of lines from the standard input stream can involve intimidating problems with locking and lifetimes. First, the user has to call the free function stdin() to get a handle on the stream; then, the user would have to call the lock() method to gain access to the lines() method. At this point, lifetime issues rapidly arise: the following code (playground)

use std::io::{self, prelude::*};
fn main() {
    let mut lines = io::stdin().lock().lines();
    loop {
        print!("prompt: ");
        io::stdout().flush();
        if let Some(_line) = lines.next() {
            // do stuff
        } else {
            break;
        }
    }
}

produces this error:

error[E0716]: temporary value dropped while borrowed
 --> src/main.rs:3:21
  |
3 |     let mut lines = io::stdin().lock().lines();
  |                     ^^^^^^^^^^^               - temporary value is freed at the end of this statement
  |                     |
  |                     creates a temporary which is freed while still in use
...
7 |         if let Some(_line) = lines.next() {
  |                              ----- borrow later used here
  |
  = note: consider using a `let` binding to create a longer lived value

The need to create a let binding to the handle seems confusing and frustrating, especially if the program does not need to use the handle again. The explanation is that the lock (and the iterator produced by lines()) behaves as if it borrows the original handle from stdin(), and the temporary value created for the call to the lock() method is dropped at the end of the statement, invalidating the borrow. That explanation might be beyond the current level of understanding of a beginner who simply wants to write an interactive terminal program.

Although #86845 makes it easier to obtain locked stdio handles, it would be even better if beginners didn't have to deal with the concept of locking at all at early stages of their learning.

There is precedent in the Stdin::read_line forwarder method that implicitly locks Stdin and calls the BufRead::read_line method. However, read_line() is somewhat difficult to use, because it requires that the user first allocate a String, and it doesn't remove newlines. In contrast, lines() provides an iterator over input lines that removes line endings, including both carriage return (CR) and line feed (LF) characters.

This proposal also includes a split() forwarder method, because it is similar in nature and usability to the lines() method. The remaining exclusive methods of BufRead are less useful to beginners, and require more experience to use.

Public API

// std::io

impl Stdin {
    pub fn lines(self) { /* ... */ }
}

Steps / History

• Implementation: add Stdin::lines, Stdin::split forwarder methods #86847
• Delete split forwarder delete Stdin::split forwarder #93134
• Final comment period (FCP)
• Stabilization PR: Stabilize Stdin::lines. #95185

Unresolved Questions

• During stabilization, we might want to update existing documentation to recommend these forwarder methods, and include examples of their use.

@rustbot label +A-io +D-newcomer-roadblock

[joshtriplett]

These methods have been around for a while, and they seem reasonable. I think it'd be appropriate to consider stabilizing them.

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

Concerns:

• better temporary lifetimes resolved by Tracking Issue for Stdin::lines forwarder method #87096 (comment)
• split-is-too-niche resolved by Tracking Issue for Stdin::lines forwarder method #87096 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[BurntSushi]

If #86845 ended up making stdin.lock() work, would we still want these methods?

[dtolnay]

#15023 is an accepted RFC to just make let lines = io::stdin().lock().lines() work.

I think I would want to check in with the lang team and/or compiler team on an outlook for that before we start adding surface area like this only motivated by working around it.

@rfcbot concern better temporary lifetimes

[dtolnay]

We discussed this API briefly in the libs-api meeting. After another look, I am on board with these methods since the motivation isn't just about working around the way that temporary lifetimes are currently handled — it's beneficial to be able to iterate lines of stdin without needing to consider that locking needs to be involved.

@rfcbot resolve better temporary lifetimes

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[matklad]

Two non-strong feelings:

• it feels a bit odd to stabilize this before Tracking Issue for owned locked stdio handles #86845. The .lines() construct indirectly exposes StdinLock<'static> type, which you otherwise can’t observe. I don’t see a way this can be actively problematic though.
• motivation for stabilizing split seems comparatively weaker than motivation for lines. It’s byte-level split, it yields Vec<u8> rather than String and as such seems like a niche API. Between read_line, lines and split, split wound be the only non-text re-exported API. For a beginner, who might think of stdin as a text stream, .split() which doesn’t split on whitespace, a-la Python, might be confusing.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[BurntSushi]

@rfcbot concern split-is-too-niche

Yeah I think there is some concern with split here. If we're adding these routines specifically to work around common use cases, then I'm not sure split is one of them in a way that's consistent with the rest of std's APIs (which really wants all text oriented processing to happen inside String/&str). It looks like the FCP is done though, so I'm not sure if registering a concern is possible? I'm just now catching up with stuff after the holidays.

Also, these APIs are very poorly documented at the moment. I kind of feel like APIs should have full documentation on them before we consider stabilization. For example, I think these APIs should mention the motivation for their existence, in that it permits more convenient line iteration when compared to having to deal with locking stdin explicitly.

[tlyu]

@BurntSushi I'm willing to remove the split forwarder. Can the API documentation improvement happen during stabilization? There were some API documentation changes that I didn't want to make before stabilization, because I would want higher-level std::io documentation to preferentially mention the forwarder method. My understanding is that we can't do that until during stabilization; is that right?

[joshtriplett]

I'm not opposed to dropping split; lines seems like the most valuable item here.

[tlyu]

I'm not opposed to dropping split; lines seems like the most valuable item here.

Thanks; I'll work on a PR to delete the split forwarder.

[tlyu]

I'm not opposed to dropping split; lines seems like the most valuable item here.

Thanks; I'll work on a PR to delete the split forwarder.

#93134 if someone who's already commented here is interested in reviewing it. maybe @joshtriplett ?

[Patrick-Poitras]

For what its worth I've used split for processing input with a non-endline separator, but I agree that the implementation here is probably not the one we want for the sake of consistency. Perhaps the idea would be worth revisiting but behind another feature flag as to not stop the stabilization of the other method.

[joshtriplett]

@BurntSushi The removal of split is about to be merged; that should address your concern.

[BurntSushi]

@rfcbot resolve split-is-too-niche

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[m-ou-se]

Looks like rfcbot is sleeping (:

---

🤖 📣

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the temporary human substitute for the temporarily unavailable automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[scottmcm]

rfcbot commented earlier (#87096 (comment)), so I guess it got confused by the concern+resolve after the FCP had "completed".