Skip to content
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

std: Remove ?Sized bounds from many I/O functions #23316

Merged
merged 1 commit into from
Mar 15, 2015

Conversation

alexcrichton
Copy link
Member

It is a frequent pattern among I/O functions to take P: AsPath + ?Sized or
AsOsStr instead of AsPath. Most of these functions do not need to take
ownership of their argument, but for libraries in general it's much more
ergonomic to not deal with ?Sized at all and simply require an argument P
instead of &P.

This change is aimed at removing unsightly ?Sized bounds while retaining the
same level of usability as before. All affected functions now take ownership of
their arguments instead of taking them by reference, but due to the forwarding
implementations of AsOsStr and AsPath all code should continue to work as it
did before.

This is strictly speaking a breaking change due to the signatures of these
functions changing, but normal idiomatic usage of these APIs should not break in
practice.

[breaking-change]

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@bors
Copy link
Contributor

bors commented Mar 13, 2015

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

@aturon
Copy link
Member

aturon commented Mar 13, 2015

Lovely!

@bors: r+ 9a6d653

@bors
Copy link
Contributor

bors commented Mar 14, 2015

⌛ Testing commit 9a6d653 with merge f6c8476...

@bors
Copy link
Contributor

bors commented Mar 14, 2015

💔 Test failed - auto-mac-64-opt

@Manishearth
Copy link
Member

/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sys/unix/os.rs:256:37: 256:62 error: mismatched types:
 expected `ffi::os_str::OsString`,
    found `&_`
(expected struct `ffi::os_str::OsString`,
    found &-ptr) [E0308]
/Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/libstd/sys/unix/os.rs:256         Ok(PathBuf::new::<OsString>(&OsStringExt::from_vec(v)))

It is a frequent pattern among I/O functions to take `P: AsPath + ?Sized` or
`AsOsStr` instead of `AsPath`. Most of these functions do not need to take
ownership of their argument, but for libraries in general it's much more
ergonomic to not deal with `?Sized` at all and simply require an argument `P`
instead of `&P`.

This change is aimed at removing unsightly `?Sized` bounds while retaining the
same level of usability as before. All affected functions now take ownership of
their arguments instead of taking them by reference, but due to the forwarding
implementations of `AsOsStr` and `AsPath` all code should continue to work as it
did before.

This is strictly speaking a breaking change due to the signatures of these
functions changing, but normal idiomatic usage of these APIs should not break in
practice.

[breaking-change]
@alexcrichton
Copy link
Member Author

@bors: r=aturon

@bors
Copy link
Contributor

bors commented Mar 15, 2015

@bors r=aturon 60a4a2d

@bors
Copy link
Contributor

bors commented Mar 15, 2015

⌛ Testing commit 60a4a2d with merge 54660fc...

bors added a commit that referenced this pull request Mar 15, 2015
It is a frequent pattern among I/O functions to take `P: AsPath + ?Sized` or
`AsOsStr` instead of `AsPath`. Most of these functions do not need to take
ownership of their argument, but for libraries in general it's much more
ergonomic to not deal with `?Sized` at all and simply require an argument `P`
instead of `&P`.

This change is aimed at removing unsightly `?Sized` bounds while retaining the
same level of usability as before. All affected functions now take ownership of
their arguments instead of taking them by reference, but due to the forwarding
implementations of `AsOsStr` and `AsPath` all code should continue to work as it
did before.

This is strictly speaking a breaking change due to the signatures of these
functions changing, but normal idiomatic usage of these APIs should not break in
practice.

[breaking-change]
@bors bors merged commit 60a4a2d into rust-lang:master Mar 15, 2015
semarie added a commit to semarie/rust that referenced this pull request Mar 15, 2015
SimonSapin added a commit to servo/rust-url that referenced this pull request Mar 17, 2015
Ms2ger added a commit to servo/rust-png that referenced this pull request Mar 21, 2015
Ms2ger added a commit to servo/rust-png that referenced this pull request Mar 21, 2015
Ms2ger added a commit to servo/rust-png that referenced this pull request Mar 21, 2015
Ms2ger added a commit to servo/rust-png that referenced this pull request Mar 21, 2015
@alexcrichton alexcrichton deleted the less-question-sized branch March 27, 2015 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants