__UnpinStruct is restricting the pinned field.

[ogoffart]

Unless I misunderstand something, it seems the logic to put the field in __UnpinStruct is reversed, as it checks that the #[pin]'ed field are Unpin.

For example:

// This structure cannot be moved
struct Property {
    pin: std::marker::PhantomPinned
}

#[pin_project::pin_project]
struct Container {
    #[pin]
    xxx : Property
}

Result in this error:

error[E0277]: the trait bound `std::marker::PhantomPinned: std::marker::Unpin` is not satisfied in `__unpin_scope_Container::__UnpinStructContainer`
   |
20 | #[pin_project::pin_project]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ within `__unpin_scope_Container::__UnpinStructContainer`, the trait `std::marker::Unpin` is not implemented for `std::marker::PhantomPinned`
   |

__UnpinStruct constraints all the pin'ed types. But I believe it should be the opposite, only the types that are not pinned should satisfy Unpin.

[Aaron1011]

This logic is current. __UnpinStruct: Unpon is used as a bound for the pin-projected strict. That is, you can only treat pin projections as Unpin when all of the pinned types are also Unpin (I.e. when IRS safe to ignore the fact that they are pinned).

[taiki-e]

This error is because the Unpin implementation provided by pin_project adds a Unpin bound to a field that is known not to implement Unpin (It doesn't matter if it is explicit). This can be resolved if RFC 2056 (rust-lang/rust#48214) stabilizes.

The workaround I know is using pin_project(UnsafeUnpin) and not providing a manual UnsafeUnpin implementation. EDIT: this is wrong.

pin-project/src/lib.rs

Lines 93 to 95 in 1b0e552

	/// Note that if you specify `#[pin_project(UnsafeUnpin)]`, but do *not*
	/// provide an impl of `UnsafeUnpin`, your type will never implement [`Unpin`].
	/// This is effectly the same thing as adding a [`PhantomPinned`] to your type

However, perhaps we can provide an option to support this. (Add the PhantomPinned argument to pin_project or parse the field to automatically adjust the Unpin implementation. Personally, I prefer to pass arguments explicitly.) EDIT: Perhaps automatic detection will not actually work.

(FYI: I found this problem about two weeks ago and added two tests.)

• pin-project/tests/ui/pin_project/phantom-pinned.rs
  

  pin-project/tests/ui/pin_project/phantom-pinned.stderr

  Lines 1 to 16 in 1b0e552

  	error[E0277]: the trait bound `std::marker::PhantomPinned: std::marker::Unpin` is not satisfied in `UnpinStructFoo`
  	--> $DIR/phantom-pinned.rs:10:1
  	|
  	10 | #[pin_project]
  	| ^^^^^^^^^^^^^^ within `UnpinStructFoo`, the trait `std::marker::Unpin` is not implemented for `std::marker::PhantomPinned`
  	|
  	= help: the following implementations were found:
  	<std::marker::PhantomPinned as std::marker::Unpin>
  	= note: required because it appears within the type `Inner`
  	= note: required because it appears within the type `UnpinStructFoo`
  	= help: see issue #48214
  	= help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
  	error: aborting due to previous error
  	For more information about this error, try `rustc --explain E0277`.

• pin-project/tests/ui/pin_project/trivial_bounds.rs

[ogoffart]

Actually, the full error message tells that the problem is because of rust-lang/rust#48214

This reduce the utility of this macro if it can only be done for generic types.

Maybe the pin_project macro should add a <T = ()>

[taiki-e]

@ogoffart

Maybe the pin_project macro should add a <T = ()>

I'm not sure about how does this resolve the problem - Could you explain in a little more detail?

[ogoffart]

Because having a generic in the structure work around the issue 48214.
But i actually think pin_project should not add that, because it would also then add a field which we don't want.

I don't understand how to use pin_project(UnsafeUnpin) to work this around. It just change the error to

error[E0277]: the trait bound `Container: pin_project::UnsafeUnpin` is not satisfied

[taiki-e]

I don't understand how to use pin_project(UnsafeUnpin) to work this around. It just change the error to

Oops, sorry, my comment is wrong. It doesn't work.

[taiki-e]

Hmm... I'm not sure how to support this without unstable features (I think it needs trivial_bounds or optin_builtin_traits).

cc @cramertj @RalfJung @Nemo157 Do you know a workaround for this?

[ogoffart]

@taiki-e your MustNotImplUnpin does not seem to work. I get the compilation error even if i comment the impl Unpin

[taiki-e]

Oh... you are right.

[taiki-e]

This worked well in 0.3, so I think it should actually be considered a regression.

[taiki-e]

Perhaps a realistic solution would be to add a new #[unsafe_project] that removes the guarantee about Unpin from #[pin_project]. The document probably has the following wording: "Safety: This macro is safe unless you manually add an Unpin implementation." and "This macro is basically deprecated. Should not be used except in use cases like #102."

@Aaron1011 Any thoughts?

[ogoffart]

How about #[pin_project(unsafe)] so the unsafe keyword appears literally in the source.

But yes, the documentation should maybe recommend the trick to add a (useless) generic parameter so it compiles.

#[pin_project::pin_project]
struct Container<T = ()> {
    #[pin]
    xxx : Property,
    phantom: PhantomData<T>,
}

It also work with a generic lifetime

#[pin_project::pin_project]
struct Container<'a> {
    #[pin]
    xxx : Property,
    phantom: PhantomData<&'a u32>,
}

And the compilation error can be hinting about this. The macro can detect that there is no generic argument and have its own error, or rename the field __UnpinStructContainer to __UnpinStructContainer_CheckIssue_xxx or

[taiki-e]

How about #[pin_project(unsafe)] so the unsafe keyword appears literally in the source.

Can #[warn(unsafe_core)] or cargo geiger detect this?

But yes, the documentation should maybe recommend the trick to add a (useless) generic parameter so it compiles.

I don't think the problem is related to generics. EDIT: this is wrong
Unpin implementation provided by pin-project adds Unpin bounds for all fields (it does not matter whether fields are generics or not) annotated with #[pin] .
The problem is that if this includes a type that compiler know doesn't implement Unpin, you get an error.

[taiki-e]

For clarity:
The major goal of 0.4 is to prevent you from writing unsound code unless you use unsafe code.
The problem is that the method we use to guarantee this introduces a limitation.
(Note: It is not safe to implement Unpin when using pin projection).
Currently, the stable version of rustc cannot use the negative trait implementation, so if you want to ban to implement Unpin without unsafe, pin-project needs to implement Unpin in some way.
This will not work if Unpin cannot be implemented like this case. But, if pin-project does not implement Unpin, the user can write an unsound Unpin implementation without unsafe.
(If Unpin is an unsafe trait, these are not a problem...)

[ogoffart]

Can #[warn(unsafe_core)] or cargo geiger detect this?

Depends what code the macro generates. But if you use the Span from the unsafe keyword, it will be reported as a waring/error there.

I don't think the problem is related to generics.

It is a problem related to not having generic. (rust issue 48214)

If the struct have generics, everything works fine.

[taiki-e]

It is a problem related to not having generic. (rust issue 48214)

If the struct have generics, everything works fine.

Oh, you are right, I confirmed locally. I didn't understand about this feature properly... sorry.

Depends what code the macro generates. But if you use the Span from the unsafe keyword, it will be reported as a waring/error there.

This seems to work well, thanks. (If this is actually adopted, it seems necessary to propagate allow(unsafe) correctly)

[taiki-e]

(In fact, many of my comments on this issue seem wrong... sorry for that.)

[taiki-e]

@ogoffart

