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

serde_json 1.0.3 broke Diesel's build #357

Closed
sgrif opened this issue Sep 5, 2017 · 3 comments
Closed

serde_json 1.0.3 broke Diesel's build #357

sgrif opened this issue Sep 5, 2017 · 3 comments
Labels

Comments

@sgrif
Copy link

sgrif commented Sep 5, 2017

The following code will compile successfully on its own, but fails if extern crate serde_json; is added where serde_json is 1.0.3. 1.0.2 will compile successfully.

fn returns_generic_vec_result<T>() -> Result<Vec<T>, ()> {
    Ok(Vec::new())
}

fn main() {
    let bools = vec![true, false];
    let stuff = returns_generic_vec_result().unwrap();
    assert_eq!(bools, stuff);
}

I believe that this commit is what broke things. That is technically a minor breaking change as defined in RFC #1105. That said, I personally think that implementing a trait that is not defined in your library for a type that is not defined in your library should be reserved for semver-major, as it can cause completely unrelated code to break in surprising and hard to debug ways (like this case). I figured I'd bring it to your attention either way

@dtolnay
Copy link
Member

dtolnay commented Sep 10, 2017

Thanks for reporting this! I will be more mindful of new trait impls in the future.

@RalfJung
Copy link

Cc @ehuss, should this be mentioned in the semver compatibility rules / guidelines?

@ehuss
Copy link

ehuss commented Dec 11, 2023

That's a weird one! I added it to the list at rust-lang/cargo#8736. It might already be covered by one of the other "implement a trait can break things" entries, but this seems subtly different. I'm not too familiar with the inference algorithm, so I'm not sure how best to express inference issues (there's also a question of whether or not we should consider inference changes as breaking).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants