2

Implement DerefMut for PathBuf by zertosh · Pull Request #105018 · rust-lang/rus...

 1 year ago
source link: https://github.com/rust-lang/rust/pull/105018
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Implement DerefMut for PathBuf #105018

Conversation

Contributor

@zertosh zertosh commented Nov 28, 2022

Without this, there's no way to get a &mut Path from PathBuf without
going through into_boxed_path. This is relevant now that #105002 adds
PathBuf::as_mut_os_string and Path::as_mut_os_str.

Collaborator

rustbot commented Nov 28, 2022

r? @thomcc

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

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

Nov 28, 2022

Collaborator

rustbot commented Nov 28, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

Contributor

Author

zertosh commented Nov 28, 2022

Now sure if I should include these here or make separate PRs, but the following impls would also be useful now but are missing:

  • impl BorrowMut<Path> for PathBuf
  • impl AsMut<Path> for PathBuf
  • impl AsMut<Path> for Path
  • impl AsMut<OsStr> for Path
  • impl From<&mut Path> for PathBuf
  • impl From<&mut OsStr> for PathBuf

dtolnay

added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

and removed T-libs Relevant to the library team, which will review and decide on the PR/issue.

labels

Nov 28, 2022

Member

dtolnay commented Nov 28, 2022

Deref from &mut PathBuf to &mut Path seems non-problematic given that the same thing is already expressible less conveniently as:

fn mutate_path_buf(path_buf: &mut PathBuf) {
    let tmp: PathBuf = mem::take(path_buf);
    let mut tmp: Box<Path> = tmp.into_boxed_path();
    mutate_path(&mut *tmp);
    *path_buf = tmp.to_path_buf();
}

fn mutate_path(path: &mut Path) {
    // ...
}

@rust-lang/libs-api:
@rfcbot fcp merge

rfcbot commented Nov 28, 2022

edited by Amanieu

Team member @dtolnay has proposed to merge 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

added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it.

labels

Nov 28, 2022

inquisitivecrystal

added relnotes Marks issues that should be documented in the release notes of the next release. needs-fcp This change is insta-stable, so needs a completed FCP to proceed.

labels

Nov 30, 2022

rfcbot

added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label

Dec 6, 2022

rfcbot commented Dec 6, 2022

bellThis is now entering its final comment period, as per the review above. bell

rfcbot

removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label

Dec 6, 2022

fn from_inner_mut(inner: &mut OsStr) -> &mut Path {

// SAFETY: Path is just a wrapper around OsStr,

// therefore converting &mut OsStr to &mut Path is safe.

unsafe { &mut *(inner as *mut OsStr as *mut Path) }

I don't see a #[repr(transparent)] on the struct definition - I don't think this transmute is correct.

This is correct for standard library code.

rfcbot

added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting

and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised.

labels

Dec 16, 2022

rfcbot commented Dec 16, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Member

dtolnay commented Dec 16, 2022

@bors r+

Contributor

bors commented Dec 16, 2022

pushpin Commit 2c54178 has been approved by dtolnay

It is now in the queue for this repository.

bors

added S-waiting-on-bors 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

Dec 16, 2022

Contributor

bors commented Dec 16, 2022

hourglass Testing commit 2c54178 with merge 9c07efe...

Contributor

bors commented Dec 16, 2022

sunny Test successful - checks-actions
Approved by: dtolnay
Pushing 9c07efe to master...

bors

added the merged-by-bors This PR was explicitly merged by bors label

Dec 16, 2022

bors

merged commit 9c07efe into

rust-lang:master

Dec 16, 2022

11 checks passed

zertosh

deleted the path_buf_deref_mut branch

Dec 16, 2022

Collaborator

rust-timer commented Dec 16, 2022

Finished benchmarking commit (9c07efe): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

Cycles

This benchmark run did not return any relevant results for this metric.

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

Assignees

dtolnay

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors needs-fcp This change is insta-stable, so needs a completed FCP to proceed. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects

None yet

Milestone

1.68.0

Development

Successfully merging this pull request may close these issues.

None yet

9 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK