Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move automatic generation of Unpin implementation to proc-macro-derive #77

Merged
merged 6 commits into from
Sep 7, 2019

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Sep 7, 2019

To generate the correct Unpin implementation, we need to collect the
types of the pinned fields. However, since proc-macro-attribute is
applied before cfg, we cannot be collecting field types at this timing.
So instead of generating the Unpin implementation here, we need to
delegate automatic generation of the Unpin implementation to
proc-macro-derive.

Fixes #68

@taiki-e taiki-e added this to the v0.4 milestone Sep 7, 2019
@@ -0,0 +1,5 @@
#!/bin/bash

# A script to run compile tests with the same condition that pin-project executes with CI.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc #35

@taiki-e taiki-e requested a review from Aaron1011 September 7, 2019 02:59
proj_fields.push(quote!(#ident: ::core::pin::Pin<&#lifetime mut #ty>));
proj_body.push(quote!(#ident: ::core::pin::Pin::new_unchecked(#ident)));
proj_fields.push(quote! {
#(#cfg)* #ident: ::core::pin::Pin<&#lifetime mut #ty>
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(In the future, it may be preferable to move these processes to proc-macro-derive.)

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 7, 2019

Hmm... tuple structs don't seem to work well either.

Edit:
https://doc.rust-lang.org/reference/attributes.html

Enum variants and struct and union fields accept outer attributes.

To generate the correct `Unpin` implementation, we need to collect the
types of the pinned fields. However, since proc-macro-attribute is
applied before cfg, we cannot be collecting field types at this timing.
So instead of generating the `Unpin` implementation here, we need to
delegate automatic generation of the `Unpin` implementation to
proc-macro-derive.
We need to check this on proc-macro-derive because cfg may reduce the
fields. On the other hand, if we check this only on proc-macro-derive,
it may generate unhelpful error messages. So, we need to check this on
both proc-macro-attribute and proc-macro-derive.
use pin_project::pin_project;

#[pin_project] //~ ERROR pattern does not mention field `__field`
#[add_pinned_field]
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have never seen a crate actually attacked like this, but it should be preferable to be able to detect it.

@taiki-e taiki-e self-assigned this Sep 7, 2019
@taiki-e taiki-e force-pushed the fix-issue-68 branch 2 times, most recently from 41eee20 to 6a8f202 Compare September 7, 2019 15:53
@taiki-e
Copy link
Owner Author

taiki-e commented Sep 7, 2019

bors r+

bors bot added a commit that referenced this pull request Sep 7, 2019
77: Move automatic generation of Unpin implementation to proc-macro-derive r=taiki-e a=taiki-e

To generate the correct `Unpin` implementation, we need to collect the
types of the pinned fields. However, since proc-macro-attribute is
applied before cfg, we cannot be collecting field types at this timing.
So instead of generating the `Unpin` implementation here, we need to
delegate automatic generation of the `Unpin` implementation to
proc-macro-derive.

Fixes #68

Co-authored-by: Taiki Endo <te316e89@gmail.com>
@bors
Copy link
Contributor

bors bot commented Sep 7, 2019

Build succeeded

  • taiki-e.pin-project

@bors bors bot merged commit 6a0d419 into master Sep 7, 2019
@taiki-e taiki-e deleted the fix-issue-68 branch September 7, 2019 16:39
@taiki-e taiki-e mentioned this pull request Sep 25, 2019
bors bot added a commit that referenced this pull request Sep 25, 2019
109: Release 0.4.0 r=taiki-e a=taiki-e

cc #21

### Changes since the latest 0.3 release:

* **Pin projection has become a safe operation.** In the absence of other unsafe code that you write, it is impossible to cause undefined behavior. (#18)

* `#[unsafe_project]` attribute has been replaced with `#[pin_project]` attribute. (#18, #33)

* The `Unpin` argument has been removed - an `Unpin` impl is now generated by default. (#18)

* Drop impls must be specified with `#[pinned_drop]` instead of via a normal `Drop` impl. (#18, #33, #86)

* `Unpin` impls must be specified with an impl of `UnsafeUnpin`, instead of implementing the normal `Unpin` trait. (#18)

* `#[pin_project]` attribute now determines the visibility of the projection type/method is based on the original type. (#96)

* `#[pin_project]` can now be used for public type with private field types. (#53)

* `#[pin_project]` can now interoperate with `#[cfg()]`. (#77)

* Added `project_ref` method to `#[pin_project]` types. (#93)

* Added `#[project_ref]` attribute. (#93)

* Removed "project_attr" feature and always enable `#[project]` attribute. (#94)

* `#[project]` attribute can now be used for `impl` blocks. (#46)

* `#[project]` attribute can now be used for `use` statements. (#85)

* `#[project]` attribute now supports `match` expressions at the position of the initializer expression of `let` expressions. (#51)

### Changes since the 0.4.0-beta.1 release:

* Fixed an issue that caused an error when using `#[pin_project(UnsafeUnpin)]` and not providing a manual `UnsafeUnpin` implementation on a type with no generics or lifetime. (#107)


Co-authored-by: Taiki Endo <te316e89@gmail.com>
bors bot added a commit that referenced this pull request Oct 14, 2019
135: Move most of the processing to proc-macro-derive r=taiki-e a=taiki-e

To generate the correct `Unpin` implementation and the projection methods, we need to collect the types of the pinned fields.  However, since proc-macro-attribute is applied before `#[cfg]` and `#[cfg_attr]` on fields, we cannot be collecting field types properly at this timing. So instead of generating the `Unpin` implementation and the projection methods here, delegate their processing to proc-macro-derive.

#77 moved only the automatic generation of `Unpin` implementation to proc-macro-derive but found that it could not support `#[cfg_attr]`, so this moves more processing.

See also #68 (comment).

This also adds support for the use of `#[cfg]` on tuple structs and tuple variants.

Closes #123

Co-authored-by: Taiki Endo <te316e89@gmail.com>
@taiki-e taiki-e removed their assignment Oct 14, 2019
@taiki-e taiki-e added the A-pin-projection Area: #[pin_project] label Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pin-projection Area: #[pin_project]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pin-project does not interoperate well with #[cfg()]
1 participant