Surprising bivariance error in safe code

[comex]

Unit structs are desirable for various reasons; one of them is to allow dynamically choosing, via trait objects, between implementations of functions that don't inherently require a self argument. In this simplified motivating example, I have a trait which has a static method to create an instance of the implementing type:

trait ThingDoer {
    fn create() -> Self;
    fn do_something(&self);
}

Suppose I want to allow callers to choose at runtime which implementation to create an instance of, receiving a trait object. I can do this by creating a generic wrapper unit struct which represents a single implementation, and can be converted to a trait object for a "factory" trait that wraps the create() static method:

trait ThingDoerFactory {
    fn create(&self) -> Box<ThingDoer>;
}
struct ThingDoerFactoryImpl<T: ThingDoer>;
impl<T: ThingDoer> ThingDoerFactory for ThingDoerFactoryImpl<T> {
    fn create(&self) -> Box<ThingDoer> { Box::new(T::create()) }
}

As written, this doesn't compile because ThingDoer is not object safe (bleh). That's easily fixed by splitting the static method(s) out into a separate trait, but there's another problem:

test.rs:10:29: 10:30 error: parameter `T` is never used [E0392]
test.rs:10 struct ThingDoerFactoryImpl<T: ThingDoer>;
                                       ^
test.rs:10:29: 10:30 help: consider removing `T` or using a marker such as `core::marker::PhantomData`

The suggestion to use PhantomData is initially confusing, because ThingDoerFactoryImpl instances do not "contain" any instances of T, not even logically / indirectly via unsafe code. After reading up on it, I guess the issue is that as written, T is bivariant because the struct does not have any fields putting variance constraints on it, and bivariance isn't usually particularly useful. In this case, since there is no point in converting instances of ThingDoerFactoryImpl<A> to ThingDoerFactoryImpl<B>, the issue can be resolved by adding a PhantomData<*const T> field to indicate invariance.

However, PhantomData is, frankly, rather arcane feeling, especially with the raw pointer type inside it; while it's necessary as a way for unsafe code to inform the compiler about some things it's doing behind its back, and the way it works does make sense once you understand the problem, there is no unsafe code here, so I think requiring this type of thing should be avoided except as a last resort. In this case, while, as I said, converting instances is not useful, it's also not harmful, so nothing bad would happen if the compiler simply accepted the struct as bivariant - but I realize that there's no really good way to differentiate between what I'm doing and the unsafe cases the error was designed for, so I guess silently accepting it would be a bad idea.

(EDIT: Another problem is that PhantomData causes the factory to lose Sync, Copy, etc., which it shouldn't.)

I can think of two possible solutions:

1. Add a simpler standard library marker type that just marks a parameter as invariant, without having the more complicated interface of PhantomData. (I can't think of a good reason to want phantom covariant and contravariant parameters in safe code, so invariant should be enough.)
2. Turn the bivariance error into a deny-by-default lint, so an #[allow()] declaration could be used if a type is really meant to be bivariant .

Notes:

• Incidentally, it would be nice if it were somehow possible to have a trait object corresponding to a type rather than an instance of one: just like a single implementation of a trait containing instance methods supports use via both monomorphization and dynamic dispatch "for free", without additional boilerplate, something similar should apply to traits containing static methods.
• In some cases a simpler approach would be to remove the static methods altogether, expect every ThingDoer implementation to also implement their own ThingDoerFactory, and move the real create logic to there, but that would result in it only being possible to create ThingDoers as trait objects. I want to keep the static interface for callers that know at compile time which implementation they want, in addition to the trait object interface.

[bluss]

Isn't the logical PhantomData to use PhantomData<fn() -> T> ?

I thought the idiomatic way to indicate invariance was PhantomData<Cell<T>>, but maybe that has unintended side effects too (Send / Sync?).

As I understood it, variance is a far too complicated thing to burden programmers with, so PhantomData can be used to describe it in simpler terms ("this produces T, this consumes T")..

[pnkfelix]

I think @bluss has it exactly right: You want to indicate the intent of the factory's relationship with T. Since it is intended to produce instances of T, I think a PhantomData of a function type like @bluss wrote makes sense.

(Having said that, it is possible we could revise the rules so that in the absence of unsafe pointers in such a struct, we ignore the bivariance of unused parameters. However, PhantomData has use for purposes other than variance determinations, such as the drop-check rule, which to me indicates that we may find other uses for it in the future, and thus we are all probably better off if we continue requiring the use of a PhantomData in cases like these, with the explanation that one probably needs to pick one of the three usual boilerplate cases for each type param.)