Surprising bivariance error in safe code
Nobody has claimed this yet.
- Dominant language
- Markdown
- Stars
- 6.6k
- Forks
- 1.7k
- Avg merge
- 16h 14m
- Merged PRs (30d)
- 1
Description
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:
- 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.) - 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
ThingDoerimplementation to also implement their ownThingDoerFactory, and move the real create logic to there, but that would result in it only being possible to createThingDoers 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.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Begin with the motivating Rust example and the discussion of bivariance, PhantomData, and object safety. Compare the two proposed changes—an invariant marker type or an allowable lint—and define a settled language-design direction; no file or test is named in the issue.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100