rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Limiting `upstream crates may add new impl of trait in future versions`

Open p-avital opened this issue 6 years ago • 20 comments

Hello,

I've run into this one a few times now when working with generics, and I haven't really found a way to bypass it in most cases.

I completely get why this is here, "be careful, your behaviour may change if you update your crates, it may even stop compiling altogether" seems like a fair thing to warn against.

BUT: most of the time, when this error appears, it's for types that have been stabilised a while ago, and are way more likely never to add new implementations.

Wouldn't it be a good thing to be able to tell Rust "Yes, I know, this might conflict in the future, but with the current version, it's fine, so let me build until there ARE conflicts" ?

Rust's generics through traits and monomorphisation is one of my favourite features, but sometimes, in situations like this, Rust is TOO smart for its own good, and I think there should be ways to temper it.

This would also help with the problems that have been mentioned in the context of specialization and negative reasoning (maybe this could even enable negative reasoning which has the advantage of being more flexible and expressive than the currently selected "default" oriented specialisation), since breaking changes due to new implementations have always been limiting factors for them.

p-avital avatar Sep 05 '19 13:09 p-avital

This restriction is really strict, sometimes backward compatibility is not possible, let alone forward compatibility.

s977120 avatar Nov 27 '19 12:11 s977120

In unstable, one can circumvent this by telling the compiler that they won’t add new blanket impls for the type using #[fundamental]: rust-lang/rust#29635

So if fundamental is stabilized, this can be an error. Until then this needs to be a warning.

flying-sheep avatar Dec 26 '19 21:12 flying-sheep

I actually hardly understand this. If I want to build something that fits right away, why should I bother about something theoretically possible in the future? I'm fine with dealing the impl conflicts once they actually appear. Also I was asking regarding the usage of #[fundamental] in a non-compiler code and got a strict disallow.

snuk182 avatar Jan 27 '20 16:01 snuk182

You can work around this with macro (impl_xxx!(TypeA, TypeB, ...)) - that way you're not using trait constraints because you are explicit.

But this is something which definitely should be fixed in language. I've seen it so many times already it's obvious that this was bad idea.

BTW: my recent case was using Pod from bytemuck - I wanted to impl something for all types which are "binary-safe" and then add extra impl for String and boom, impossible. So I can duplicate the trait myself or add a macro. Again. I'm writing macros all the time and it feels wrong.

cztomsik avatar Feb 13 '21 15:02 cztomsik

I guess it's worth pointing to this tracking issue about negative impls: https://github.com/rust-lang/rust/issues/68318

cmpute avatar Mar 07 '22 18:03 cmpute

I'll leave here an example i've found if this "feature" causing problems

pub trait Responseable {
    fn into_stream (self) -> Box<dyn Stream<Item = u8>>;
}

impl<S: 'static + Stream<Item = u8>> Responseable for S {
    #[inline(always)]
    fn into_stream (self) -> Box<dyn Stream<Item = u8>> {
        Box::new(self)
    }
}

// conflicting implementations of trait `server::response::Responseable` for type `()`.
// upstream crates may add a new impl of trait `futures::Stream` for type `()` in future versions
impl Responseable for () {
 #[inline(always)]
    fn into_stream (self) -> Box<dyn Stream<Item = u8>> {
        Box::new(futures::stream::empty())
    }
}

As you see, rust is basically saying me "well, maybe someone sometime tries to do the same as you, so you can't do it". This causes problems very often for no reason, and it should probably be a warning, not an error

Aandreba avatar Apr 22 '22 18:04 Aandreba

recent case was using Pod from bytemuck

^^ Exact same thing here, I try to implement a trait to use bytemuck::cast_slice for all Pod + Zeroable type. And the problem appear when I add for Vec<T>

gcollombet avatar Oct 08 '22 15:10 gcollombet

This is bizarre, why is this still a thing? Literally anything can be changed and broken in a future version of a crate why pick this in particular???

SalsaGal avatar Dec 20 '22 13:12 SalsaGal

I guess it's because new trait implementations are not commonly considered breaking changes (even though they can be).

Suppose your crate b depends on crate a at version 1.0. You implement a trait for a type within a that is not conflicting in the current version. Later on, a releases a breaking change in minor version 1.1 by adding a trait implementation that conflicts with your implementation. If you specified a's version as 1.0 and not ~1.0 or =1.0, a user that depends on your crate would use version 1.1 by default, in which case your crate no longer compiles.

It still feels unnecessarily strict to disallow all implementations that might conflict in the future, though. My use case is that I want my API to work with anything that implements FromStr, but also with time::OffsetDateTime which doesn't implement FromStr, among others. I thought I could solve this with a new trait similar to FromStr, with a blanket implementation for anything that already implements FromStr, along with separate implementations for OffsetDateTime and other types. But that is considered conflicting. As a workaround, I have to make a newtype for OffsetDateTime and implement FromStr for it, which is much less convenient for users of the API.

IMO, a warning that an implementation might conflict in the future would be enough. The warning can be silenced with an annotation, and that annotation would immediately make clear what the problem is if a conflicting implementation is ever added. The warning could also remind you to use a stricter version range for the dependency.

Tortoaster avatar Jun 27 '23 10:06 Tortoaster

impl<E> From<E> for MyErrorType where E: Into<BaseLibErrorType> {
    fn from(value: E) -> Self {
        Self::BaseError(value.into())
    }
}
impl From<ErrorTypeFrom3rdPartyLib> for MyErrorType {
    // 🚫 ⚠️ 🛑 STOP 🛑 ⚠️ 🚫
    // BaseLibErrorType might implement From<AnyTypeThatCouldEverExistUntilTheEndOfTime>
    // This may result in conflicting traits in the future.
    // Shame on you for not considering this possibility
}

smh, guess I have to write a macro which calls a bunch of other macros, then.

ARitz-Cracker avatar Mar 21 '24 13:03 ARitz-Cracker