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.

Sign up for GitHub

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

Jump to bottom

Add core::stream::Stream #79023

Merged
merged 2 commits into from
Jan 30, 2021
Merged

Add core::stream::Stream #79023

merged 2 commits into from
Jan 30, 2021

Conversation

yoshuawuyts
Copy link
Member

@yoshuawuyts yoshuawuyts commented Nov 13, 2020 •
edited
Loading

[Tracking issue: #79024]

This patch adds the core::stream submodule and implements core::stream::Stream in accordance with RFC2996. The RFC hasn't been merged yet, but as requested by the libs team in rust-lang/rfcs#2996 (comment) I'm filing this PR to get the ball rolling.

Documentatation

The docs in this PR have been adapted from std::iter, async_std::stream, and futures::stream::Stream. Once this PR lands my plan is to follow this up with PRs to add helper methods such as stream::repeat which can be used to document more of the concepts that are currently missing. That will allow us to cover concepts such as "infinite streams" and "laziness" in more depth.

Feature gate

The feature gate for Stream is stream_trait. This matches the #[lang = "future_trait"] attribute name. The intention is that only the APIs defined in RFC2996 will use this feature gate, with future additions such as stream::repeat using their own feature gates. This is so we can ensure a smooth path towards stabilizing the Stream trait without needing to stabilize all the APIs in core::stream at once. But also don't start expanding the API until after stabilization, as was the case with std::future.

edit: the feature gate has been changed to async_stream to match the feature gate proposed in the RFC.

Conclusion

This PR introduces core::stream::{Stream, Next} and re-exports it from std as std::stream::{Stream, Next}. Landing Stream in the stdlib has been a mult-year process; and it's incredibly exciting for this to finally happen!

r? @KodrAus
cc/ @rust-lang/wg-async-foundations @rust-lang/libs

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 13, 2020
@yoshuawuyts yoshuawuyts mentioned this pull request Nov 13, 2020
Closed
@yoshuawuyts
Copy link
Member Author

oh, I just realized the RFC already sets a feature name: async_stream. Updating the PR to gate on that instead.

@yoshuawuyts yoshuawuyts mentioned this pull request Nov 13, 2020
Open
9 tasks
taiki-e
taiki-e reviewed Nov 13, 2020
View reviewed changes
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
@yoshuawuyts
Copy link
Member Author

Updated with @taiki-e's feedback!

taiki-e
taiki-e reviewed Nov 13, 2020
View reviewed changes
library/std/src/stream.rs Outdated Show resolved Hide resolved
@taiki-e
Copy link
Member

taiki-e commented Nov 13, 2020 •
edited
Loading

@yoshuawuyts: I think you missed #79023 (comment).

@nikomatsakis
Copy link
Contributor

Technically this should probably be reviewed by someone in the @rust-lang/compiler area, since we're supposed to be managing the actual implementation. I'm going to put

r? @tmandry

@rust-highfive rust-highfive assigned tmandry and unassigned KodrAus Nov 13, 2020
taiki-e
taiki-e reviewed Nov 13, 2020
View reviewed changes
library/std/src/lib.rs Outdated Show resolved Hide resolved
bluetech
bluetech reviewed Nov 14, 2020
View reviewed changes
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/stream/mod.rs Outdated Show resolved Hide resolved
@yoshuawuyts
Copy link
Member Author

Updated with @bluetech and @nagisa's feedback!

@Keruspe Keruspe mentioned this pull request Nov 14, 2020
Closed
@yoshuawuyts
Copy link
Member Author

Ping @tmandry; if you have time, a review on this would be great!

@sfackler sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 2, 2020
@sfackler
Copy link
Member

sfackler commented Dec 2, 2020

LGTM, but @nikomatsakis did you want to wait for @tmandry to get a chance to review as well?

tmandry
tmandry approved these changes Dec 9, 2020
View reviewed changes
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/stream/mod.rs Show resolved Hide resolved
camelid
camelid reviewed Dec 24, 2020
View reviewed changes
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

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

A few documentation nits. Great docs by the way :)

library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
library/core/src/stream/mod.rs Outdated Show resolved Hide resolved
@olanod olanod mentioned this pull request Jan 4, 2021
Merged
@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented Jan 15, 2021 •
edited
Loading

Stream::next has temporarily been removed to reflect the changes in the RFC; I've added an entry to the tracking issue to ensure we resolve the dynamic dispatch issue before stabilization.

I've also implemented @camelid's feedback, and resolved the other remaining comments on this PR. This should be ready for review by the libs team.

@Thomasdezeeuw Thomasdezeeuw mentioned this pull request Jan 15, 2021
Closed
@yoshuawuyts
Add core::stream::Stream
0c8db16 
This patch adds the `core::stream` submodule and implements `core::stream::Stream` in accordance with RFC2996.

