RFC: Ghost Busting

[Centril]

🖼️ Rendered

📝 Summary

1. Defines unused type parameters to act as if they are logically owned and covariant.

2. Introduces phantom T pseudo-fields which improves the ergonomics of changing
   variance and drop checking behavior when it is needed. A phantom T field also has
   the same behavior with respect to auto traits as PhantomData<T> does.

3. PhantomData<T> is redefined as struct PhantomData<T: ?Sized>; and deprecated.

4. The lang item phantom_data is removed.

5. Derive macros for derivable standard library traits will take advantage of statically known
   phantom types and fields to generate more permissive impls.

That's it folks; Happy 👻👤!

💖 Thanks

Thanks to @nikomatsakis and @eddyb for interesting discussions.

[Havvy]

not be able*

[Havvy]

This specific instance could (and should) be fixed using the non_exhaustive attribute. Do you have a better use case for privacy in phantom fields?

[Centril]

The documentation of #[non_exhaustive] says:

Structs marked as non_exhaustive will not be able to be created normally outside of the defining crate.

Essentially, this is the same as pub(crate).

Allowing for more fine grained visibility as pub(super) is more general and lets you control visibility on level not permitted by #[non_exhaustive]. I also believe that pub(crate) is a lot more clear than #[non_exhaustive] wrt. intent and what it means. I think that #[non_exhaustive] should be used for when you actually intend to add more fields or are unsure about that while visibility (pub, ..) should be used for controlling who can see what.

[notriddle]

It's not like you couldn't work around it by writing

struct Id<A, B> { phantom A, phantom B, _priv: () }

It really makes no sense for phantom fields to be subject to visibility checks. You can always add a unit field, and if that's not acceptable, then the problem can be fixed in a more general way.

[Centril]

The idea of using a _priv field is discussed in the alternatives section. You say that there is no reason for phantom fields to be subject to visibility, but a _priv field will need to be passed in struct literals, making things less ergonomic, so there are reasons to make phantom fields subject to visibility. It also increases uniformity of fields to do so.

[alercah]

It seems to me that phantom () is a more natural way to force a private field, than requiring it be named anyway.

[Havvy]

Also should be using the non_exhaustive attribute.

[Centril]

Hmm yes, in this case I'm inclined to agree... However on a second look, the example was just wrong and didn't add anything, so I just removed it =)

[scottmcm]

My first thought here is that the RFC is missing a section convincing me that unused type parameters and phantom data are such a common problem that adding special syntax is justified. For example, doesn't this force macros that consume struct definitions to understand the special phantom pseudo-keyword?

In particular, I'm not convinced that phantom: ::std::marker::PhantomData<T> is so substantially worse than phantom T would be. Certainly if (1) is a big issue, then one could argue for prelude inclusion instead -- PhantomData is a pretty unique name -- for far less impact than new syntax.

(3) acknowledge the field by adding an expression PhantomData when a value of the type is to be constructed.

