implement const iterator using `rustc_do_not_const_check` by fee1-dead · Pull Request #106541 · rust-lang/rust

fee1-dead · 2023-01-06T19:04:42Z

Previous experiment: #102225.

Explanation: rather than making all default methods work under const all at once, this uses rustc_do_not_const_check as a workaround to "trick" the compiler to not run any checks on those other default methods. Any const implementations are only required to implement the next method. Any actual calls to the trait methods other than next will either error in compile time (at CTFE runs), or run the methods correctly if they do not have any non-const operations. This is extremely easy to maintain, remove, or improve.

rustbot · 2023-01-06T19:04:47Z

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

fee1-dead · 2023-01-13T14:05:06Z

cc @oli-obk for compiler changes

rustbot · 2023-01-13T14:05:24Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

onestacked · 2023-01-16T15:23:35Z

If this approach is chosen, how is the plan to remove the workaround later?
Will we start bringing some functions slowly from the "Actual" const implementation in while keeping the rest with #[rustc_do_not_const_check]?

fee1-dead · 2023-01-17T01:16:56Z

Will we start bringing some functions slowly from the "Actual" const implementation in while keeping the rest with #[rustc_do_not_const_check]?

Yes. Once const closures hit beta.

bors · 2023-01-25T20:55:51Z

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

m-ou-se · 2023-01-31T16:12:18Z

This seems okay, as long as it doesn't affect the stable behaviour of course. But, what safety mechanisms in place to make sure it doesn't affect stable behaviour, neither now nor (accidentally) in the future?

joshtriplett · 2023-01-31T16:15:09Z

Could we also have a test to make sure this only affects default method implementations, not the functions overriding those implementations in an impl Iterator for Type?

fee1-dead · 2023-01-31T16:46:35Z

This seems okay, as long as it doesn't affect the stable behaviour of course. But, what safety mechanisms in place to make sure it doesn't affect stable behaviour, neither now nor (accidentally) in the future?

Were you talking about the rustc_do_not_const_check attribute or the process of making Iterator const in general? For the attribute, no codegen/mir would be changed as it is only used by the const checker which will skip those marked with the attribute.

As for the process of making Iterator const, I think we can move forward with a conservative approach of only making it const without using workarounds that would make it harder to maintain. (e.g. safest would be only adding const as function modifier or for const closures) The changes to libcore should be easy to revert as to not add additional maintainer burden.

Could we also have a test to make sure this only affects default method implementations, not the functions overriding those implementations in an impl Iterator for Type?

Yes, I will add that in a moment

onestacked · 2023-01-31T16:47:47Z

A test to make that #[rustc_do_not_const_check] can't be used without the feature gate would probably also be good.

onestacked · 2023-02-04T15:50:05Z

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

    /// ```
    #[inline]
    #[stable(feature = "rust1", since = "1.0.0")]
+    #[rustc_do_not_const_check]


This is not needed as this is trivially const anyways.

onestacked · 2023-02-04T15:51:50Z

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

    /// ```
    #[inline]
    #[stable(feature = "rust1", since = "1.0.0")]
+    #[rustc_do_not_const_check]


Can be removed using const_closures

onestacked · 2023-02-04T15:54:34Z

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

    #[inline]
    #[stable(feature = "rust1", since = "1.0.0")]
+    #[rustc_do_not_const_check]
    fn last(self) -> Option<Self::Item>


Can be removed simply by making the inner function const.
In fact looking at this most of those seem to be no longer needed now that const_closures is in bootstrap.

fee1-dead · 2023-02-15T01:51:40Z

@chriss0612 I still think that this PR should be as minimal as possible and we can make followup PRs for it to do more things.

Are there any concerns that are still blocking this?

@rustbot ready

onestacked · 2023-02-15T12:44:55Z

@chriss0612 I still think that this PR should be as minimal as possible and we can make followup PRs for it to do more things.

Yeah that's fine, I mostly wanted to mention it.

thomcc · 2023-02-22T15:22:39Z

My understanding is this has gotten libs-api approval. The implementation seems fine to me, so

@bors r+

bors · 2023-02-22T15:22:46Z

📌 Commit 3aeb43c has been approved by thomcc

It is now in the queue for this repository.

…o, r=thomcc implement const iterator using `rustc_do_not_const_check` Previous experiment: rust-lang#102225. Explanation: rather than making all default methods work under `const` all at once, this uses `rustc_do_not_const_check` as a workaround to "trick" the compiler to not run any checks on those other default methods. Any const implementations are only required to implement the `next` method. Any actual calls to the trait methods other than `next` will either error in compile time (at CTFE runs), or run the methods correctly if they do not have any non-const operations. This is extremely easy to maintain, remove, or improve.

Rollup of 10 pull requests Successful merges: - rust-lang#106541 (implement const iterator using `rustc_do_not_const_check`) - rust-lang#106918 (Rebuild BinaryHeap on unwind from retain) - rust-lang#106923 (Restore behavior when primary bundle is missing) - rust-lang#108169 (Make query keys `Copy`) - rust-lang#108287 (Add test for bad cast with deferred projection equality) - rust-lang#108370 (std: time: Avoid to use "was created" in elapsed() description) - rust-lang#108377 (Fix ICE in 'duplicate diagnostic item' diagnostic) - rust-lang#108388 (parser: provide better suggestions and errors on closures with braces missing) - rust-lang#108391 (Fix `is_terminal`'s handling of long paths on Windows.) - rust-lang#108401 (diagnostics: remove inconsistent English article "this" from E0107) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