Add feedback from @camelid
@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented Jan 22, 2021 •
edited
Loading

Updated with @camelid's feedback on the docs!

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot) 
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.15s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
 Documenting core v0.0.0 (/checkout/library/core)
error: unresolved link to `<Item>`
  --> library/core/src/stream/mod.rs:46:25
   |
46 | //! yields [`Option`][][`<Item>`](Stream::Item).
   |                         ^^^^^^^^ missing type for generic parameters
   |
   = note: `-D broken-intra-doc-links` implied by `-D warnings`
error: aborting due to previous error

error: could not document `core`


Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustdoc --edition=2018 --crate-type lib --crate-name core library/core/src/lib.rs --target x86_64-unknown-linux-gnu -o /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/doc --error-format=json --json=diagnostic-rendered-ansi --markdown-css rust.css --markdown-no-toc -Z unstable-options --resource-suffix 1.51.0 --index-page /checkout/src/doc/index.md -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/release/deps --cfg=bootstrap -Dwarnings -Winvalid_codeblock_attributes --crate-version '1.51.0-nightly
  (d33b05f87
  2021-01-22)'` (exit code: 1)


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "-p" "core" "--" "--markdown-css" "rust.css" "--markdown-no-toc" "-Z" "unstable-options" "--resource-suffix" "1.51.0" "--index-page" "/checkout/src/doc/index.md"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap doc --stage 0 library/std
Build completed unsuccessfully in 0:00:09

@yoshuawuyts
Remove Stream::next
a1b1132 
This is a temporary change only, as we wait to resolve dynamic dispatch issues. The `Stream::next` method and corresponding documentation are expected to be fully restored once we have a path to proceed.

Ref: rust-lang/rfcs#2996 (comment)

update docs
@taiki-e taiki-e mentioned this pull request Jan 25, 2021
Merged
@KodrAus
Copy link
Contributor

KodrAus commented Jan 29, 2021

Looks like we're good to go here!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 29, 2021

📌 Commit a1b1132 has been approved by KodrAus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 29, 2021
This was referenced Jan 29, 2021
Closed
Closed
Closed
@jonas-schievink jonas-schievink mentioned this pull request Jan 29, 2021
Closed
@JohnTitor JohnTitor mentioned this pull request Jan 30, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2021
@bors
Auto merge of rust-lang#81545 - JohnTitor:rollup-zlt3tn6, r=JohnTitor
7ce1b3b 
Rollup of 16 pull requests

Successful merges:

 - rust-lang#79023 (Add `core::stream::Stream`)
 - rust-lang#80562 (Consider Scalar to be a bool only if its unsigned)
 - rust-lang#80886 (Stabilize raw ref macros)
 - rust-lang#80959 (Stabilize `unsigned_abs`)
 - rust-lang#81291 (Support FRU pattern with `[feature(capture_disjoint_fields)]`)
 - rust-lang#81409 (Slight simplification of chars().count())
 - rust-lang#81468 (cfg(version): treat nightlies as complete)
 - rust-lang#81473 (Warn write-only fields)
 - rust-lang#81495 (rustdoc: Remove unnecessary optional)
 - rust-lang#81499 (Updated Vec::splice documentation)
 - rust-lang#81501 (update rustfmt to v1.4.34)
 - rust-lang#81505 (`fn cold_path` doesn't need to be pub)
 - rust-lang#81512 (Add missing variants in match binding)
 - rust-lang#81515 (Fix typo in pat.rs)
 - rust-lang#81519 (Don't print error output from rustup when detecting default build triple)
 - rust-lang#81520 (Don't clone LLVM submodule when download-ci-llvm is set)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ecd7cb1 into rust-lang:master Jan 30, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 30, 2021
@m-ou-se m-ou-se mentioned this pull request Jan 30, 2021
Closed
@yoshuawuyts yoshuawuyts deleted the stream branch February 13, 2021 16:33
@driftluo driftluo mentioned this pull request Mar 23, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ratmice ratmice ratmice left review comments

@seanmonstar seanmonstar seanmonstar left review comments

@Nemo157 Nemo157 Nemo157 left review comments

@Keruspe Keruspe Keruspe left review comments

@nagisa nagisa nagisa left review comments

@Darksonn Darksonn Darksonn left review comments

@bluetech bluetech bluetech left review comments

@olanod olanod olanod left review comments

@sfackler sfackler sfackler left review comments

@tmandry tmandry tmandry approved these changes

@guswynn guswynn guswynn left review comments

@jyn514 jyn514 jyn514 left review comments

@camelid camelid camelid left review comments

@taiki-e taiki-e taiki-e left review comments

Assignees

@tmandry tmandry

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.51.0
Development

Successfully merging this pull request may close these issues.