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

Cannot interoperate with other field attributes #3

Open
taiki-e opened this issue Oct 22, 2019 · 7 comments
Open

Cannot interoperate with other field attributes #3

taiki-e opened this issue Oct 22, 2019 · 7 comments
Labels
C-bug Category: related to a bug. help wanted Call for participation: Help is requested to fix this issue

Comments

@taiki-e
Copy link
Owner

taiki-e commented Oct 22, 2019

In the current implementation, an error will occur if the field has an attribute other than #[pin].

pin_project! {
   #[derive(serde::Deserialize)]
   struct Struct1<T, U> {
       #[serde(default)] //~ ERROR 
       pinned: Option<T>,
       unpinned: U,
    }
}
@taiki-e taiki-e added C-bug Category: related to a bug. C-enhancement Category: A new feature or an improvement for an existing one labels Oct 22, 2019
@taiki-e
Copy link
Owner Author

taiki-e commented Oct 22, 2019

This is a bug, but I'm not sure if it can be fully fixed.

@taiki-e taiki-e added the help wanted Call for participation: Help is requested to fix this issue label Oct 22, 2019
@taiki-e taiki-e removed help wanted Call for participation: Help is requested to fix this issue C-enhancement Category: A new feature or an improvement for an existing one labels Jun 4, 2020
@taiki-e
Copy link
Owner Author

taiki-e commented Oct 5, 2020

