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

Inconsistent visibility on some derive-synthesized types #2177

Open
joshlf opened this issue Dec 21, 2024 · 1 comment
Open

Inconsistent visibility on some derive-synthesized types #2177

joshlf opened this issue Dec 21, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@joshlf
Copy link
Member

joshlf commented Dec 21, 2024

Prompted by this CI failure in rend.

Seems like this is caused by #2055 and #2127. Fixing it should be a simple matter of using the same visibility on synthesized types as we see on the type decorated with #[derive(KnownLayout)] and adding #[doc(hidden)].

@joshlf joshlf added the bug Something isn't working label Dec 21, 2024
@jswrenn
Copy link
Collaborator

jswrenn commented Dec 22, 2024

Fixing it should be a simple matter of using the same visibility on synthesized types as we see on the type decorated with #[derive(KnownLayout)] and adding #[doc(hidden)].

We currently use field visibility, not the outer type visibility: https://github.com/google/zerocopy/blob/2c8ef7463c109138aa3202622bd3f93b884385a9/zerocopy-derive/src/lib.rs#L281C1-L282C1

This is correct, as it allows us to accommodate types whose fields have a type which is less visible than the enclosing type.

What's going on here is way weirder. This is a minimum replication of the issue:

use zerocopy_derive::KnownLayout;

macro_rules! define {
    ($name:ident, $repr:ty) => {
        #[derive(KnownLayout)]
        #[repr(C)]
        pub struct $name($repr);
    }
}

define!(Foo, u8);

...that triggers the private_bounds lint:

warning: type `__Zerocopy_Field_0` is more private than the item `__ZerocopyKnownLayoutMaybeUninit`
  --> examples/foo.rs:7:9
   |
7  |         pub struct $name($repr);
   |         ^^^ struct `__ZerocopyKnownLayoutMaybeUninit` is reachable at visibility `pub`
...
11 | define!(Foo, u8);
   | ---------------- in this macro invocation
   |
note: but type `__Zerocopy_Field_0` is only usable at visibility `pub(crate)`
  --> examples/foo.rs:5:18
   |
5  |         #[derive(KnownLayout)]
   |                  ^^^^^^^^^^^
...
11 | define!(Foo, u8);
   | ---------------- in this macro invocation
   = note: `#[warn(private_bounds)]` on by default
   = note: this warning originates in the macro `define` (in Nightly builds, run with -Z macro-backtrace for more info)

What's very weird is the macro indirection is essential here. (Don't even ask how long it took me to figure this out.) This program, for example, does not trigger the lint:

use zerocopy_derive::KnownLayout;

#[derive(KnownLayout)]
#[repr(C)]
pub struct Foo(u8);

fn main() {}

This strikes me as probably a rustc bug. I'll dig deeper to confirm, but the good news is that the fix on our end is as simple as slapping an allow(private_bounds) onto our derive output.

jswrenn added a commit that referenced this issue Dec 23, 2024
For reasons not-yet-known, the `private_bounds` lint flags the
output of `derive(KnownLayout)` on `repr(C)` structs generated
by macros. This is likely a rustc bug, but to preserve a good
experience for our users, we nonetheless, `allow(private_bounds)`.

Fixes #2177
jswrenn added a commit that referenced this issue Dec 23, 2024
For reasons not-yet-known, the `private_bounds` lint flags the
output of `derive(KnownLayout)` on `repr(C)` structs generated
by macros. This is likely a rustc bug, but to preserve a good
experience for our users, we nonetheless, `allow(private_bounds)`.

Fixes #2177
github-merge-queue bot pushed a commit that referenced this issue Dec 23, 2024
For reasons not-yet-known, the `private_bounds` lint flags the
output of `derive(KnownLayout)` on `repr(C)` structs generated
by macros. This is likely a rustc bug, but to preserve a good
experience for our users, we nonetheless, `allow(private_bounds)`.

Fixes #2177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants