Generate `PhantomData<UnsafeCell<T>>` to make generic FFI structs lifetime invariant

[fitzgen]

Talked with @aturon about this yesterday. This change is needed to make our generated structs invariant, since we can't understand the real variance of the C/C++ things' lifetimes.

[fitzgen]

To avoid putting T: 'static trait bounds on everything, we should actually generate PhantomData<UnsafeCell<T>>, which is the easiest way to get an invariant type without lifetimes AFAIK.

[fitzgen]

@Kowasaki, perhaps this is something you'd be interested in?

[Kowasaki]

Hello @fitzgen , yes I will take a crack at this!

[fitzgen]

I would start by verifying that you can build + run tests. See CONTRIBUTING.md.

The relevant code for this bug is in src/codegen/mod.rs. Look for where we generate CompInfo's generic template parameters, if any exist. For each generic template parameter, we should also push a field of type ::$crate::marker::PhantomData<UnsafeCell<$ident>>.

https://github.com/servo/rust-bindgen/blob/master/src/codegen/mod.rs#L1314

Background for this issue is https://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science) and lifetime subtyping in generics.

Let me know if you run into any troubles or have any questions!

[Kowasaki]

I've confirmed that I can build and run tests, but I am confused about current_bitfield_layout mentioned in https://github.com/servo/rust-bindgen/blob/master/src/codegen/mod.rs#L1314 . I am not clear on the relationship between the bitfield and the the generic template parameters?

[fitzgen]

I should have linked to a specific commit, the line of code just got moved and that comment has nothing to do with this sorry.

Here is where that should have pointed: https://github.com/servo/rust-bindgen/blob/50ee737/src/codegen/mod.rs#L1394

[Kowasaki]

Ah I see! So it looks like I should get familiar with the AstBuilder--or at least with the generics?

[fitzgen]

You should be able to just take a look at surrounding code and how it is used. For example, just above that line is this: https://github.com/servo/rust-bindgen/blob/50ee737/src/codegen/mod.rs#L1384-L1387

You'll basically do the same thing, but the ty in this case will be a ::$prefix::marker::PhantomData<UnsafeCell<$name>>. Check out how the quote_ty! macro is used elsewhere in this file.

https://github.com/servo/rust-bindgen/blob/50ee737/src/codegen/mod.rs#L1437 is an example of getting (and later using) the crate prefix (std vs core).

[Kowasaki]

Oh that is helpful. Thanks!

[Kowasaki]

Is there a specific test case that I should be using or should I just be making sure that all generated structs be PhantomData<UnsafeCell<T>> instead of PhantomData<T> ?

[fitzgen]

Is there a specific test case that I should be using or should I just be making sure that all generated structs be PhantomData<UnsafeCell> instead of PhantomData ?

We used to generate PhantomData<T> if a type parameter wasn't used, but it turned out our is-a-template-parameter-used analysis wasn't smart enough, so I rewrote it as the stuff in src/ir/named.rs and stopped generating generic T parameters if they aren't used (see the comments at the top of that file if you're curious as to why).

So what you should be looking for is the new generation of PhantomData<UnsafeCell<T>> for any test case that has a template (and uses the template parameter). There is no longer any old PhantomData<T> members that are being replaced.

Check out the tests/headers/template-param-usage-2.hpp header for a test case that should end up with PhantomData<UnsafeCell<T>> after your changes.

Does that make sense? Anything else I can clarify?