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.

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

Tracking Issue for constifying some {BTreeMap,Set} functions #71835

Open
1 of 2 tasks
LG3696 opened this issue May 3, 2020 · 18 comments
Open
1 of 2 tasks

Tracking Issue for constifying some {BTreeMap,Set} functions #71835

LG3696 opened this issue May 3, 2020 · 18 comments
Labels
A-collections Area: std::collections. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@LG3696
Copy link
Contributor

LG3696 commented May 3, 2020

This tracks stabilization of the const_btree_new feature, which allows calling the below in a constant expression:

Steps

@LG3696 LG3696 added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label May 3, 2020
@jonas-schievink jonas-schievink added A-collections Area: std::collections. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. labels May 3, 2020
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 29, 2020
m-ou-se added a commit to m-ou-se/rust that referenced this issue Oct 31, 2020
Constantify more BTreeMap and BTreeSet functions

Just because we can:

- `BTreeMap::len`
- `BTreeMap::is_empty`
- `BTreeSet::len`
- `BTreeSet::is_empty`

Note that I put the `const` under `const_btree_new`, because I don't think their is a need to create another feature flag for that.

cc rust-lang#71835
m-ou-se added a commit to m-ou-se/rust that referenced this issue Oct 31, 2020
Constantify more BTreeMap and BTreeSet functions

Just because we can:

- `BTreeMap::len`
- `BTreeMap::is_empty`
- `BTreeSet::len`
- `BTreeSet::is_empty`

Note that I put the `const` under `const_btree_new`, because I don't think their is a need to create another feature flag for that.

cc rust-lang#71835
@a1phyr
Copy link
Contributor

a1phyr commented Oct 31, 2020

Can the following functions be added to the top statement ?

  • BTreeMap::len
  • BTreeMap::is_empty
  • BTreeSet::len
  • BTreeSet::is_empty

@JohnTitor JohnTitor changed the title Tracking Issue for making BTreeMap::new() and BTreeSet::new() const functions Tracking Issue for constifying some BTreeMap functions Mar 18, 2021
@JohnTitor JohnTitor changed the title Tracking Issue for constifying some BTreeMap functions Tracking Issue for constifying some {BTreeMap,Set} functions Mar 18, 2021
@tvladyslav
Copy link
Contributor

@jonas-schievink @JohnTitor , could you please start FCP?
I'm about to add closing PR. There were some tests added in #71839 and #78581 but now they are gone. Should I add more tests?

@Noratrieb
Copy link
Member

What's the state of this?

@JohnTitor
Copy link
Member

cc @rust-lang/libs-api --^

@Amanieu
Copy link
Member

Amanieu commented Aug 12, 2022

I'm 100% in favor of making BTreeMap::new const, but len/is_empty seem a bit much given that we don't even have those as const on Vec.

So I'm starting an FCP just for constifying the new function.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Aug 12, 2022

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 12, 2022
@jplatte
Copy link
Contributor

jplatte commented Aug 17, 2022

len/is_empty seem a bit much given that we don't even have those as const on Vec.

This seemed wrong to me, so I looked it up. Apparently the [T] methods are const, but Vec has its own len and is_empty¹ that aren't const. I also discovered two rustdoc bugs while looking this up 😅

¹ so vec.len() is not exactly equivalent to (*vec).len() for some reason

@dtolnay
Copy link
Member

dtolnay commented Aug 17, 2022

for some reason

Actually a very good reason, if I recall correctly. vec.len() can get the len right out of the Vec's len field without materializing a reference to the heap data. Whereas taking the &Vec, derefing it to &[T], and then getting len via slice's len() method would needlessly invalidate all outstanding raw pointers into the heap data which are later used for mutable access.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 13, 2022
@rfcbot
Copy link

rfcbot commented Sep 13, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 23, 2022
@rfcbot
Copy link

rfcbot commented Sep 23, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

fee1-dead added a commit to fee1-dead-contrib/rust that referenced this issue Sep 26, 2022
…lacrum

Stabilize const `BTree{Map,Set}::new`

The FCP was completed in rust-lang#71835.

Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this issue Sep 26, 2022
…lacrum

Stabilize const `BTree{Map,Set}::new`

The FCP was completed in rust-lang#71835.

Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 29, 2022
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 6, 2022
…lacrum

Stabilize const `BTree{Map,Set}::new`

The FCP was completed in rust-lang#71835.

Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
@Stebalien
Copy link
Contributor

This was stabilized in 1.66.

@Amanieu Amanieu closed this as completed Jun 30, 2023
@Amanieu Amanieu reopened this Jun 30, 2023
@Amanieu
Copy link
Member

Amanieu commented Jun 30, 2023

Actually len and is_empty are still const-unstable, keeping this open.

@Stebalien
Copy link
Contributor

Ah, sorry about that.

@bitdivine
Copy link

Hello all. This looks interesting, but is it possible to create a const BTreeMap with data? As far as I can see, new() is the only const fn for BTreeMap.

@jplatte
Copy link
Contributor

jplatte commented Jul 20, 2023

No, that depends on #79597, which doesn't even have an experimental implementation yet AFAIK.

@bitdivine
Copy link

Thanks for the info and pointer, @jplatte .

@bitdivine
Copy link

bitdivine commented Jul 20, 2023

I do wonder why this is ties to heap allocation though. A const, immutable BTree map would not have to live on the heap. I guess my mental model of const mem = a heap computed at compile time and immutable thereafter is wrong.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 20, 2023

This is about the heap emulation during compile time. We need to figure out some way to store the data while doing const evaluation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: std::collections. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests