Make it possible to have `const` `impl`s for `Iterator` #92433

fee1-dead · 2021-12-30T11:58:20Z

Currently based on #92432. PR is unblocked and considered to be ready when #92432 is merged and all the issue numbers are updated.

rust-highfive · 2021-12-30T11:58:24Z

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

the8472 · 2021-12-30T18:04:23Z

library/core/src/iter/traits/iterator.rs

+        F: ~const FnMut(&Self::Item) -> R,
+        F: ~const Drop,
+        R: ~const Try<Output = bool>,
        R::Residual: Residual<Option<Self::Item>>,
-    {
-        #[inline]
-        fn check<I, V, R>(
-            mut f: impl FnMut(&I) -> V,
-        ) -> impl FnMut((), I) -> ControlFlow<R::TryType>
-        where
-            V: Try<Output = bool, Residual = R>,
-            R: Residual<Option<I>>,
-        {
-            move |(), x| match f(&x).branch() {
-                ControlFlow::Continue(false) => ControlFlow::CONTINUE,
-                ControlFlow::Continue(true) => ControlFlow::Break(Try::from_output(Some(x))),
-                ControlFlow::Break(r) => ControlFlow::Break(FromResidual::from_residual(r)),
-            }
+        <<R as Try>::Residual as Residual<Option<Self::Item>>>::TryType:
+            ~const Try<Output = Option<Self::Item>>,
+        <<R as Try>::Residual as Residual<Option<Self::Item>>>::TryType: ~const FromResidual,
+        Self::Item: ~const Drop,
+    {
+        pub struct Check<F>(F);
+
+        impl_const_closure! {
+            impl<F, I, V, R> const FnMut for Check<F>
+            where
+                F: ~const FnMut(&I) -> V,
+                F: ~const Drop,
+                I: ~const Drop,
+                V: ~const Try<Output = bool, Residual = R>,
+                R: Residual<Option<I>>,
+                R::TryType: ~const Try<Output = Option<I>>,
+                R::TryType: ~const FromResidual,


Holy boilerplate. 😬

Maybe we need some "everything ~const" sugar?

Adding ~const super traits could possibly make this easier, however it needs work and we need to wait for it to land in bootstrap to incoporate its changes.

library/core/src/iter/traits/collect.rs

oli-obk · 2022-01-11T15:11:56Z

library/core/src/ops/control_flow.rs

        match self {
-            ControlFlow::Continue(..) => None,
+            ControlFlow::Continue(_x) => None,


what's up with that? please add a HACK comment referencing the issue that will allow us to go back to the previous

See https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Exact.20semantics.20for.20match.20and.20drop.20glue.20emitted

Ah right, I remember this. Please open an issue for it and link it here

dtolnay

I think I would prefer not to land this change until rustdoc's handling of ~const bounds is in better shape. Iterator is such a central trait, it's used all the time by beginners, and many of its signatures are gnarly enough already prior to this PR that I think it's not the right tradeoff to introduce this much noise and glitchiness into it right now.

Now is the time to tap the brakes a little on filling the standard library with ~const and allow rustdoc to catch up a little. Maybe if the rustdoc folks aren't already on top of it, one of you would be willing to shift some attention over there?

Below is a representative screenshot of before and after. The before is completely within the reach of a beginner to understand. The after is totally unintelligible.

Someone needs to figure out what to do in rustdoc to avoid these API docs becoming unintelligible in the presence of ~const. Do ~const Drop bounds need to be signaled at all? Do ~const bounds need to be shunted behind an "expand" toggle or accordion or something?

@rust-lang/libs-api @rust-lang/libs @rust-lang/libs-contributors

BurntSushi · 2022-01-16T04:29:07Z

Not only is that signature unintelligible to beginners, it's also unintelligible to at least one libs-api member. I don't know what ~const is.

fee1-dead · 2022-01-16T05:14:20Z

With the current change and x doc --stage 1 most of the changes in rustdoc have been fixed. But there are still some regressions for some nightly apis: try_find and try_reduce.

The ~const modifier should not be shown at all. It is still experimental at this point and its syntax might change.

fee1-dead · 2022-01-22T06:56:11Z

Just concealing the ~const doesn't solve the comprehensibility issue here.

You just found another issue that was introduced in #92229. Basically that PR removes ~const Drop bounds but the whole where clause is still kept.

Would you be willing to engage with the rustdoc folks explicitly to work out a plan for reducing the negative impact of the ~const work on documentation?

Yes, but most of the work needed (for rustdoc) is done in this PR, and the best way forward would be changes from the language side i.e. ~const super traits, and ~const associated type bounds that removes the need for bounds like <T as IntoIterator>::IntoIter: ~const Iterator on some of these methods.

The specific regression has been fixed. After a thorough review I think the latest change only introduces minor noise on some methods and only major bound jargon on unstable try_reduce and try_find. Sorry for bothering you with back and forth reviewing, I should have reviewed the doc changes before I opened this PR :/

@rustbot ready

dtolnay

At this point I still don't find this an acceptable regression in the usability of the Iterator docs. I put more Before/After below. Someone from @rust-lang/libs-api should feel free to let me know if I am being too picky on this. I feel there needs to be more design work from the rustdoc side to find a way to simplify and present the more complex bounds that ~const seems to necessitate.

I'm going to remove this PR from review by converting it to a draft and I don't plan to review it again, but would ask the author to send any rustdoc work separately for review by the rustdoc folks, or guidance on how to proceed here. We can keep this PR open as a draft to hold rebases of the Iterator changes for comparison as any rustdoc fixes start landing in other PRs.

Before	After

fee1-dead · 2022-01-28T15:04:40Z

In order to make no changes to the library documentation, we need two things:

Small tweaks on rustdoc's side, Improve rustdoc const bounds #93412
Improve our traits' API by adding ~const in assoc type bounds and super traits. (To avoid inserting extra bounds at the generic functions that use those traits) Super traits require Allow trait A: ~const B #93429 to land in bootstrap first.

There is another option which could be easier: Add an attribute to configure and prevent rustdoc from showing the actual complexity and show a minimized version, rustdoc_show_these_generics_instead_of_def(where ...)

bors · 2022-01-29T19:39:33Z

☔ The latest upstream changes (presumably #93457) made this pull request unmergeable. Please resolve the merge conflicts.

…nds, r=GuillaumeGomez Improve rustdoc const bounds - Rustdoc no longer displays `~const` in trait bounds, because it currently means nothing for stable users, and because we still haven't decided on the final syntax yet. - Rustdoc will hide trait bounds where the trait is `Drop` AND it is `~const`, i.e. `~const Drop` bounds because it has no effect on stable users as well. - Because of additional logic that hides the whole `where` statement where it consists of `~const Drop` bounds (so it doesn't display `struct Foo<T>() where ;` like that), bounds that have no trait e.g. `where [T; N+1]: ;` are also hidden. Cherry-picked from rust-lang#92433.

Dylan-DPC · 2022-04-08T13:17:35Z

@fee1-dead any updates on this?

fee1-dead · 2022-04-09T00:30:19Z

@fee1-dead any updates on this?

currently blocked on several other changes

rust-highfive assigned dtolnay Dec 30, 2021

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 30, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 30, 2021

This comment has been minimized.

Sign in to view

the8472 reviewed Dec 30, 2021

View reviewed changes

fee1-dead force-pushed the const-it branch from 4bb140b to 25a457a Compare December 31, 2021 18:17

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the const-it branch from 25a457a to 659d669 Compare January 1, 2022 09:00

fee1-dead marked this pull request as ready for review January 1, 2022 12:30

fee1-dead changed the title ~~[Draft] Make it possible to have const impls for Iterator~~ Make it possible to have const impls for Iterator Jan 1, 2022

fee1-dead marked this pull request as draft January 1, 2022 12:31

This was referenced Jan 1, 2022

Tracking Issue for const forwarding impls for Fn family traits #92474

Closed

Tracking Issue for const Extend impls in libcore #92475

Open

Tracking Issue for const Iterator impls and related const fns #92476

Open

fee1-dead marked this pull request as ready for review January 1, 2022 13:18

oli-obk reviewed Jan 11, 2022

View reviewed changes

library/core/src/iter/traits/collect.rs Show resolved Hide resolved

oli-obk reviewed Jan 11, 2022

View reviewed changes

fee1-dead mentioned this pull request Jan 11, 2022

Modifying a single subpattern in match changes MIR dramatically #92766

Open

camelid added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 11, 2022

dtolnay requested changes Jan 16, 2022

View reviewed changes

dtolnay added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2022

fee1-dead force-pushed the const-it branch from 4299ddd to 6ea061f Compare January 16, 2022 05:06

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 16, 2022

fee1-dead removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jan 17, 2022

dtolnay added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 22, 2022

fee1-dead added 4 commits January 22, 2022 14:05

Make it possible to have const impls for Iterator

cc4c7b7

Update tracking issues

529148d

Do not display ~const in rustdoc

9a88652

Do not display hidden ~const Drop bounds

2114ec8

fee1-dead force-pushed the const-it branch from 6ea061f to 2114ec8 Compare January 22, 2022 06:46

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

This comment has been minimized.

Sign in to view

Bless rustdoc test

8bd2182

fee1-dead force-pushed the const-it branch from f24100e to 8bd2182 Compare January 22, 2022 16:32

yaahc mentioned this pull request Jan 24, 2022

Unstably constify impl<I: Iterator> IntoIterator for I #90602

Merged

dtolnay reviewed Jan 28, 2022

View reviewed changes

dtolnay marked this pull request as draft January 28, 2022 05:18

dtolnay removed their assignment Jan 28, 2022

fee1-dead mentioned this pull request Jan 28, 2022

Improve rustdoc const bounds #93412

Merged

fee1-dead added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2022

fee1-dead mentioned this pull request Nov 17, 2022

Allow const iterator implementations #102225

Closed

fee1-dead closed this Nov 21, 2022

fee1-dead deleted the const-it branch November 21, 2022 14:29

dtolnay self-assigned this Mar 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make it possible to have `const` `impl`s for `Iterator` #92433

Make it possible to have `const` `impl`s for `Iterator` #92433

fee1-dead commented Dec 30, 2021

rust-highfive commented Dec 30, 2021

This comment has been minimized.

the8472 Dec 30, 2021

fee1-dead Dec 31, 2021

This comment has been minimized.

oli-obk Jan 11, 2022

fee1-dead Jan 11, 2022

oli-obk Jan 11, 2022

dtolnay left a comment •

edited

Loading

BurntSushi commented Jan 16, 2022

fee1-dead commented Jan 16, 2022

fee1-dead commented Jan 22, 2022

This comment has been minimized.

This comment has been minimized.

dtolnay left a comment

fee1-dead commented Jan 28, 2022

bors commented Jan 29, 2022

Dylan-DPC commented Apr 8, 2022

fee1-dead commented Apr 9, 2022

Make it possible to have const impls for Iterator #92433

Make it possible to have const impls for Iterator #92433

Conversation

fee1-dead commented Dec 30, 2021

rust-highfive commented Dec 30, 2021

This comment has been minimized.

the8472 Dec 30, 2021

Choose a reason for hiding this comment

fee1-dead Dec 31, 2021

Choose a reason for hiding this comment

This comment has been minimized.

oli-obk Jan 11, 2022

Choose a reason for hiding this comment

fee1-dead Jan 11, 2022

Choose a reason for hiding this comment

oli-obk Jan 11, 2022

Choose a reason for hiding this comment

dtolnay left a comment • edited Loading

Choose a reason for hiding this comment

BurntSushi commented Jan 16, 2022

fee1-dead commented Jan 16, 2022

fee1-dead commented Jan 22, 2022

This comment has been minimized.

This comment has been minimized.

dtolnay left a comment

Choose a reason for hiding this comment

fee1-dead commented Jan 28, 2022

bors commented Jan 29, 2022

Dylan-DPC commented Apr 8, 2022

fee1-dead commented Apr 9, 2022

Make it possible to have `const` `impl`s for `Iterator` #92433

Make it possible to have `const` `impl`s for `Iterator` #92433

dtolnay left a comment •

edited

Loading