taiki-e / pin-project-lite

A lightweight version of pin-project written with declarative macros.
https://docs.rs/pin-project-lite
Apache License 2.0
216 stars 15 forks source link

Calling pin_project with a proxy macro does not mark inner projections as pub(crate) #77

Open notgull opened 1 year ago

notgull commented 1 year ago

Finally tracked down the source of https://github.com/smol-rs/event-listener/issues/61. If another macro is used to call pin_project!, pub will sometimes not be properly matched by pin_project!, and a pub projection method will be emitted.

See here for my full reproduction, but the gist of the issue is as follows:

Crate #1:

pub use pin_project_lite;

#[macro_export]
macro_rules! wrapper {
    ($vis:vis) => {
        $crate::pin_project_lite::pin_project! {
            $vis struct Wrapper<T> {
                #[pin]
                pub inner: T,
            }
        }
    };
}

Crate #2:

//! Docs!

#![forbid(missing_docs)]

use exporter_crate::wrapper;

wrapper! { pub }

Result of running cargo build:

error: missing documentation for a struct
 --> user-crate/src/lib.rs:9:1
  |
9 | wrapper! { pub }
  | ^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> user-crate/src/lib.rs:5:11
  |
5 | #![forbid(missing_docs)]
  |           ^^^^^^^^^^^^
  = note: this error originates in the macro `$crate::__pin_project_reconstruct` which comes from the expansion of the macro `wrapper` (in Nightly builds, run with -Z macro-backtrace for more info)

...along with a handful of other errors complaining about missing docs on other things isolated to the pin_project_lite constant.

This smells like a rustc bug to me, as this issue doesn't crop up when the import goes across modules instead of across crates.

taiki-e commented 1 year ago
        $crate::pin_project_lite::pin_project! {
            $vis struct Wrapper<T> {
                #[pin]
                pub inner: T,
            }
        }

The warning seems reasonable since you are passing an undocumented pub struct to pin_project!.

pin_project! only downgrades the visibility of the projected struct, not the visibility of the input struct itself.

notgull commented 1 year ago

Even if I document the struct and make its type private...

pub use pin_project_lite;

#[macro_export]
macro_rules! wrapper {
    ($vis:vis) => {
        $crate::pin_project_lite::pin_project! {
            /// Docs!
            $vis struct Wrapper<T> {
                #[pin]
                inner: T,
            }
        }
    };
}

...I still get this error:

error: missing documentation for a method
 --> user-crate/src/lib.rs:9:1
  |
9 | wrapper! { pub }
  | ^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> user-crate/src/lib.rs:5:11
  |
5 | #![forbid(missing_docs)]
  |           ^^^^^^^^^^^^
  = note: this error originates in the macro `$crate::__pin_project_struct_make_proj_method` which comes from the expansion of the macro `wrapper` (in Nightly builds, run with -Z macro-backtrace for more info)
taiki-e commented 1 year ago

Oh, that's odd. If pin_project! is correctly downgrading visibility in the generated code, it sounds like https://github.com/rust-lang/rust/issues/77973. In that case, #78 is the correct workaround. EDIT: https://github.com/taiki-e/pin-project-lite/issues/77#issuecomment-1667988850

Another possibility is that the visibility of the input was first interpreted as :vis in wrapper!, so it did not interpret as pub in pin_project! and visibility was not downgraded. This case is also a compiler bug, but the correct workaround is more complex https://github.com/taiki-e/pin-project-lite/issues/77#issuecomment-1666943632.

taiki-e commented 1 year ago

In the latter case, the reasonable workaround is probably to remove the code for visibility parsing and always use pub(crate) as the visibility of the projected struct.

taiki-e commented 1 year ago

I checked the output from cargo-expand and it is the latter.

#![feature(prelude_import)]
//! Docs!
#![forbid(missing_docs)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
use exporter_crate::wrapper;
/// Docs!
pub struct Wrapper<T> {
    inner: T,
}
#[allow(explicit_outlives_requirements)]
#[allow(single_use_lifetimes)]
#[allow(clippy::unknown_clippy_lints)]
#[allow(clippy::redundant_pub_crate)]
#[allow(clippy::used_underscore_binding)]
const _: () = {
    #[allow(dead_code)]
    #[allow(single_use_lifetimes)]
    #[allow(clippy::unknown_clippy_lints)]
    #[allow(clippy::mut_mut)]
    #[allow(clippy::redundant_pub_crate)]
    #[allow(clippy::ref_option_ref)]
    #[allow(clippy::type_repetition_in_bounds)]
    pub struct Projection<'__pin, T>
    where
        Wrapper<T>: '__pin,
    {
        inner: ::pin_project_lite::__private::Pin<&'__pin mut (T)>,
    }
    #[allow(dead_code)]
    #[allow(single_use_lifetimes)]
    #[allow(clippy::unknown_clippy_lints)]
    #[allow(clippy::mut_mut)]
    #[allow(clippy::redundant_pub_crate)]
    #[allow(clippy::ref_option_ref)]
    #[allow(clippy::type_repetition_in_bounds)]
    pub struct ProjectionRef<'__pin, T>
    where
        Wrapper<T>: '__pin,
    {
        inner: ::pin_project_lite::__private::Pin<&'__pin (T)>,
    }
    impl<T> Wrapper<T> {
        #[inline]
        pub fn project<'__pin>(
            self: ::pin_project_lite::__private::Pin<&'__pin mut Self>,
        ) -> Projection<'__pin, T> {
            unsafe {
                let Self { inner } = self.get_unchecked_mut();
                Projection {
                    inner: ::pin_project_lite::__private::Pin::new_unchecked(inner),
                }
            }
        }
        #[inline]
        pub fn project_ref<'__pin>(
            self: ::pin_project_lite::__private::Pin<&'__pin Self>,
        ) -> ProjectionRef<'__pin, T> {
            unsafe {
                let Self { inner } = self.get_ref();
                ProjectionRef {
                    inner: ::pin_project_lite::__private::Pin::new_unchecked(inner),
                }
            }
        }
    }
    #[allow(non_snake_case)]
    pub struct __Origin<'__pin, T> {
        __dummy_lifetime: ::pin_project_lite::__private::PhantomData<&'__pin ()>,
        inner: T,
    }
    impl<'__pin, T> ::pin_project_lite::__private::Unpin for Wrapper<T>
    where
        __Origin<'__pin, T>: ::pin_project_lite::__private::Unpin,
    {}
    trait MustNotImplDrop {}
    #[allow(clippy::drop_bounds, drop_bounds)]
    impl<T: ::pin_project_lite::__private::Drop> MustNotImplDrop for T {}
    impl<T> MustNotImplDrop for Wrapper<T> {}
    #[forbid(unaligned_references, safe_packed_borrows)]
    fn __assert_not_repr_packed<T>(this: &Wrapper<T>) {
        let _ = &this.inner;
    }
};
taiki-e commented 1 year ago

The main problem here is these two:

Then:

In the short term, I will adopt the last one to mitigate the problems (address the second problem), and in the medium term, I plan to fix it with #79 or another way.