This one's more interesting to me, but I think it's largely applicable to many ZSTs, not just PhantomData. Like if I make a pub struct Foo(()); to keep it from being externally constructed, I need to pass the (). There are a bunch of possible alternatives that would work for this, like a trait on types that are only ever set to their default, an attribute on the field to set to default, field initializers, allowing them to be unnamed (like #2102 since the type has no fields, and thus "disappears" other than its type), etc.

More permissive automatic deriving

Wouldn't your #2353 handle this just fine? And I think it needs a more realistic example than struct Label<T>;, since it seems that one would be handled just fine by type Label<T> = PhantomData<T>; And the Vec example certainly can't derive most of its traits.

[Centril]

For example, doesn't this force macros that consume struct definitions to understand the special phantom pseudo-keyword?

For all practical intents and purposes, yes.

In particular, I'm not convinced that phantom: ::std::marker::PhantomData<T> is so substantially worse than phantom T would be.

I believe that (3) makes it substantially worse. If we can provide a good and alternative way to solve (3) together with (1) (with prelude inclusion..), then I believe they are on par.

Certainly if (1) is a big issue, then one could argue for prelude inclusion instead -- PhantomData is a pretty unique name -- for far less impact than new syntax.

That's a good and cheap alternative, but it does not solve 2. and 3. (the latter is the most annoying part to me..). I've added this suggestion to the list of alternatives.

a bunch of possible alternatives that would work for this, like a trait on types that are only ever set to their default, an attribute on the field to set to default, field initializers, allowing them to be unnamed (like #2102 since the type has no fields, and thus "disappears" other than its type), etc.

Can you expand on these?

Wouldn't your #2353 handle this just fine?

It would indeed, but it would be less automatic since you'd have to acknowledge the phantomness doubly on the type parameter(s) (or the type) and with a PhantomData<P> field. In cases where you don't need phantom T to change variance, the advantage of struct Label<T>; is 0 steps vs. 2 steps with PhantomData<T> + #[derive_no_bound].

What I like about unused parameters => phantom P is that they together form a neat consistent package.

[matthewjasper]

And auto trait implementations?

[Centril]

Elaborate?

[matthewjasper]

PhantomData<T> only implements auto traits that T does. This is used in fmt to opt out of all auto traits.

[Centril]

Today I learned... Interesting. I'll write something about it when I'm more lucid =P

[Centril]

I've updated the RFC accordingly now.

[scottmcm]

but it does not solve 2

I think that "add a pseudo-field using phantom" and "add a read field using PhantomData" are essentially the same for (2). But of course I like the "you have to mention all your parameters somehow to say what you wanted" behaviour, so I'm sure we have different perspectives here.

Can you expand on these?

Sure, though be warned this is an unfiltered splat of things -- I haven't thought about the details for most, and some of them might just be just terrible.

• trait ImplicitDefault : Default {} and impl ImplicitDefault for (), then struct initializers add x: ImplicitDefault::default() for any field x that wasn't specified. (Could also change elseless-if desugar to else { ImplicitDefault::default() }, but that's a different conversation.)
• struct Foo(#[always_default] ());, allowed only on a suffix of the fields for tuple-structs, prohibits providing a value in initializers and always uses Default::default()
• struct Foo(() = ());, along the lines of Default Struct Field Values #1806
• struct Foo { _: () }, the struct still contains the type, but initializing and reading "inlines" the fields -- all zero of them -- so there's nothing to need to specify in the initializer.

[leoyvens]

Is writing phantom T here optional?

[Centril]

No, phantom T is required to let dropck know that T is logically owned, which is not the case if it only sees data: *const T. I will clarify.

[leoyvens]

Here it seems phantom T is indeed optional.

[Centril]

Yep, that's right.

[Centril]

@scottmcm Ah yes, (2) only goes away morally when you don't have to acknowledge unused type parameters.

But of course I like the "you have to mention all your parameters somehow to say what you wanted" behaviour, so I'm sure we have different perspectives here.

I guess the central question wrt. eliding unused parameters is if we believe that we help or hurt users substantially by not/throwing a "you have to use the parameter and think about variance always" error in their face.

When not dealing with unsafe code, the importance of invariance/not shouldn't even arise and owned covariance is the right thing; I think this is very common due to "unconstrained type parameters" when trying to impl a trait, so you add a type param and now you have to add PhantomData also. This seems quite the unergonomic experience when zero unsafe code is involved. Personally I think we should avoid penalizing safe code for the worries of unsafe code.

I'd also like to note that even if you get "unused type parameter" thrown in your face, some people might be inclined just to casually add PhantomData<P> (assuming P is the type parameter) without giving it much thought. Furthermore, variance is non-trivial and incredibly easy to get wrong even if you think about it, a case in point is:

• pub struct Id<S, T>(::std::marker::PhantomData<(*const S, *const T)>); (covariance => we can do transmute_lifetime in safe Rust, https://docs.rs/refl/0.1.2/refl/struct.Id.html#a-note-on-variance-and-safety). Defining Id<S, T> this way was actually done in the wild.

trait ImplicitDefault : Default {}

I think that's essentially /~https://github.com/Centril/rfcs/blob/rfc/ghost-busting/text/0000-ghost-busting.md#alternative-allow-filling-unspecified-fields-with-default
I also got the idea from #1806.

struct Foo(#[always_default] ());

Seems like a variation on ImplicitDefault. Interesting, but seems quite niche tho.

struct Foo { _: () }

I think this was @eddyb's idea? This would be: struct Foo<T> { _: PhantomData<T> } and work for any ZST?

[leoyvens]

I see two possible intents of a user that gets the unused type parameter error:

1. They forgot to use it (forgot to add a field) or removed the use (removed the field) and forgot to remove the parameter.
2. They meant to use PhantomData.

The status-quo optimizes the experience of writing definitions for the first case, this RFC optimizes it for the second case. Which is better depends on what we believe the common case is. I believe that 1 is the common case and the status-quo is better, this RFC could retain the benefits of the status-quo by making the phantom T annotation mandatory.

[Centril]

@leodasvacas I think that's a good analysis. I'd like to add that I think that 1. is more common when "in-transition" (refactoring) as opposed to just starting out with a new type. That a field is missing will be very noticeable when you try to use that field but notice that it isn't there.

I'm certainly open to only keeping phantom T if we believe that is the right balance.

[petrochenkov]

+1 to defaults for unused type parameters (while keeping unused_type_parameters as a usual unused lint that can be turned off during prototyping) (this also should be enough to get rid of phantom_data lang item).

-1 to the new special syntax and phantom fields sometimes behaving as normal fields and sometimes not.

[Centril]

@petrochenkov Might perhaps be better to make #[allow(unused_type_parameters)] the default and then #[deny(unused_type_parameters)] after?

-1 to the new special syntax and phantom fields sometimes behaving as normal fields and sometimes not.

Heh... and I was hoping to not get caught between two camps wanting to change things in opposite directions =P

What in particular are you referring to wrt. "sometimes [..] sometimes not"?

[petrochenkov]

@Centril

What in particular are you referring to wrt. "sometimes [..] sometimes not"?

Privacy treatment, for example (phantom is a field) and construction/pattern matching (phantom is not a field).

[Centril]

@petrochenkov

Privacy treatment, for example (phantom is a field) and construction/pattern matching (phantom is not a field).

Ah; Why tho is this a problem (and what category of problem, teachability, consistency, compiler complexity, ..)?

Personally, I think this discrepancy falls naturally out of the way PhantomData<T> is used today - you never care about / want to pattern match on a PhantomData, and adding it in construction is a pain, but privacy is a matter of correctness so you have to care?

[nox]

I agree with @scottmcm that I don't feel like specific syntax is required to replace PhantomData, I don't believe removing PhantomData is a thing worth pursuing.

If the community decides it is worth pursuing though, I don't think the way described here is a bad way to do it.

[est31]

I don't like the general idea of this PR to add a custom keyword to the language for a niche feature... Rust actually came from that place where it had tons of language features. They then gradually moved to the standard library. Now we see built in stuff re-appearing like the ? operator or now this PR....

Adding PhantomData to the prelude should be enough. I side with @scottmcm and @nox here.

[Centril]

I don't like the general idea of this PR to add a custom keyword to the language for a niche feature...

I don't agree that phantoms are niche; I find that they show up plenty when using a little more advanced type features.

Now we see built in stuff re-appearing like the ? operator or now this PR....

And ? was added for good reason, it improved ergonomics over try!(..) a lot. And PhantomData has both ergonomical and technical problems (deriving). Improving phantoms by introducing phantom T was also suggested in 2014, before 1.0. For me, the way phantoms dealt with are really about the behavior of the language, and relegating it to libraries worsens ergonomics.

Adding PhantomData to the prelude should be enough.

Certainly open to that if we decide that is the better route. I'd like for us to also find a solution that solves the greatest ergonomics problems wrt. phantoms: the need to provide marker: PhantomData, in literals...

[nical]

I am very very sympathetic with removing the need for declaring a PhantomData<..> member. I use phantom type parameters a lot for various things like math types or Ids and the mandatory member makes it impossible to have a simple and clean public interface.

Take euclid for instance: I would love for people to be able to write v = Vector2D { x: 1.0, y: 2.0 }; but has to be either v = Vector2D { x: 1.0, y: 2.0, _marker: PhantomData }; everywhere or force a constructor like Vector2D::new(1.0, 2.0); on everyone.

My experience so far has been that for every use of phantom type parameters I have had to trade a safer API with degraded ergonomics, and I would use them a lot more if it wasn't this way.

I understand that the cost of adding a keyword is big and maybe too big to fix this. however, some of the things this RFC wants to solve could maybe be achieved in other ways. Maybe with something like attributes?

#[phantom(T)]
Vetcor2D<T> {
  x: f32,
  y: f32,
}
type ScreenVector = Vector2D<ScreenSpace>;
type WorldVector = Vector2D<WorldSpace>;

let a = WorldVector { x: f32, y: f32 };
let WorldVector { x, y } = some_function();
// etc. Notice how nice it is to not deal with the extra member here.

This kind of thing is painless to do in C++.

I believe that removing the need to add members for unused type parameters is very much worth pursuing and could be done in a backward-compatible way without the cost of adding syntax, I am mentioning attributes but there probably many other ways.

[nox]

@nical AFAIK you can't build structs from a type alias, so even if PhantomData was unneeded you still wouldn't be able to do WorldVector { x, y }. That is a separate issue and doesn't change you point though.

Did you ever consider doing things like that?

struct Unit<U>(PhantomData<U>);

enum World {}
const WORLD: Unit<World> = Unit(PhantomData);

Vector2D { x, y, u: WORLD }

I think this requires as many imports as what you described with #[phantom], for almost no additional syntax boilerplate.

[Centril]

@nical

I understand that the cost of adding a keyword is big and maybe too big to fix this.

Actually, I think the changes to the grammar are larger if you want to go the attribute route, since then, you have to allow arbitrary types inside attributes as in #[phantom(<type>)] (AFAIK this is not permitted yet).

If you take a look at the grammar changes here: /~https://github.com/Centril/rfcs/blob/rfc/ghost-busting/text/0000-ghost-busting.md#grammar, you see that no new keywords are introduced (phantom T is contextual) and that the delta is quite small.

I'd argue that the mental cost is also quite small or even reduced compared to PhantomData<T> since there now are (substantially, I think) fewer places where explicit phantoms turn up.

[Centril]

@quininer, @fstirlitz, @tanriol, can you elaborate on your concerns?

@kennytm, what is confusing you? can I elaborate on something?

[kornelski]

This RFC is mostly about syntax sugar.

I think Rust should have more room to experiment with various syntaxes by supporting "transpilers" like JavaScript's Babel. This way users could try out, in real-world code, alongside other syntax changes, whether it makes sense as a phantom field, where clause, magic parameter or something else.

https://internals.rust-lang.org/t/pre-rfc-first-class-support-for-compile-to-rust-languages/7610

[vorner]

I don't have a strong opinion about the introduction of new syntax for phantom T. However, I'm concerned about disabling the warning for unused type parameters. In the history of receiving that warning on my own code, for me it has almost always meant "you forgot to make the code match the type parameters" or "you deleted some code and need to adjust the type parameters" or "you wrote the type parameters you thought you needed and forgot to update them when it turned out you didn't", and almost never meant "you need a PhantomData".

Recently, I've written some code that uses phantom data more than the usual amount, and it actually was the „You need PhantomData“ case. And considering the experience, I'd say I'm against defaulting to PhantomData<T> for the unused T. While the former problem (I forgot to remove T) would manifest early on as „Type annotations needed“, defaulting in my case would probably lead to very confusing problems much further away.

In other words, at least half of the time I didn't end up using PhantomData<T>, but something like PhantomData<fn() -> T> or PhantomData<fn(T)>, because the structure didn't own any T (even conceptually). It either produced it or consumed it. If there was the default, I wouldn't be reminded by the compiler to think about how it should act and if someone tried to use the structure with T: !Send, the whole structure would end up !Send ‒ but that's not the case if the structure can generate a new T in whatever thread it currently lives in. I believe this goes to similar category of not allowing any fields to be missing from initialization of a structure (because the default might be wrong), but on the type level.

[alercah]

Based on my experience I think I'm inclined to agree.

[joshtriplett]

This has been open for a long time,and I feel like it doesn't have sufficient justification. There's a solution for this problem already, it's documented and understood, and I don't think the proposed syntax is a sufficient improvement to justify expanding the surface area of the language.

Thus:

@rfcbot close

[rfcbot]

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged team members:

• @Centril
• @aturon
• @cramertj
• @eddyb
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[scottmcm]

@pnkfelix I've seen talk of similar things before (I've called something similar ImplicitDefault, and I think it came up in the Ok(()) discussions), but I don't think it's ever gotten to a full RFC proposal.

[kornelski]

A new trait would work only for marker types, but a default value for fields could work for marker types and other more general cases. There's a postponed RFC for it: #1806 Given that constexpr has become usable, maybe it'd be a good time to reopen it?

Default fields don't improve destructuring.

[withoutboats]

I'd be in favor of a much smaller RFC that just made the variance implied by _marker: PhantomData<T> the default variance if the type parameter isn't used in the struct. I believe a few things:

• The vast majority of interactions with the PhantomData type come from people doing compiler-driven-development who have mocked out their struct, and just want to get the code to compile but need a parameter in the interface. These people gain nothing at all from requirements around having PhantomData, since eventually the parameter will appear in the definition properly.
• Requiring PhantomData is not seriously causing people to think about variance. There is an easy solution that doesn't require deep thought and is nearly always correct: _marker: PhantomData<T>. I think this is how most users solve the bug; the remainder already understand variance and hopefully don't need a reminder. I don't think this "explicitness" is buying anything; its overly conservative and just naggy.

However the phantom T type parameter change is, IMO, not a worthwhile addition to the language when those use cases can just use the PhantomData type.

[withoutboats]

Actually on second thought rather than getting rid of the error entirely, I'm in favor of downgrading it to a warning, using the default variance rules if the type is omitted. That way you still get nagged if, once you've finished filling out your code, you still haven't used that type parameter (encouraging you to use PhantomData then if its really necessary), but mid-mock you just get another dead code warning that you can ignore, as you are probably already doing.

Anyway that's a different RFC from this one.

[glaebhoerl]

(@withoutboats I like the idea of a warning, which seems like a good resolution; just w.r.t. previous comment, note @vorner's comment above, according to which PhantomData<T> is not nearly always correct.)

[withoutboats]

@glaebhoerl I read @vorner's comment, I think @vorner's experience is squarely in the very small space excluded by "nearly always." But I think reducing unused type parameters to a warning would suffice for everyone.

[joshtriplett]

On Sun, Jan 20, 2019 at 03:04:26PM -0800, boats wrote: Actually on second thought rather than getting rid of the error entirely, I'm in favor of downgrading it to a *warning*, using the default variance rules if the type is omitted. That way you still get nagged if, once you've finished filling out your code, you still haven't used that type parameter (encouraging you to use PhantomData *then* if its really necessary), but mid-mock you just get another dead code warning that you can ignore, as you are probably already doing.

I'm not a fan of the idea of turning this error into a warning or dropping it; I'd like it to remain an error.

Anyway that's a different RFC from this one.

True.

[mark-i-m]

What about a deny by default lint?

[alercah]

I see basically no value in that.

…

On Tue., Jan. 22, 2019, 00:19 Who? Me?! ***@***.*** wrote: What about a deny by default lint? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2357 (comment)>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/AT4HVaS2wYZceD_UTDYHVwKfpTiAE9oOks5vFfa4gaJpZM4SbPbp> .

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[nikomatsakis]

I feel like it'd be useful to enumerate some of the scenarios where phantom data is annoying.

One use case for me is kind of "typed indices" (I think the pattern is more general, but I can't think of a good summary term, so I'll leave it at that). I have been annoying a number of times because I have structs that are like

struct Index<T> { .. }

where the T is not stored within the index, but more something that we want to "remember", because some other bit of code will do something like:

fn get<T>(x: impl Index<T>) -> T

In these cases, I basically always want invariant, and I... probably don't really want to consider T owned by the index. The auto-trait behavior is kind of neither here nor there. Typically, the index is sort of 'inert' without the container. But it'd be interesting to collect more such examples and try to see if that is true for all of them.

(Honestly, I feel like what I want here is sort of an "associated type" for a struct, e.g., struct Index { type T; ... }, where I can write Index<T = ...>. Except I don't want necessarily want to write that, and it sort of opens up a can of worms.)

I also agree with @withoutboats that refactoring is annoying.

[nical]

I feel like it'd be useful to enumerate some of the scenarios where phantom data is annoying.

Another scenario where phantom data is annoying (somewhat similar to @nikomatsakis's Index<T> example) is euclid's strongly typed 2D/3D linear algebra crate where types can be tagged with a dummy phantom type prevent mistakes. for example one will typically create aliases such as:

type WorldPoint2D = TypedPointe3D<f32, WorldSpace>;
type ScreenPoint2D = TypedPoint3D<f32, ScreenSpace>;
type Projection = TypedTransform3D<f32, WorldSpace, ScreenSpace>;

TypedPoint3D source code: /~https://github.com/servo/euclid/blob/1244c867a8a7bcd4333c6555ea1f7305abf997d9/src/point.rs#L417

It'd be great if euclid could let you write:

// No can do, gotta use WorldVector3D::new(1.0, 2.0, 3.0)
let v: WorldVector3D = Vector3D { x: 1.0, y: 2.0, z: 3.0 };

However at the moment the language enforces the _unit phantom data member to be explicitly specified.
Also as the author of this code, I've had to learn about fancy words like "variance" to rub the compiler the right way and do something that is simple in C++ (at least in comparison).

My other main exposure to phantom types is the sid crate which corresponds to @nikomatsakis's Index<T> example I think.

In both cases the phantom type is only there as a some sort of tag and properties of the actual type T don't really matter.

[burdges]

A priori, there is no subtyping relationship among unit types, so you would not expect the variance to matter. In fact, any normal data would be covariant because someone might define traits Metric and Kilo or whatever.

As an aside, I just learned that no variance annotation in a C# interface denotes invariance, which does not exist ion Rust, but still might confuse someone.

Anyway there is nothing wrong with defining some #[covariant_default] attribute.

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

By the power vested in me by Rust, I hereby close this RFC.