Skip to content

Default for &Path #40378

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

Closed
wants to merge 1 commit into from
Closed

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented Mar 9, 2017

This was separated from #40009.

This has precedence due to the fact that PathBuf and Box<Path> already implement Default.

@rust-highfive
Copy link
Contributor

r? @brson

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

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 9, 2017
@alexcrichton
Copy link
Member

We discussed this during libs triage today and the conclusion was that due to the surprising semantics of Path::new("") we'd prefer to avoid this implementation for now, so I'm going to close this.

@clarcharr we also consider the addition of Default for Box<Path> to be an accidental mistake (we didn't mean to let that through), would you mind sending a PR to remove it?

@clarfonthey clarfonthey deleted the path_default branch March 15, 2017 14:03
@aturon
Copy link
Member

aturon commented Mar 15, 2017

Oops, I saw @alexcrichton's comment too late, and already submitted a PR to remove the existing impl.

@clarfonthey clarfonthey restored the path_default branch March 15, 2017 16:49
@clarfonthey clarfonthey deleted the path_default branch March 15, 2017 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants