String::as_ref() Into<String> type inference regression #60958
Labels
regression-from-stable-to-beta
Performance or correctness regression from stable to beta.
T-libs-api
Relevant to the library API team, which will review and decide on the PR/issue.
Comments
jonas-schievink
added
I-nominated
regression-from-stable-to-nightly
|
Likely caused by #59825, but considered acceptable breakage. |
|
This breaks some crates that compiled before, e.g. |
|
These cases can be made to work again by locally elaborating types or the used impl (eg. using |
|
Specifically, since this is essentially inference breakage it is permitted
per our policies. If there is some reason this causes significant
difficulty for you, though, we'd like to hear about that and may reconsider.
|
pietroalbini
added
regression-from-stable-to-beta
|
@Mark-Simulacrum Thanks for your response. This doesn't cause any problem at all for me – I was just curious about what kind of breakage is permitted and what isn't. I found the commitments in this blog post as well. |
jonas-schievink
added
regression-from-stable-to-stable
|
I hit this problem today when trying to update to the latest nightly release when one of my dependencies (rustyline) depended on this behavior. Because of this I'm unable to update because I don't control the dependency. Edit: Looks like rustyline has already fixed this issue upstream. Sorry for the noise :) |
Tiwalun
added a commit
to probe-rs/probe-rs
that referenced
this issue
May 28, 2019
jtgeibel
added a commit
to jtgeibel/crates.io
that referenced
this issue
May 28, 2019
This will fix our CI on the beta branch, which is currently failing due to a known breaking inference change. See rust-lang/rust#60958 for more info. I have reviewed the [upstream changes] relative to our current version and see no issues with the changes there (although we could alternatively backport just the needed fix). [upstream changes]: lettre/lettre@v0.9.0...0ead3cd
jtgeibel
added a commit
to jtgeibel/crates.io
that referenced
this issue
May 29, 2019
This will fix our CI on the beta branch, which is currently failing due to a known breaking inference change. See rust-lang/rust#60958 for more info. I have reviewed the [upstream changes] relative to our current version and see no issues with the changes there (although we could alternatively backport just the needed fix). [upstream changes]: lettre/lettre@v0.9.0...0ead3cd
bors
added a commit
to rust-lang/crates.io
that referenced
this issue
May 29, 2019
Update to latest master for lettre and lettre_email This will fix our CI on the beta branch, which is currently failing due to a known breaking inference change. See rust-lang/rust#60958 for more info. I have reviewed the [upstream changes] relative to our current version and see no issues with the changes there (although we could alternatively backport just the needed fix). [upstream changes]: lettre/lettre@v0.9.0...0ead3cd
|
This libs team discussed this yesterday and agreed this falls within our policy. If anyone has any difficulty upgrading though or working through this, please let us know and we can try to help out. |
|
Triaging crater, this caused a number of regressions. Code-ified most of the detailed regression lists below to avoid pinging lots of folks who are not the root cause of the regression; I'm reopening this due to the extent of the ecosystem damage here and re-nominating. root: rustyline - 73 detected crates which regressed due to this; cc @kkawakam, @gwenn root: ucg - 2 detected crates which regressed due to this; cc @zaphar root: wee-rl - 1 detected crates which regressed due to this; cc @jblondin root: tcalc-rustyline - 1 detected crates which regressed due to this; cc @dubrowgn root: amethyst_assets - 6 detected crates which regressed due to this; cc @Xaeroxe, @torkleyy, @Rhuagh, @Jojolepro, @Moxinilian root: fbxcel - 1 detected crates which regressed due to this; cc @lo48576 root: fui - 1 detected crates which regressed due to this; cc @xliiv root: lettre_email - 13 detected crates which regressed due to this; cc @amousset root: liquid - 8 detected crates which regressed due to this; cc @johannhof root: redis-async - 5 detected crates which regressed due to this; cc @benashford |
|
Regarding |
|
Regarding |
|
cc @rust-lang/libs, this was reopened due to the impact found on crater (see above). We'll discuss this in the next triage meeting but figured y'all would want to be aware ahead of time. |
|
|
|
It's up to you, of course, but if you don't then it's likely that downstream crates which still depend on the 0.2.x series will stop compiling in 1.36+. |
Mark-Simulacrum
changed the title
|
FWIW in the build/test crater run which seems to hit more crates I'm seeing numerous regressions beyond even those listed above (if it'd be helpful, I can spend some time preparing a full list, pinging the relevant folks, etc.). However, I'm thinking that we should revert the change that caused this -- the impl doesn't seem sufficiently useful (essentially saving a |
|
Fixed in amethyst by amethyst/amethyst#1619 |
|
Fixed in fbxcel 0.2.1. |
|
We discussed this at libs triage again yesterday but reached the same conclusion as before. If anyone has difficulties in migrating though please let us know and we can try to help out! |
This was referenced Jun 13, 2019
|
I suspect this will catch a lot of users by surprise when stable is released. The |
|
There have been several questions about why stringVariable.asRef() no longer compiles, particularly with developers using older pinned versions of libraries. It certainly looks like there was an awareness that this change was going to cause some pain, but there was no mention of the type of errors in the 1.36 announcement or release notes that I saw. I also spent some time looking for documentation on what is considered acceptable breakage, and couldn't find it. (I was surprised it wasn't easy to find on the website.) Is it worth-while to communicate (more) about acceptable breakage? My thought is that when an issue this arises, at least acknowledge that 'acceptable breakage' was found during the beta automated testing, and provide at least a link to the GitHub issue in the release notes (and if more explanation is needed, perhaps a blog article). |
|
The stability commitments are explained in this blog post, which I linked in an earlier comment in this thread. I agree that this information deserves a more prominent location. |
|
Specifically AsRef will be hopefully better documented soon (#62586); we've mostly hesitated from doing so in the past because there's technically a ton of possible breakage that users might encounter that is similar to this -- any impl, method added may cause conflicts downstream. I do agree that some way of saying "here's how to fix that" would be great, or at least pointing users at what we already found. But we have no great place to do that today -- the releases.md file isn't a great place. Maybe we can start a page on forge.rust-lang.org or something along those lines. |
|
While I understand this type of breakage is acceptable and falls within stability guarantees, would it have made sense to first add a warning/deprecation notice for one or two releases prior to the actual change? Assuming that is doable without a large effort of course. |
|
There's no infrastructure to do that currently AFAIK. |
svenfoo
added a commit
to svenfoo/wasm-game-of-life
that referenced
this issue
Sep 11, 2019
The Travis CI builds keep pulling in liquid 0.15 even with this extra dependency. Unfortunately that version triggers an accepted regression in Rust, see rust-lang/rust#60958
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following code has been working with stable compiler (rustc 1.34.2), but doesn't compile with nightly (rustc 1.36.0-nightly (963184b 2019-05-18)).
code in playground
Error the nightly compiler is given:
However, stable compiler is happy with the code and prints the expected hello world.
Stable compiler:
Nightly compiler:
The text was updated successfully, but these errors were encountered: