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 RFC 2341, Allow locals and destructuring in const fn #48821

Closed
4 tasks done
Centril opened this issue Mar 7, 2018 · 22 comments
Closed
4 tasks done
Assignees
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Mar 7, 2018

This is a tracking issue for the RFC "Allow locals and destructuring in const fn" (rust-lang/rfcs#2341).

Steps:

Unresolved questions:

None.

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Mar 7, 2018
@eddyb
Copy link
Member

eddyb commented Mar 7, 2018

just remove the few checks

It's a bit trickier, because rustc_mir::transform::qualify_consts needs to track local variables just like it does only temporaries right now, but it shouldn't

Also, was let in other const contexts in the RFC, i.e. const X: i32 = { let x = 2 + 3; x * x};?

@Centril
Copy link
Contributor Author

Centril commented Mar 7, 2018

@eddyb Feel free to edit the issue description =)

Also, was let in other const contexts in the RFC, i.e. const X: i32 = { let x = 2 + 3; x * x}; ?

Yes, because (quoting the summary):

Allow let bindings in the body of constants and const fns. Additionally enable destructuring in let bindings and const fn arguments.

@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. and removed B-RFC-implemented Blocker: Approved by a merged RFC and implemented. labels Mar 7, 2018
@bstrie
Copy link
Contributor

bstrie commented Mar 8, 2018

A question about the "Drawbacks" section from the original RFC:

You can create mutable locals in constants and then actually modify them. This has no real impact on the constness, as the mutation happens entirely at compile time and results in an immutable value.

Can you elaborate on this? Is this just referring to something like const foo: i32 = { let mut x = 0; x += 1; x };? That's my straightforward reading of the text, but it also seems completely expected so I'm not sure whether there's a more pernicious drawback I ought to be seeing.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 8, 2018

Not a drawback really, but a note that this is new behaviour, because before you could never modify the same memory twice, only emulate the same program behaviour via const fn.

bors added a commit that referenced this issue May 22, 2018
Allow let bindings and destructuring in constants and const fn

r? @eddyb

cc #48821
@oli-obk oli-obk added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Jun 2, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jun 2, 2018

This works now on nightly (behind a feature gate). Please play around with this feature liberally and report any findings! Any help in documenting and pushing it over the stabilization line is appreciated.

@nexplor

This comment has been minimized.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 25, 2018

Imo it would be fine if added behind its own feature gate. So if you're looking for something easy to do:

  1. add a new feature gate (just search for const_let and copy all the defining parts)
  2. wrap https://github.com/rust-lang/rust/blob/master/src/librustc_mir/transform/qualify_consts.rs#L276 in a feature gate check (so if the feature gate is active, then that line needs to be not called)
  3. Add regression tests

@dtolnay
Copy link
Member

dtolnay commented Aug 20, 2018

Short-circuiting operators need to be fixed before stabilizing const_let. Otherwise it is possible to observe wrong results when changing a fn to const fn. #53515

@oli-obk
Copy link
Contributor

oli-obk commented Aug 20, 2018

I already added it to the OP ;)

@dtolnay

This comment has been minimized.

@qwerty19106

This comment has been minimized.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 21, 2018

We could stabilize let bindings in const fn (but not in const/static), because const functions do not have the issue that && and || short circuit (since these operations are illegal in const functions until control flow in general is allowed).

Due to backwards compatibility constraints we cannot make && and || illegal in constants and statics.

cc @Centril

@Centril
Copy link
Contributor Author

Centril commented Nov 22, 2018

@oli-obk Hmm... there are a bunch of bug reports cited in #48821 (comment)... we'd have to feel comfortable with the implementation before stabilizing.

However, my main question re. stabilizing is whether stabilizing let without control flow would bring much benefit without any control flow? What is it that we "unlock" by stabilizing in const fn only?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2018

I have a fix ready for these bugs in #56070

However, my main question re. stabilizing is whether stabilizing let without control flow would bring much benefit without any control flow? What is it that we "unlock" by stabilizing in const fn only?

You can do things like

fn bar() -> [u32; 500] {
    let mut foo = [0; 500];
    foo[3] = 42;
    foo
}

Or just generally make your code more readable in case the return expression is very complex.

@Centril
Copy link
Contributor Author

Centril commented Nov 22, 2018

@oli-obk

I have a fix ready for these bugs in #56070

Cool. :)

Or just generally make your code more readable in case the return expression is very complex.

I somehow doubt expressions can become complex without any sort of control flow but I suppose it doesn't hurt. Clearly enough time has passed since it was landed in nightly...

Could you? prepare:

  1. a special new issue or alternatively a stabilization PR (the latter is my preference)
  2. with a report that points out:
    0. when the initial implementation landed in nightly
    1. relevant tests
    2. what exactly is being stabilized
    3. what is not being stabilized
      1. and why
    4. any divergences from the RFC (there are such because of c) )
  3. change this tracking issue with a checkbox pointing out the remaining work after the initial stabilization

@Centril

This comment has been minimized.

@oli-obk

This comment has been minimized.

@Centril

This comment has been minimized.

@mjbshaw
Copy link
Contributor

mjbshaw commented Dec 29, 2018

Does this tracking issue include mutable references? Or is that a separate RFC/tracking issue?

@oli-obk
Copy link
Contributor

oli-obk commented Dec 29, 2018

Mutable references require their own full rfc imo. There are so many edge cases. I have started tracking some in rust-lang/const-eval#16

@Centril
Copy link
Contributor Author

Centril commented Dec 30, 2018

Stabilization proposed: #57175 (comment)

@Centril
Copy link
Contributor Author

Centril commented Jan 12, 2019

Stabilized in #57175; Documentation issue: rust-lang/reference#506

Everything is done so closing therefore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants