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

regression: AsRef inference breakage #82665

Closed
Mark-Simulacrum opened this issue Mar 1, 2021 · 5 comments
Closed

regression: AsRef inference breakage #82665

Mark-Simulacrum opened this issue Mar 1, 2021 · 5 comments
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.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

I'm not quickly finding the PR which introduced the new impls, but it looks like some AsRef impls were added in 1.51, and those are causing breakage as determined through beta crater. Usually we don't do anything here, but would be good to at least ping authors.

@Mark-Simulacrum Mark-Simulacrum added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Mar 1, 2021
@Mark-Simulacrum Mark-Simulacrum added this to the 1.51.0 milestone Mar 1, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Mar 1, 2021
@apiraino
Copy link
Contributor

apiraino commented Mar 3, 2021

I've tried bisecting this issue (if it helps):

searched nightlies: from nightly-2021-01-01 to nightly-2021-03-03
regressed nightly: nightly-2021-01-26
searched commits: from 1d0d76f to f4eb5d9
regressed commit: d3163e9

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2021-01-01 

somewhere in this rollup #81355

@m-ou-se
Copy link
Member

m-ou-se commented Mar 3, 2021

somewhere in this rollup #81355

Hm, none of the PRs in that rollup should affect AsRef for String, which is what all these regressions are about. There's only one trait implementation added in that rollup: impl Error for &(impl Error).

@JohnTitor
Copy link
Member

This shows the regression (playground):

use std::io;

pub fn foo() -> io::Result<i32> {
    Err(io::Error::new(
        io::ErrorKind::Other,
        format!("Error!").as_ref(),
    ))
}

fn main() {}

And yes, confirmed that #75180 is the cause.

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Mar 5, 2021
@LeSeulArtichaut
Copy link
Contributor

It looks like those were expected regressions (see #75180 (comment)).

@JohnTitor
Copy link
Member

Triage: Close this issue as an acceptable regression, we mentioned it in #75180 (comment) and consider it acceptable through FCP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

No branches or pull requests

6 participants