-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow locals and destructuring in const fn #2341
Conversation
text/0000-const-locals.md
Outdated
|
||
## Why is this design the best in the space of possible designs? | ||
|
||
Being the only design makes it the best design by definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤣
text/0000-const-locals.md
Outdated
[guide-level-explanation]: #guide-level-explanation | ||
|
||
`let` bindings in constants and const fn work just like `let` bindings | ||
everywhere else. Historically these did not exist in constants and const fn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let
bindings "everywhere else" (normal fn
s) introduce bindings which have move semantics when typeof(binding) /: Copy
. Does this hold in const fn
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. All code is compiled normally in const fn. The only difference is that the execution is on virtual hardware instead of real hardware. So if your code fails to compile outside of a const environment, it will also fail to compile within a const environment. The reverse does not hold, there's a lot of code that compiles right now that will never compile in a const environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 👍. I'd include an explanation of this in the RFC text to make things crystal clear =)
text/0000-const-locals.md
Outdated
|
||
This means that you can only move out of any let binding once, even though in a | ||
const environment obtaining a copy of the object could be done by executing the | ||
code twice, side effect free. All invariants held by runtime code are also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const fn
are also callable from outside of a constant context, so it's not always true that their arguments can be reproduced. 1 for the RFC's approach.
@rfcbot fcp merge This is well-motivated, already implemented, and as far as I know it's really the only possible approach to having |
Team member @cramertj has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), 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. |
This one seems particularly obviously fine, since you can get new variables in const fn today -- just with absolutely terrible syntax because you have to define a new function to do so 😆 |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period is now complete. |
Huzzah! This RFC has been merged! Tracking issue: rust-lang/rust#48821 |
After rust-lang/rust#46882 adding
let
bindings and destructuring to const fn and blocks in constants is just the removal of a few checks. We want this some day in the future anyway. I don't see any reason not to do this.I propose we do this without adding an intermediate unstable
feature
and just go with it, but if there are any concerns raised about let bindings or destructuring, we could first add a feature for it.cc @eddyb
r? @nikomatsakis
Rendered
Tracking issue: rust-lang/rust#48821