[rust-lang/rust#106541][1] added `#[const_trait]` to `Iterator` as well as internal attribute to bypass the constness check for other methods than `Iterator::next`. As a result, user code can now implement `const Iterator` in a normal way, i.e., just by implementing `Iterator::next`. [1]: rust-lang/rust#106541

@oli-obk

…k, r=oli-obk constify `Iterator`, take IV Like its predecessors (rust-lang#92433, rust-lang#102225, rust-lang#106541), this PR allows one to make `Iterator` implementations `const`, and thus enables the ability to have `for` loops in `const` contexts. I've also included constifying `Option as IntoIterator`, `option::IntoIter as Iterator` as a minimal dogfooding example. But unlike its predecessors, it uses a new attribute (not unsound anymore!) that prevents any `Iterator` extension methods from being called. This is intentionally made minimal for an initial approval, the fun stuff like `.fold`, `Range as Iterator` could be done later. cc @rust-lang/wg-const-eval, cc @oli-obk to review the compiler parts cc rust-lang#92476

@oli-obk

Rollup merge of #151281 - fee1-dead-contrib:push-zmqtzvuvlmuk, r=oli-obk constify `Iterator`, take IV Like its predecessors (#92433, #102225, #106541), this PR allows one to make `Iterator` implementations `const`, and thus enables the ability to have `for` loops in `const` contexts. I've also included constifying `Option as IntoIterator`, `option::IntoIter as Iterator` as a minimal dogfooding example. But unlike its predecessors, it uses a new attribute (not unsound anymore!) that prevents any `Iterator` extension methods from being called. This is intentionally made minimal for an initial approval, the fun stuff like `.fold`, `Range as Iterator` could be done later. cc @rust-lang/wg-const-eval, cc @oli-obk to review the compiler parts cc #92476

@oli-obk

constify `Iterator`, take IV Like its predecessors (rust-lang/rust#92433, rust-lang/rust#102225, rust-lang/rust#106541), this PR allows one to make `Iterator` implementations `const`, and thus enables the ability to have `for` loops in `const` contexts. I've also included constifying `Option as IntoIterator`, `option::IntoIter as Iterator` as a minimal dogfooding example. But unlike its predecessors, it uses a new attribute (not unsound anymore!) that prevents any `Iterator` extension methods from being called. This is intentionally made minimal for an initial approval, the fun stuff like `.fold`, `Range as Iterator` could be done later. cc @rust-lang/wg-const-eval, cc @oli-obk to review the compiler parts cc rust-lang/rust#92476

rustbot assigned thomcc Jan 6, 2023

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 Jan 6, 2023

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the no-const-check-no branch from f402cc9 to 4d31609 Compare January 7, 2023 19:26

This comment has been minimized.

Sign in to view

fee1-dead force-pushed the no-const-check-no branch from 4d31609 to 85ffe50 Compare January 13, 2023 13:27

fee1-dead marked this pull request as ready for review January 13, 2023 14:05

fee1-dead force-pushed the no-const-check-no branch from 85ffe50 to 87a2cbe Compare January 13, 2023 14:05

fee1-dead added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 13, 2023

oli-obk approved these changes Jan 13, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

the8472 mentioned this pull request Jan 14, 2023

Allow using Range as an Iterator in const contexts. #104100

Merged

thomcc added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jan 18, 2023

fee1-dead added S-waiting-on-team and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 27, 2023

fee1-dead added 2 commits January 31, 2023 17:09

implement const iterator using rustc_do_not_const_check

6bf2c4d

add and bless tests

3aeb43c

fee1-dead force-pushed the no-const-check-no branch from 87a2cbe to 3aeb43c Compare January 31, 2023 17:27

onestacked mentioned this pull request Feb 3, 2023

Experiment: Const iterrator range with #[rustc_do_not_const_check] #107647

Closed

onestacked reviewed Feb 4, 2023

View reviewed changes

m-ou-se removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Feb 7, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 15, 2023

fee1-dead removed the S-waiting-on-team label Feb 15, 2023

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 Feb 22, 2023

Dylan-DPC mentioned this pull request Feb 23, 2023

Rollup of 5 pull requests #108393

Closed

Dylan-DPC mentioned this pull request Feb 24, 2023

Rollup of 10 pull requests #108421

Merged

bors merged commit 8c135ee into rust-lang:master Feb 24, 2023

rustbot added this to the 1.69.0 milestone Feb 24, 2023

fee1-dead mentioned this pull request Mar 21, 2023

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

Open

3 tasks

tgross35 mentioned this pull request Oct 30, 2024

Remove do_not_const_check from Iterator methods #132368

Merged

compiler-errors mentioned this pull request Oct 31, 2024

Constifying large traits in libstd (i.e. Iterator) rust-lang/project-const-traits#15

Open

fee1-dead mentioned this pull request Jan 18, 2026

constify Iterator, take IV #151281

Merged

Uh oh!

Conversation

fee1-dead commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 6, 2023

Uh oh!

This comment has been minimized.

This comment has been minimized.

fee1-dead commented Jan 13, 2023

Uh oh!

rustbot commented Jan 13, 2023

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

onestacked commented Jan 16, 2023

Uh oh!

fee1-dead commented Jan 17, 2023

Uh oh!

bors commented Jan 25, 2023

Uh oh!

m-ou-se commented Jan 31, 2023

Uh oh!

joshtriplett commented Jan 31, 2023

Uh oh!

fee1-dead commented Jan 31, 2023

Uh oh!

onestacked commented Jan 31, 2023

Uh oh!

onestacked Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

onestacked Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

onestacked Feb 4, 2023

Choose a reason for hiding this comment

Uh oh!

fee1-dead commented Feb 15, 2023

Uh oh!

onestacked commented Feb 15, 2023

Uh oh!

thomcc commented Feb 22, 2023

Uh oh!

bors commented Feb 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

fee1-dead commented Jan 6, 2023 •

edited

Loading