constify from_fn, try_from_fn, try_map, map by bend-n · Pull Request #147071 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation67 Commits3 Checks11 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
adds the const_array feature
reimplements try_map in more or less the same way
Status: This is awaiting some action (such as code changes or more information) from the author.
Relevant to the library team, which will review and decide on the PR/issue.
labels
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Status: This is awaiting some action (such as code changes or more information) from the author.
label
r? @ibraheemdev
rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
| else { |
|---|
| // SAFETY: this slice will contain only initialized objects. |
| unsafe { |
| x.array_mut.get_unchecked_mut(..x.initialized).assume_init_drop(); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What part of this isn't const evaluable? The get_unchecked_mut?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assume_init_drop, and changing it to be const had some rammifications for some reason, causing some tests to fail. #147071 (comment)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like just trivial stderr file changes. Feel free to just bless them.
But is that code path testable in const? Since it's only happening in a panic path, I think it's unreachable anyway in const contexts. They never unwind and never will unwind. So maybe just document that and move the entire drop method body into the non-const code path... Or can we keep it non-const and avoid any drop happening for it by "disarming it" in the success path?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its used in try_from_fn, so it can get dropped in const context?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right. Hmm. Wondering how to write an observable test for this. I think if you use a type with a reference to a Cell and in its const drop impl increment that cell, you should see in a const context that the cell isn't incremented if try_map errors/returns None.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah oops, wanted to write this test but forgot about it when I was on a computer.
Need to reinvestigate this to get it all back into cache.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It oughtnt really be needed now, as the behaviour is identical to the non const version?
The Miri subtree was changed
cc @rust-lang/miri
This comment has been minimized.
| where |
|---|
| F: [const] FnMut(T) -> U, |
| { |
| extern "rust-call" fn call_mut(&mut self, (i,): (usize,)) -> Self::Output { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be possible to ignore this i and just use moved, but, uh, i think that would be somewhat confusing.
This comment has been minimized.
This comment has been minimized.
Some changes occurred in src/tools/cargo
cc @ehuss
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
📌 Commit e3a2c23 has been approved by oli-obk
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
bors added a commit that referenced this pull request
Rollup of 8 pull requests
Successful merges:
- #147071 (constify from_fn, try_from_fn, try_map, map)
- #148930 (tweak editor configs)
- #149320 (-Znext-solver: normalize expected function input types when fudging)
- #149363 (Port the
#![windows_subsystem]attribute to the new attribute system) - #149378 (make run-make tests use 2024 edition by default)
- #149381 (Add
impl TrustedLenonBTree{Map,Set}iterators) - #149388 (remove session+blob decoder construction)
- #149390 (
rust-analyzersubtree update)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit that referenced this pull request
Rollup merge of #147071 - bend-n:const_array-ops, r=oli-obk
constify from_fn, try_from_fn, try_map, map
adds the const_array feature
reimplements try_map in more or less the same way
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the library team, which will review and decide on the PR/issue.