Skip to content

Update mod.rs #141935

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
Closed

Conversation

kiseitai3
Copy link

@kiseitai3 kiseitai3 commented Jun 2, 2025

Small clarification on the usage of read_to_string and read_to_end trait methods. The goal is to make it clear that these trait methods will become locked up if attempting to read to the end of stdin (which is a bit non-sensical unless the other end closes the pipe).

Fixes: #141714

Small clarification on the usage of `read_to_string` and `read_to_end` trait methods.
@rustbot
Copy link
Collaborator

rustbot commented Jun 2, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 2, 2025
@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I think this is a worthwhile change but the wording could use some adjustment.

Could you please also update the commit summary and PR description? They currently both say "Update mod.rs" but instead need to say what the actual change is.

Comment on lines 967 to 975
/// # Notes
///
/// Be careful using this trait method with streams that are expected to be continuous. For example, using
/// `read_to_string` with streams like `stdin` will simply lock the application into waiting on the
/// transmission of data to conclude. It is recommended you use this method if you know the stream will be closed
/// at the other end. The problem is that EOF or End of File is never reached for streams that never close or are finite.
///
/// (See also the [`std::fs::read_to_string`] convenience function for
/// reading from a file.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, could you move the new section under the existing "see also"? Since the "see also" is referring to reading a file in the example above.

Copy link
Author

Choose a reason for hiding this comment

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

Only issue now is that committing to my feature branch generated internal merges and I did not realize that was prohibited. Tried resetting and rebasing my branch without success.

Copy link
Contributor

Choose a reason for hiding this comment

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

What are you running into? Usually fixing it looks something like this:

# If you haven't yet, add rust-lang/rust as a remote and update it
git remote add upstream [email protected]:rust-lang/rust.git
git fetch upstream master

# Do the rebase (from this branch)
git rebase -i upstream/master

Then for every line except the first replace pick with f (or fixup), and git push --force-with-lease to update this PR.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2025
@rustbot

This comment has been minimized.

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Jun 4, 2025
@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Comment on lines 922 to 927
/// `read_to_end` attempts to read a source until EOF, but many sources are continuous streams
/// that do not send EOF. In these cases, `read_to_end` will block indefinitely. Standard input
/// is one such stream which may be finite if piped, but is typically continuous. For example,
/// `cat <file> | <my_rust_program>` will correctly terminate with an `EOF` upon closure of cat.
/// `yes "Data" | pv | <my_rust_program` or `tail -f <file> | <my_rust_program>` may not close
/// the stream or insert an `EOF` termination character.
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks much better to me. I think the cat example would be good to keep, but maybe translate the other example into words in case someone doesn't know what the -f flag does

    /// `read_to_end` attempts to read a source until EOF, but many sources are continuous streams
    /// that do not send EOF. In these cases, `read_to_end` will block indefinitely. Standard input
    /// is one such stream which may be finite if piped from a finite source (e.g. `cat foo.txt |
    /// my-program`), but is typically continuous (reading user input or sources that stay open).

Copy link
Author

Choose a reason for hiding this comment

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

I am making adjustments, but did not have time to complete tonight.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a problem, there is no deadline :)

oli-obk and others added 11 commits June 5, 2025 20:06
This adds an `iter!` macro that can be used to create movable
generators.

This also adds a yield_expr feature so the `yield` keyword can be used
within iter! macro bodies. This was needed because several unstable
features each need `yield` expressions, so this allows us to stabilize
them separately from any individual feature.

