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

Breaking Change - encode_utf8 destabilised #32460

Closed
thepowersgang opened this issue Mar 24, 2016 · 8 comments
Closed

Breaking Change - encode_utf8 destabilised #32460

thepowersgang opened this issue Mar 24, 2016 · 8 comments
Labels
regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@thepowersgang
Copy link
Contributor

The change 2b3c5acb1d41a870015c53825689e9ebe4b724e8 converted char::encode_utf8 from a stable method of an unstable trait to an unstable method.

The trait CharExt is imported as part of libcore's prelude, which can be done on the stable build (see https://play.rust-lang.org/?gist=7320c1273bdc3dde5450&version=stable).
Under current nightly, the same code fails to compile due to this same API.

This is technically a regression, and may need a crater run to check if it breaks anything (unlikely), and may need closer scrutiny.

@thepowersgang
Copy link
Contributor Author

cc #27784 @alexcrichton

@thepowersgang
Copy link
Contributor Author

Note, discussion in the PR #32204 on the relevant lines. Appears that breakage was acknowledged and assumed to not have any impact.

@bluss bluss added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Mar 24, 2016
@bluss
Copy link
Member

bluss commented Mar 24, 2016

It's a regression in the sense that it breaks / changes behavior of stable code (but this doesn't alone imply that, or imply that it not, needs to be fixed.)

@alexcrichton
Copy link
Member

triage: I-nominated

nominating for discussion in a libs triage meeting. @thepowersgang is this enough enough to work around for you? For example, do you have crates that build on stable which require this method?

Ideally we could let this slide, but if the breakage is too much we'll just have to bite the bullet and revert.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 24, 2016
@thepowersgang
Copy link
Contributor Author

The only breakage I had was on a nightly-only project, I just noted that I needed to add a feature flag to a crate that didn't need any originally (so no real problem at all).

I just created this ticket to make sure that it was noticed and properly documented.

@alexcrichton
Copy link
Member

Ok, thanks for the info and heads up! We'll discuss this at the next meeting and can decide what to do.

@thepowersgang
Copy link
Contributor Author

Note - I am all for the new API, so if this change doesn't impact anyone else then probably just as good to keep it.

@alexcrichton
Copy link
Member

The libs team discussed this regression during triage today and the decision was that this is acceptable for now. This is a clear case of fixing a bug. We'll do a crater run to analyze more crates and see what pops up, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants