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

Add AsciiExt::into_ascii_{upper,lower}case #31335

Merged
merged 1 commit into from
Mar 1, 2016

Conversation

SimonSapin
Copy link
Contributor

The default implementations (with where Self: Sized) are so that methods that take self by value can exist in a trait that’s implemented for dynamically-sized types (str and [u8]).

CC #27809 (comment)
CC @alexcrichton

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

The default implementations (with where Self: Sized) are so that methods
that take `self` by value can exist in a trait that’s implemented for
dynamically-sized types (`str` and `[u8]`).

CC rust-lang#27809 (comment)
@alexcrichton
Copy link
Member

This basically needs to be decided in tandem with #27809 due to impls of traits being insta-stable. This seems fine to me, though.

@SimonSapin
Copy link
Contributor Author

Sure, this PR is mostly meant as input for the discussion in #27809.

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

The libs team decided to punt discussion of this until FCP for #27809 comes to a close, so removing the T-libs tag

@alexcrichton alexcrichton removed the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 11, 2016
@alexcrichton
Copy link
Member

@bors: r+ 700ac0e

See #27809 (comment)

Thanks @SimonSapin!

@bors
Copy link
Contributor

bors commented Feb 25, 2016

⌛ Testing commit 700ac0e with merge 86ca437...

@bors
Copy link
Contributor

bors commented Feb 26, 2016

💔 Test failed - auto-linux-64-opt

@alexcrichton
Copy link
Member

@bors: retry

ummm... what?

On Thu, Feb 25, 2016 at 4:53 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-linux-64-opt
http://buildbot.rust-lang.org/builders/auto-linux-64-opt/builds/8185


Reply to this email directly or view it on GitHub
#31335 (comment).

@bors
Copy link
Contributor

bors commented Feb 26, 2016

⌛ Testing commit 700ac0e with merge 91f1c22...

@bors
Copy link
Contributor

bors commented Feb 26, 2016

💔 Test failed - auto-linux-64-nopt-t

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Mar 1, 2016

⌛ Testing commit 700ac0e with merge 9af57bc...

@bors
Copy link
Contributor

bors commented Mar 1, 2016

💔 Test failed - auto-win-gnu-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Mon, Feb 29, 2016 at 8:40 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-win-gnu-64-opt
http://buildbot.rust-lang.org/builders/auto-win-gnu-64-opt/builds/3259


Reply to this email directly or view it on GitHub
#31335 (comment).

@bors
Copy link
Contributor

bors commented Mar 1, 2016

⌛ Testing commit 700ac0e with merge 0a52494...

bors added a commit that referenced this pull request Mar 1, 2016
The default implementations (with `where Self: Sized`) are so that methods that take `self` by value can exist in a trait that’s implemented for dynamically-sized types (`str` and `[u8]`).

CC #27809 (comment)
CC @alexcrichton
@bors bors merged commit 700ac0e into rust-lang:master Mar 1, 2016
@SimonSapin SimonSapin deleted the ascii-into branch March 1, 2016 13:14
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 17, 2016
The addition of these methods in rust-lang#31335 required adding impls of the trait for
the `String` and `Vec<T>` types. This unfortunately caused a regression (rust-lang#32074)
in type inference for using these methods which the libs team has decided to not
push forward with. These methods were stabilized in rust-lang#32020 which was intended to
get backported to beta, but the backport hasn't happened just yet. This commit
reverts both the addition and stabilization of these methods.

One proposed method of handling this, in rust-lang#32076, was to move the methods to an
extra trait to avoid conflicts with type inference. After some discussion,
however, the libs team concluded that we probably want to reevaluate what we're
doing here, so discussion will continue on the tracking issue, rust-lang#27809.
bors added a commit that referenced this pull request Mar 19, 2016
std: Revert addition of `into_ascii_*` methods

The addition of these methods in #31335 required adding impls of the trait for
the `String` and `Vec<T>` types. This unfortunately caused a regression (#32074)
in type inference for using these methods which the libs team has decided to not
push forward with. These methods were stabilized in #32020 which was intended to
get backported to beta, but the backport hasn't happened just yet. This commit
reverts both the addition and stabilization of these methods.

One proposed method of handling this, in #32076, was to move the methods to an
extra trait to avoid conflicts with type inference. After some discussion,
however, the libs team concluded that we probably want to reevaluate what we're
doing here, so discussion will continue on the tracking issue, #27809.

Closes #32074
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 24, 2016
The addition of these methods in rust-lang#31335 required adding impls of the trait for
the `String` and `Vec<T>` types. This unfortunately caused a regression (rust-lang#32074)
in type inference for using these methods which the libs team has decided to not
push forward with. These methods were stabilized in rust-lang#32020 which was intended to
get backported to beta, but the backport hasn't happened just yet. This commit
reverts both the addition and stabilization of these methods.

One proposed method of handling this, in rust-lang#32076, was to move the methods to an
extra trait to avoid conflicts with type inference. After some discussion,
however, the libs team concluded that we probably want to reevaluate what we're
doing here, so discussion will continue on the tracking issue, rust-lang#27809.

Conflicts:
	src/libstd/ascii.rs
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