Co-authored-by: Oli Scherer <[email protected]>
Co-authored-by: Jieyou Xu <[email protected]>
Co-authored-by: Travis Cross <[email protected]>
The panic machinery uses TLS, so panicking if no TLS keys are left can lead to infinite recursion (see rust-lang#140798 (comment)). Rather than having separate logic for the panic count and the thread name, just always abort the process if a TLS key allocation fails. This also has the benefit of aligning the key-based TLS implementation with the documentation, which does not mention that a panic could also occur because of resource exhaustion.
- Add AbiMapping for encoding the nuance of deprecated ABIs
makes entry_abi a lowering of the ABI string, so now it can be
```json
  "entry_abi": "C",
  "entry_abi": "win64",
  "entry_abi": "aapcs",
```
tgross35 and others added 9 commits June 5, 2025 20:06
The submodule was causing issues in rust-lang/rust, so eliminiate it
here. `build-musl` is also removed from `libm-test`'s default features
so the crate doesn't need to be built by default.
This will be used by `josh` tooling.
Create a crate that handles pulling from and pushing to rust-lang/rust.
This can be invoked with the following:

    $ cargo run -p josh-sync -- rustc-pull
    $ RUSTC_GIT=/path/to/rust/checkout cargo run -p josh-sync -- rustc-push <username>
To prepare for merging from rust-lang/rust, set the version file to:

    df8102f Auto merge of rust-lang#142002 - onur-ozkan:follow-ups2, r=jieyouxu
This was introduced before `#[panic_handler]` was stable, but should no
longer be needed. Additionally, we only need it for
`builtins-test-intrinsics`, not as a dependency of `compiler-builtins`.
@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc PG-exploit-mitigations Project group: Exploit mitigations T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jun 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 6, 2025

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

compiler-builtins is developed in its own repository. If possible, consider making this change to rust-lang/compiler-builtins instead.

cc @tgross35

Some changes occurred to constck

cc @fee1-dead

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rustbot
Copy link
Collaborator

rustbot commented Jun 6, 2025

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust.git master
    $ git push --force-with-lease
    

@rustbot

This comment has been minimized.

@kiseitai3 kiseitai3 force-pushed the 141714_stdin_read_to_string_docs branch from 61ca3f6 to 7298055 Compare June 6, 2025 01:05
@rustbot
Copy link
Collaborator

rustbot commented Jun 6, 2025

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust.git master
    $ git push --force-with-lease
    

@kiseitai3 kiseitai3 closed this Jun 6, 2025
@kiseitai3 kiseitai3 deleted the 141714_stdin_read_to_string_docs branch June 6, 2025 01:08
@kiseitai3
Copy link
Author

Well, the rebase made a mess. I decided to kill the messy branch and cherry pick my commit into a fresh version of the branch. I will open a new PR.

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-2 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
 Documenting std v0.0.0 (/checkout/library/std)
error: unresolved link to `read`
   --> library/std/src/io/mod.rs:929:58
    |
929 |     /// Using `.lines()` with a [`BufReader`] or using [`read`] can provide a better solution
    |                                                          ^^^^ no item named `read` in scope
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
    = note: `-D rustdoc::broken-intra-doc-links` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(rustdoc::broken_intra_doc_links)]`

error: unresolved link to `read`
   --> library/std/src/io/mod.rs:983:58
    |
983 |     /// Using `.lines()` with a [`BufReader`] or using [`read`] can provide a better solution
    |                                                          ^^^^ no item named `read` in scope
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

error: unresolved link to `read`
    --> library/std/src/io/mod.rs:1297:54
     |
1297 | /// Using `.lines()` with a [`BufReader`] or using [`read`] can provide a better solution
     |                                                      ^^^^ no item named `read` in scope
     |
     = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

error: could not document `std`
warning: build failed, waiting for other jobs to finish...
[RUSTC-TIMING] std test:false 7.970
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:01:03
  local time: Fri Jun  6 01:47:57 UTC 2025
  network time: Fri, 06 Jun 2025 01:47:57 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@RalfJung
Copy link
Member

RalfJung commented Jun 6, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-rustc-dev-guide Area: rustc-dev-guide A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc has-merge-commits PR has merge commits, merge with caution. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot read from STDIN with read_to_string if reading before piping Rust program writes to STDOUT in another Rust program