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

Can't use derive macros when bytemuck is re-exported from another crate. #159

Open
GlennFolker opened this issue Jan 4, 2023 · 8 comments

Comments

@GlennFolker
Copy link

error[E0433]: failed to resolve: could not find `bytemuck` in the list of imported crates
  --> crates\avocado_winit\src\render\graph.rs:65:31
   |
65 |         #[derive(Copy, Clone, Pod, Zeroable)]
   |                               ^^^ 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)

This is probably because bytemuck's proc macro crate uses ::bytemuck::* instead of bytemuck::* here.

@Lokathor
Copy link
Owner

Lokathor commented Jan 5, 2023

Using the :: prefix is the standard way to write proc-macros that impl unsafe traits. I asked on the discord and others agreed that it should stay.

If an alternative prefix is desired to be supported we'd have to expand the derive to accept a dummy attribute letting you specify the prefix, or something like that.

In general, proc-macros just aren't the best with re-exports.

@elast0ny
Copy link

elast0ny commented Jan 10, 2023

I've been running into this issue also 👍

Trying to reduce the number of duplicated dependencies in a project becomes much harder if bytemuck doesn't support being re-exported. I like your idea of adding another derive attribute so users could provide the path to bytemuck if they want.

@leod
Copy link

leod commented Apr 29, 2023

I need this in order to use #[derive(Pod)] on structs that are generated in a derive crate of mine, without the user of that derive crate having to add bytemuck as a dependency themselves. I'll admit that this is a rare situation as far as usages of bytemuck go.

I have a branch of bytemuck that seems to do the trick: main...leod:bytemuck:specify_crate_in_derive. It follows the approach suggested by @Lokathor. If folks agree that this is worth upstreaming, I'd be happy to clean it up (and maybe rename the attribute?) and create a PR.

Example usage:

mod custom_mod {
  pub use ::bytemuck;
}

#[derive(Copy, Clone, Pod, Zeroable)]
#[repr(C)]
#[bytemuck_crate(custom_mod::bytemuck)]
struct TestWithCrateAttribute {
  a: u16,
  b: u16,
}

FWIW, as @Lokathor pointed out, this is a general issue with derive macros. I wonder if there already is a pattern recognized by the Rust community that we could follow here.

@richardpringle
Copy link
Contributor

This is solved for Pod and Zeroable. Seems like it's still an issue for NoUninit

@richardpringle
Copy link
Contributor

This is solved for Pod and Zeroable. Seems like it's still an issue for NoUninit

PR here
#259

Can this issue be closed though? Or should it stay open until the other macros can do the same thing? In the above PR (at least at the time of writing), I added the bytemuck attribute to NoUninit. I'm pretty sure it could be added to the others and it should just work everywhere.

Not sure if that's desired or not.

@richardpringle
Copy link
Contributor

@Lokathor, any chance we could get a patch release of bytemuck_derive? And just curious, is there a reason bytemuck is lagging behind on its version of bytemuck_derive?

@Lokathor
Copy link
Owner

  1. yes, I'll try to remember to do this later today. Unfortunately I haven't set things up on my new phone to be able to do rust on my phone, so it'll have to be later today.

  2. regarding the version, my understanding has been that, since cargo will use the latest semver compatible version of all dependencies, bytemuck doesn't need to track the actual latest version of bytemuck_derive itself with every new bytemuck_derive release. Simply depending on the minimum version that's compatible is enough, and cargo will still select the newest version of bytemuck_derive when doing builds.

@Lokathor
Copy link
Owner

Released bytemuck_derive-v1.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants