Skip to content

Draft: Make into_parts methods on Vec associated functions #141509

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Draft: Make into_parts methods on Vec associated functions #141509

wants to merge 2 commits into from
+29 −20

Conversation

GoldsteinE
Copy link
Contributor

(doesn’t need review for now, posted to accompany stabilization report)

@rustbot
Copy link
Collaborator

rustbot commented May 24, 2025

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 24, 2025
@GoldsteinE GoldsteinE mentioned this pull request May 24, 2025
Open
2 tasks
@jieyouxu
Copy link
Member

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 24, 2025
@rustbot rustbot assigned BurntSushi and unassigned ibraheemdev May 24, 2025
@jieyouxu jieyouxu added the needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. label May 24, 2025
@GoldsteinE
Copy link
Contributor Author

GoldsteinE commented May 24, 2025 •
edited
Loading

This PR is basically just to link to these patches from #65816 (comment). This will need to be rebased after #141219 is merged, and it might not be needed at all if open questions are resolved the other way in the main issue. The proper place for FCP is probably also the tracking issue, although I’m not that familiar with the process.

GoldsteinE added 2 commits May 25, 2025 00:01
@GoldsteinE
assert_unsafe_precondition!(length <= capacity) in Vec::from_raw_parts
e3e6bc6 
This hopefully helps to catch cases where these arguments are
accidentally swapped.
@GoldsteinE
Make into_parts methods on Vec associated functions
efdd43d 
This is more consistent with `Box::into_raw()` and clears out potential
confusion about whether the method acts on a vector or on a slice.
@GoldsteinE GoldsteinE force-pushed the vec-into-raw-parts branch from 2bb1d8e to efdd43d Compare May 24, 2025 21:01
@Amanieu
Copy link
Member

Amanieu commented Jul 1, 2025

We discussed this in the @rust-lang/libs-api meeting. We felt that the main argument against making these associated function is that unlike Box/Arc/Rc, Vec doesn't Deref to an arbitrary T but instead to a slice type which has a fixed set of methods which we know this will not conflict with.

@rfcbot close

@rfcbot

This comment was marked as outdated.

Sign in to view
@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-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Jul 1, 2025
@Amanieu Amanieu removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 1, 2025
@Amanieu

This comment was marked as outdated.

Sign in to view
@rfcbot

This comment was marked as outdated.

Sign in to view
@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Jul 1, 2025
@Amanieu
Copy link
Member

Amanieu commented Jul 1, 2025

@rfcbot close

@rfcbot
Copy link
Collaborator

rfcbot commented Jul 1, 2025 •
edited by m-ou-se
Loading

Team member @Amanieu has proposed to close 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-close This PR / issue is in PFCP or FCP with a disposition to close it. 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 Jul 1, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented Jul 1, 2025

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

@bors
Copy link
Collaborator

bors commented Jul 3, 2025

☔ The latest upstream changes (presumably #143338) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@BurntSushi BurntSushi

Labels
disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. needs-fcp This change is insta-stable, or significant enough to need a team 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@GoldsteinE @rustbot @jieyouxu @Amanieu @rfcbot @bors @BurntSushi @ibraheemdev