Skip to content

Add feature gates for for and ? in consts - #87237

Merged
bors merged 5 commits into
rust-lang:masterfrom
jonas-schievink:const-for-and-try
Jul 30, 2021
Merged

Add feature gates for for and ? in consts#87237
bors merged 5 commits into
rust-lang:masterfrom
jonas-schievink:const-for-and-try

Conversation

@jonas-schievink

@jonas-schievinkjonas-schievink commented Jul 18, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

These operations seems relatively straightforward to support, and only seem to be blocked on impl const Trait.

I have included a working test for const_try, but const_for is currently unusable without reimplementing every single defaulted Iterator method, so I didn't do that.

(both features still need tracking issues before this is merged)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 18, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk

oli-obk commented Jul 18, 2021

Copy link
Copy Markdown
Contributor

but const_for is currently unusable without reimplementing every single defaulted Iterator method, so I didn't do that.

maybe #86857 helps here?

@jonas-schievink

jonas-schievink commented Jul 18, 2021

Copy link
Copy Markdown
ContributorAuthor

In theory, yeah, this works:

#![feature(const_refs_to_cell)]#![feature(const_mut_refs)]#![feature(const_fn_trait_bound)]#![feature(const_trait_impl)]traitIter:Copy + Sized{typeItem:Copy;fnnext(&mutself) -> Option<Self::Item>;#[default_method_body_is_const]fncount(mutself) -> usize{letmut count = 0;whileself.next().is_some(){
count += 1;}
count
}}#[derive(Copy,Clone)]structConstIter{next:Option<u8>,}implconstIterforConstIter{typeItem = u8;fnnext(&mutself) -> Option<u8>{matchself.next{Some(val) => {self.next = val.checked_add(1);Some(val)}None => None,}}}const _:() = {let c = ConstIter{next:Some(0)}.count();if c != 256{[()][4];}};

...but it requires a lot of feature gates and Copy bounds (since everything is generic) that wouldn't work on Iterator. Seems like there's still a bunch of work to do until iterators work in consts.

@oli-obk

oli-obk commented Jul 18, 2021

Copy link
Copy Markdown
Contributor

Then I would say we don't add a const_for feature gate, we can start figuring out const iterators without it.

@jonas-schievink

jonas-schievink commented Jul 18, 2021

Copy link
Copy Markdown
ContributorAuthor

But we'll need one eventually, right? Why not add it now?

@oli-obk

oli-obk commented Jul 18, 2021

Copy link
Copy Markdown
Contributor

Yea. Just right now it doesn't really get us anything actionable. I guess as long as we have a canary test showing the behaviour of the feature gate it should be ok to have an unusable feature gate. Just thought it unusual.

@bors

bors commented Jul 27, 2021

Copy link
Copy Markdown
Collaborator

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

This was referenced Jul 29, 2021
Open
Closed
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

bors commented Jul 29, 2021

Copy link
Copy Markdown
Collaborator

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

@usbalbin

usbalbin commented Jul 29, 2021

Copy link
Copy Markdown
Contributor

#86853 is a PR which might have some overlap with the try-operator part of this. My goal there was to try and constify the most trivial conversion impls to enable basic usage with Option and Result (however that turned out to be blocked on some more work on const_trait_impl)

@usbalbin

usbalbin commented Jul 29, 2021

Copy link
Copy Markdown
Contributor

Btw is #87576 a duplicate of #74935? :)

@jonas-schievink

jonas-schievink commented Jul 29, 2021

Copy link
Copy Markdown
ContributorAuthor

Yes, these tracking issues are pretty hard to discover

oli-obk
oli-obk reviewed Jul 29, 2021
View reviewed changes
Comment threadcompiler/rustc_feature/src/active.rs Outdated
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
oli-obk
oli-obk reviewed Jul 30, 2021
View reviewed changes
Comment threadsrc/test/ui/consts/const-for.stderr
Comment on lines +1 to +5
error[E0015]: calls in constants are limited to constant functions, tuple structs and tuple variants
--> $DIR/const-for.rs:5:14
|
LL | for _ in 0..5 {}
| ^^^^

@oli-obkoli-obkJul 30, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess we should start printing the function's name in the diagnostic. Very uninformative this error XD

@oli-obk

oli-obk commented Jul 30, 2021

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jul 30, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit c5a29f9 has been approved by oli-obk

@borsbors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2021
@borsbors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 30, 2021
@bors

bors commented Jul 30, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit c5a29f9 with merge 87dc824...

@bors

bors commented Jul 30, 2021

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 87dc824 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Jul 30, 2021
@bors
bors merged commit 87dc824 into rust-lang:masterJul 30, 2021
@rustbotrustbot added this to the 1.56.0 milestone Jul 30, 2021
This was referenced Jul 30, 2021
Merged
Merged
Merged
Closed
@jonas-schievink
jonas-schievink deleted the const-for-and-try branch July 30, 2021 15:09
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 30, 2021
@bors
Auto merge ofrust-lang#86853- usbalbin:const_try, r=oli-obk
c6007fd
Constify ?-operator for Result and Option
Try to make `?`-operator usable in `const fn` with `Result` and `Option`, see rust-lang#74935 . Note that the try-operator itself was constified in rust-lang#87237.
TODO
* [x] Add tests for const T -> T conversions
* [x] cleanup commits
* [x] Remove `#![allow(incomplete_features)]`
* [?] Await decision in rust-lang#86808 - I'm not sure
* [x] Await support for parsing `~const` in bootstrapping compiler
* [x] Tracking issue(s)? - rust-lang#88674
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obkoli-obkoli-obk left review comments

Assignees

@oli-obkoli-obk

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.56.0

Development

Successfully merging this pull request may close these issues.

7 participants

@jonas-schievink@rust-log-analyzer@oli-obk@bors@usbalbin@rust-highfive@rustbot