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

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

Open
notgull opened this issue Aug 6, 2023 · 6 comments · May be fixed by #79
Open

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

notgull opened this issue Aug 6, 2023 · 6 comments · May be fixed by #79
Labels
C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream)

Comments

@notgull
Copy link

notgull commented Aug 6, 2023

Finally tracked down the source of smol-rs/event-listener#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.

notgull added a commit to forkgull/pin-project-lite that referenced this issue Aug 6, 2023
@taiki-e
Copy link
Owner

taiki-e commented Aug 6, 2023

        $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.

@taiki-e taiki-e added the C-question Category: A question label Aug 6, 2023
@notgull
Copy link
Author

notgull commented Aug 6, 2023

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 taiki-e added C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream) and removed C-question Category: A question labels Aug 6, 2023
@taiki-e
Copy link
Owner

taiki-e commented Aug 6, 2023

Oh, that's odd. If pin_project! is correctly downgrading visibility in the generated code, it sounds like rust-lang/rust#77973. In that case, #78 is the correct workaround. EDIT: #77 (comment)

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 #77 (comment).

@taiki-e
Copy link
Owner

taiki-e commented Aug 6, 2023

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
Copy link
Owner

taiki-e commented Aug 7, 2023

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
Copy link
Owner

taiki-e commented Aug 9, 2023

The main problem here is these two:

  • (When the visibility of the original type is pub and it has been parsed once by caller macro,) the visibility is not correctly parsed by pin-project-lite due to the rustc bug, causing the projected types/methods to be generated as pub types/methods.
  • Due to the above problem, the projected types/methods appear in the documentation as if they were part of the public API.
    ss

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-upstream-bug Category: This is a bug of compiler or dependencies (the fix may require action in the upstream)
Projects
None yet
2 participants