This also affects doc-comment on fields (as that is actually#[doc = "..."] attributes).

@taiki-e
Copy link
Owner Author

taiki-e commented Oct 24, 2020

Another case: #28 (comment)

  • The #[project] (and #[project_ref]) attribute must precede the other attributes except for #[doc]:

    pin_project! {
        /// documents (`#[doc]`) can be placed before `#[project]`.
        #[derive(Clone)] // <--- Error
        #[project = EnumProj] 
        #[derive(Debug)] // <--- Ok
        enum Enum<T, U> {
            Struct {
                #[pin]
                pinned: T,
                unpinned: U,
            },
            Unit,
        }
    }

@taiki-e taiki-e added the help wanted Call for participation: Help is requested to fix this issue label Nov 13, 2020
@taiki-e
Copy link
Owner Author

taiki-e commented Dec 15, 2020

Workaround: If the attribute is #[doc] (doc comment), this can be avoided by changing it to normal comment.

  pin_project! {
     #[derive(serde::Deserialize)]
     struct Struct1<T, U> {
-        /// description
+        // description
         pinned: Option<T>,
         unpinned: U,
      }
  }

@taiki-e
Copy link
Owner Author

taiki-e commented Jan 12, 2021

Note: This issue is about attributes that apply only to the original struct/enum.

Proper handling of cfg/cfg_attr on fields and variants requires a more complex approach than the fix for this (it needs to be aware of the kind of attribute and propagate some of it to projected type). Even pin-project could not find the correct solution other than "leave to the compiler". And it is impossible to use the same trick in pin-project-lite.

See taiki-e/pin-project#68 (comment) and hyperium/hyper#2388 (comment) for a known workaround of the cfg issue.

(The cfg issue is not a "limitation that may be fixed" but a "limitation that is considered impossible to fix" and is a similar category as "wontfix". Actually, pin-project-lite cannot handle it properly on both fields and variants.)

@tesaguri
Copy link

tesaguri commented Feb 6, 2021

I have written a macro as a workaround for #[cfg] attributes on enum variants and fields.

https://gist.github.com/tesaguri/2a1c0790a48bbda3dd7f71c26d02a793

What this does is to rewrite something like this:

enum Foo {
    #[cfg(a)]
    A { /* ... */ }
    #[cfg(b)]
    B { /* ... */ }
}

to this:

#[cfg(all(a, b))]
enum Foo {
    A { /* ... */ }
    B { /* ... */ }
}

#[cfg(all(a, not(b)))]
enum Foo {
    A { /* ... */ }
}

#[cfg(all(not(a), b))]
enum Foo {
    B { /* ... */ }
}

#[cfg(all(not(a), not(b)))]
enum Foo {
}

and pass the token tree to pin_project!.

Michael-J-Ward added a commit to Michael-J-Ward/tower that referenced this issue Jul 23, 2021
Note: the doc comment on AsyncResponseFuture::service was also reduced to a regular comment.

This is a known limitation of pin_project_lite that the they have labeled as "help wanted".
taiki-e/pin-project-lite#3 (comment)
Michael-J-Ward added a commit to Michael-J-Ward/tower that referenced this issue Jul 23, 2021
pin_project_lite has the current limitation of nto supporting doc comments
taiki-e/pin-project-lite#3 (comment)

pin_project_lite does not and will not support tuple variants
https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
LucioFranco pushed a commit to tower-rs/tower that referenced this issue Jul 28, 2021
* REMOVE ME updates peak_wema test to pass

* adds pin_project_lite dependency

* uses pin_project_lite for load::Constant

* uses pin_project_lite for load::PencingRequestsDiscover

* uses pin_project_lite for load::PeakEwma

* uses pin_project_lite for load::Completion

* uses pin_project_lite for tests::support::IntoStream

Turns IntoStream into a regular struct because pin_project_lite does not and will support tuple structs.

https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408

* refactors opaque_future into a regular struct

This enables migration to pin_project_lite, which does not and will not support tuple structs
https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408

* migrates opaque_future to use pin_project_lite

* removes tuple variant from load_shed::ResponseState enum

* migrates load_shed::future to pin_project_lite

* removes tuple variant from filter::future::State

* migrates filter::future to pin_project_lite

Note: the doc comment on AsyncResponseFuture::service was also reduced to a regular comment.

This is a known limitation of pin_project_lite that the they have labeled as "help wanted".
taiki-e/pin-project-lite#3 (comment)

* migrates retry::Retry to pin_project_lite

* refactors retry::future::State to enable pin_project_lite

pin_project_lite has the current limitation of nto supporting doc comments
taiki-e/pin-project-lite#3 (comment)

pin_project_lite does not and will not support tuple variants
https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408

* migrates retry::future to pin_project_lite

* migrates spawn_ready::make to pin_project_lite

* refactors buffer::future::ResponseState to allow pin_project_lite

* migrates buffer::future to pin_project_lite

* refactors util::AndThenFuture to allow pin_project_lite

* migrates util::AndThenFuture to pin_project_lite

* migrates hedge::Future to pin_project_lite

* migrates hedge::select::ResponseFuture to pin_project_lite

* refactors hedge::delay enum for pin_project_lite

* refactors reconnect::future enum for pin_project_lite

* refactors oneshot::State enum for pin_project_lite

* migrates util::oneshot to pin_project_lite

* migrates reconnect::future to pin_project_lite

* migrates hedge::delay to pin_project_lite

* migrates hedge::latency to pin_project_lite

* migrates discover::list to pin_project_lite

* migrates timeout::future to pin_project_lite

* migrates balance::pool to pin_project_lite

* migrates balance::p2c::make to pin_project_lite

* migrates balance::p2c::service to pin_project_lite

* migrates call_all::ordered to pin_project_lite

* migrates call_all::common to pin_project_lite

* migrates call_all::unordered to pin_project_lite

* migrates util::optional::future to pin_project_lite

* migrates limit::concurrency::future to pin_project_lite

* migrates tower-balance example to pin_project_lite

* applies cargo fmt

* migrates tower-test to pin_project_lite

* fixes cargo hack check

peak_wma and pending_requests will now properly compile without the "discover" feature enabled.

* fixes lint rename warning on nightly

broken_intra_doc_links has been renamed to rustdoc::broken_intra_doc_links

* migrates buffer::Worker to pin_project_lite

pin_project_lite does support PinnedDrop
https://github.com/taiki-e/pin-project-lite/pull/25/files

However, it does not support generic trait bounds on the PinnedDrop impl.

To workaround this, I removed the T::Error bound from the Worker struct definition,
and moved `close_semaphore` to a a new impl without that trait bound.

* fixes abort_on_drop test

This test was also failing on master.

* applies cargo fmt
@tmccombs
Copy link

Workaround: If the attribute is #[doc] (doc comment), this can be avoided by changing it to normal comment.

This doesn't work if you actually want the comment in the generated documentation. And if you have #![deny(missing_docs)] enabled on your crate, you are really in a pickle, because if the field is public, it will give you an error if it isn't documented, and you can't use #[allow(missing_docs)] or #[doc(hidden)] either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: related to a bug. help wanted Call for participation: Help is requested to fix this issue
Projects
None yet
Development

No branches or pull requests

3 participants