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

Make Vec::len() and Vec::is_empty() const #78648

Closed
wants to merge 1 commit into from

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Nov 2, 2020

Since these can trivially be const, I applied rustc_const_stable right away. Is this okay?

@rust-highfive
Copy link
Collaborator

r? @cramertj

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 2, 2020

Since these can trivially be const, I applied rustc_const_stable right away. Is this okay?

@jplatte it means it has to go through FCP instead of only having a single reviewer approve. These seem like good candidates for const fn though.

@jyn514 jyn514 added A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 2, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Nov 2, 2020

Did you run into a situation where having these as const fn is useful? Or is this only for consistency with [T]s methods?

@jplatte
Copy link
Contributor Author

jplatte commented Nov 2, 2020

I made a bunch of types' parameterless constructors const (for no immediate gain as it turned out, but it worked) and it made sense to me that the methods on these types that check for an all-default / empty instance would also be const, but that didn't work because of is_empty not being const.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 4, 2020

(cc @rust-lang/wg-const-eval)

@RalfJung
Copy link
Member

RalfJung commented Nov 4, 2020

I seem to recall this was proposed before and we decided to wait until these methods are actually useful... but @oli-obk will probably remember better.^^

I personally do not think we should stabilize const fn that have no way of being useful. I have no objections to making them unstably const though.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2020

Previous iterations of this PR: #66462 and #65341 and some more discussion in #55278 (comment)

We closed all such PRs before, maybe we should leave a comment on these functions informing future constifiers that we want to delay making them const fn until we have rust-lang/const-eval#20 (or until someone comes up with a use case)

@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2020

I personally do not think we should stabilize const fn that have no way of being useful. I have no objections to making them unstably const though.

Oh, sure, we could make them unstably const fn and then mention in the tracking issue that we're blocking it on a use case being presented or const heap becoming that use case

@jplatte
Copy link
Contributor Author

jplatte commented Nov 4, 2020

Oh, sorry for not being very diligent with looking for existing PRs. I did some searching but obviously not enough (maybe it was just issues).

@jplatte jplatte closed this Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants