rust-lang/rust

forbid conditional, negative impls

nikomatsakis opened this issue ยท 8 comments

We currently permit negative impls (feature-gated, #68318) for both auto- and regular traits. However, the semantics of negative impls are somewhat unresolved. The current trait checker's implementation in particular does not work well with "conditional" negative impls -- in other words, negative impls that do not apply to all instances of a type. Further, the semantics of such impls when combined with auto traits are not fully agreed upon.

Consider:

auto trait Foo { }
struct Bar<T> { }
impl<T: Copy> !Foo for Bar<T> { }

The question is, does Bar<Box<T>> implement Foo? There is some disagreement about what would be expected here.

As a temporary step, the plan is to forbid impls of this kind using similar logic to what we use for Drop impls. There was a PR in this direction #74648.

Another similar step would be to forbid negative impls for traits that have multiple generic parameters:

trait Foo<A> { }
impl<A, B> !Foo<A> for B { } // error

There is no particular reason that we can't support multiple parameters, but I suspect that the drop logic is not designed to handle cases like this.

Related issues:

Example of us enforcing a similar rule for drop check:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=fd4485ceb5a5145a1c77e336d778aecb

Note that the Drop impl is allowed with T: Debug, because that bound appears on the struct.

Relevant code in drop-check is here:

pub fn check_drop_impl(tcx: TyCtxt<'_>, drop_impl_did: DefId) -> Result<(), ErrorReported> {
let dtor_self_type = tcx.type_of(drop_impl_did);

@spastorino do you plan to look into this? (If so, I can assign it to you.)

As we said in our call, the obvious next step is to refactor the check_drop_impl code to be generic over the trait and not specific to Drop.

We will want to extend it, also, because I don't want you to be able to do things like

impl<A, B> !Send for (A, B) where A: Copy { }

In other words, we can kind of define what the "type parameters and predicates" are for other types like tuple types, &T types, etc, and ensure that the where-clauses/predicates used by the negative impl are limited to those. But the first step would just be to make it work for structs.

This is blocking stabilization of negative impls, although we could opt to just stabilize negative impls for non-auto-traits.

lcnr commented

i am currently looking into reworking dropck, cc #95309, but i also want to generalize the predicate part to get rid of SimpleEqRelation

while I am at it I could just write #74648 take 2

pitaj commented

Negative conditional impls enables the following pattern as well. Figured I should put this here

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=752114f2762321fa148318468b7b4352

Consider:

auto trait Foo { }
struct Bar<T> { }
impl<T: Copy> !Foo for Bar<T> { }

The question is, does Bar<Box<T>> implement Foo? There is some disagreement about what would be expected here.

What's the disagreement? According to the RFC:

Intuitively, to check whether a trait Foo that contains a default impl is implemented for some type T, we first check for explicit (positive) impls that apply to T. If any are found, then T implements Foo. Otherwise, we check for negative impls. If any are found, then T does not implement Foo. If neither positive nor negative impls were found, we proceed to check the component types of T (i.e., the types of a struct's fields) to determine whether all of them implement Foo. If so, then Foo is considered implemented by T.

  • "first check for explicit (positive) impls": none are found; however if Foo and Bar are public then a downstream crate could provide an impl for a local type Local
  • "Otherwise, we check for negative impls": Box<T> is not Copy so none are found, though again a downstream crate might provide one for Local
  • we proceed to check the component types: there are none, which presumably means that Foo is considered implemented by Bar<Box<T>>

Aside: does Bar<Local>: Foo? If Local: Copy we have an explicit !Foo impl; in this case any impl of Foo or !Foo for Bar<Local> in the downstream crate would be a conflict. (This is no different than positive impls.)


As I mentioned in #13231, auto trait as used for e.g. Send and Unpin is more complex than required for general opt-out marker traits, hence should probably not be used. (Would removal of the rules concerning component types and opt-in solve the apparent issue here?)