Perhaps the minimal fix to this issue is to mention errors and workarounds in the documentation? (#102 (comment))

And do we also need to provide a way to completely disable the pin-project unpin guarantee to improve ergonomics? (#102 (comment))

[taiki-e]

The workaround I know is using pin_project(UnsafeUnpin) and not providing a manual UnsafeUnpin implementation.

I don't understand how to use pin_project(UnsafeUnpin) to work this around. It just change the error to

I think I found a way to fix this: playground

[taiki-e]

Filed #107

[ogoffart]

Nice!

I also found another way to error out if Unpin is implemented
I'll let you decide which one you prefer.

    #![allow(dead_code)]
    use core::marker::{PhantomPinned};

    mod pin_project {
    pub trait ThouShalltNotImlementUnpin {
        fn test(&self) {}
    }
    impl<T : Unpin> ThouShalltNotImlementUnpin for T {}
    
    pub trait Xx {
        fn test(&self) {}
    }
    
    impl<T> Xx for T {}
    }


    struct Foo {
        // #[pin]
        x: PhantomPinned,
    }
  
    fn test(f : &Foo) {
        #[allow(unused_imports)]
        use pin_project::ThouShalltNotImlementUnpin;
        use pin_project::Xx;
        f.test();
    }
    
    //impl Unpin for Foo {}    // this would error

[jonhoo]

I wonder if https://docs.rs/static_assertions/0.3/static_assertions/macro.assert_not_impl_any.html might be something to look into.

[taiki-e]

@ogoffart
Thanks, #107 is a fix for a known workaround that doesn't work properly, and #102 (comment) is a technique needed to improve the API that I mentioned in #102 (comment), so I think they don't overlap :)
However, that way seems can be broken the guarantee by adding a method test to Foo.

@jonhoo
Thanks. But that way doesn't seem to work well with generics.

[taiki-e]

Unfortunately, it seems difficult to guarantee negative implementations at this time (but thanks for the info @ogoffart and @jonhoo!)

[taiki-e]

I've filed #108 to investigate how to guarantee negative implementation.

[ogoffart]

However, that way seems can be broken the guarantee by adding a method test to Foo.

Good catch!
We can fix that by having the test function return a sealed type
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=d1a2fea5630d880cd04a7cb1f1f1f797

[taiki-e]

We can fix that by having the test function return a sealed type

Cool! But it still doesn't seem to work well with generics and conditional implementations.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=57d4003292043b28b52f07709ed0c893

[taiki-e]

By the way, it seems #[pin_project] (not UnsafeUnpin) can support this in the same way I used in #107 :)

If we can support this without UnsafeUnpin, I don't think we need negative implementation support.

[taiki-e]

Published 0.4.1 which fixes this issue completely. #[pin_project] can now support this without UnsafeUnpin.

[RalfJung]

Reading this thread here I am mostly left confused. Given all the corrections I do not seem to be the only one. ;)

Is there a write-up somewhere of the solution you ended up with, how it works and what it relies on from rustc?

[taiki-e]

@RalfJung The actual fix is different from what was mainly discussed in this thread because I misunderstood a lot about this error (RFC 2056, rust-lang/rust#48214). Sorry for the confusion.

A short explanation is in #107, but...

This error occurs when the where clause has an Unpin bound that satisfies the following requirements:

• The type has no generics and no lifetime
• The type does not implement Unpin

For example, this code gives an error (Wrapper<PhantomPinned> satisfies the above two requirements):

pin-project/tests/ui/unstable-features/trivial_bounds-feature-gate.rs

Lines 10 to 16 in 4803d04

	struct Wrapper<T>(T);
	impl<T> Unpin for Wrapper<T> where T: Unpin {}
	struct Bar(PhantomPinned);
	impl Unpin for Bar where Wrapper<PhantomPinned>: Unpin {} //~ ERROR E0277

pin-project/tests/ui/unstable-features/trivial_bounds-feature-gate.stderr

Lines 12 to 22 in 4803d04

	error[E0277]: the trait bound `std::marker::PhantomPinned: std::marker::Unpin` is not satisfied
	--> $DIR/trivial_bounds-feature-gate.rs:16:5
	|
	16 | impl Unpin for Bar where Wrapper<PhantomPinned>: Unpin {} //~ ERROR E0277
	| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `std::marker::PhantomPinned`
	|
	= help: the following implementations were found:
	<std::marker::PhantomPinned as std::marker::Unpin>
	= note: required because of the requirements on the impl of `std::marker::Unpin` for `phantom_pinned::Wrapper<std::marker::PhantomPinned>`
	= help: see issue #48214
	= help: add `#![feature(trivial_bounds)]` to the crate attributes to enable

playground

This can be avoided by adding a lifetime to the Wrapper in the previous example.

pin-project/tests/ui/unstable-features/trivial_bounds-feature-gate.rs

Lines 18 to 24 in 4803d04

	struct WrapperWithLifetime<'a, T>(PhantomData<&'a ()>, T);
	impl<T> Unpin for WrapperWithLifetime<'_, T> where T: Unpin {}
	struct Baz(PhantomPinned);
	impl<'a> Unpin for Baz where WrapperWithLifetime<'a, PhantomPinned>: Unpin {}

playground

As Wrapper implements Unpin regardless of a lifetime and the lifetime used in the wrapper does not overlap with the lifetime used in Baz, it should not affect the Unpin requirement. (tested in tests/ui/pin_project/proper_unpin.rs and tests/ui/unsafe_unpin/proper_unpin.rs.)

#107 and #111 used this way to avoid the restriction. i.e., they added a lifetime to the wrapper (and added some related tests). (Note: Since #53, pin-project implements Unpin via wrappers in all cases, so there hasn't been a big change in the way to generat Unpin implementation.)

[RalfJung]

Okay, so it's basically a weird hack to hack around a weird rule in the trait system. Thanks.

The number of hacks that keep piling up in this crate is quite impressive... and worrying.