Lokathor / bytemuck

A crate for mucking around with piles of bytes
https://docs.rs/bytemuck
Apache License 2.0
712 stars 77 forks source link

Error when re-exporting `bytemuck` and using derive macros #93

Open Rua opened 2 years ago

Rua commented 2 years ago

I re-export from one crate like this:

pub use bytemuck;

...and then import Zeroable and Pod from this re-exported location. Then I add

#[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]

This triggers a compiler error:

error[E0433]: failed to resolve: could not find `bytemuck` in the list of imported crates
  --> examples/src/lib.rs:14:39
   |
14 | #[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
   |                                       ^^^^^^^^ could not find `bytemuck` in the list of imported crates
   |
   = note: this error originates in the derive macro `Zeroable` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: could not find `bytemuck` in the list of imported crates
  --> examples/src/lib.rs:14:49
   |
14 | #[derive(Clone, Copy, Debug, Default, Zeroable, Pod)]
   |                                                 ^^^ could not find `bytemuck` in the list of imported crates
   |
   = note: this error originates in the derive macro `Pod` (in Nightly builds, run with -Z macro-backtrace for more info)

When I add bytemuck as a dependency directly, and use Zeroable and Pod from there, the error goes away. It seems that the internals of the derive macro can't handle an alternative export path?

Lokathor commented 2 years ago

https://github.com/Lokathor/bytemuck/blob/main/derive/src/traits.rs#L93 (and the similar methods on the other derives) seems to be the offender. They're always using ::bytemuck (with the absolute prefix) rather than bytemuck (no prefix).

I'm not super familiar with the derive code, but given that it's deriving unsafe traits i think the extra caution in the import path is probably appropriate.

If someone can submit a PR to fix this without causing potential problems with other traits under the same name ending up derivable and leading to strange bugs I'm all for that. Otherwise I might just say that in this particular case people should just use the full import path to work around this problem. Safety beats ergonomics in this case.

danielhenrymantilla commented 2 years ago

This is a known hard problem for proc-macros. I've been recently re-thinking about this, and here is the least-bad solution I can think of:

  1. Make ::bytemuck export a hidden-ish new proc-macro derive, say ZeroableUnqualified, which acts like Zeroable but for not prefixing bytemuck with the :: disambiguator.

  2. Then any dependency that wishes to re-export Zeroable, such as the OP's, can then do:

    pub mod bytemuck {
        pub use ::bytemuck::{*, ZeroableUnqualified as Zeroable};
    }
  3. A 2nd-degree dependency can then do:

    pub use ::middle_crate::bytemuck;
    
    #[derive(…, bytemuck::Zeroable)]
    struct Foo …

    or

    pub use ::middle_crate::bytemuck::{self, Zeroable};
    
    #[derive(…, Zeroable)]
    struct Foo …

Another approach, more classic but imho more cumbersome for the 2nd-degree dependency, would be for Zeroable to take an optional #[zeroable(crate = …)] parameter with which to override the currently hard-coded ::bytemuck.

This could even palliate a bit better the unsafe-ty @Lokathor was legitimately concerned about[^macro_safety]: it's easy to change #[zeroable(crate = …)] to #[zeroable(unsafe { crate = … })], for instance.

At that point the ::middle_crate can keep using pub use ::bytemuck;, as usual, and the 2nd-degree dependency would have to write:

pub use ::middle_crate::bytemuck::{self, Zeroable};

#[derive(…, Zeroable)]
#[zeroable(unsafe { crate = bytemuck })]
// or
#[zeroable(unsafe { crate = ::middle_crate::bytemuck })]
struct Foo …

[^macro_safety]: that being said, the story of macros and unsafety is known to be quite disappointing; the current realistic status quo is that the macro callsites are expected not to go to crazy with renamings and shadowings