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

Split macro implementation to a dedicated crate #13

Closed
wants to merge 3 commits into from

Conversation

pickx
Copy link

@pickx pickx commented Apr 13, 2024

fixes #12
as far as I understand it, this is the usual way proc macro crates work around this reexporting issue.
I believe this is the minimal change required here, and feel free to modify this however you like.

while changing this setup to a workspace, it's likely that I forgot something (maybe in the build script or in Cargo.toml?). please make sure that things still work.

@jun-sheaf
Copy link
Contributor

bstr is emitted in code, so why would exporting this in the macro package itself have an effect?

@pickx
Copy link
Author

pickx commented Apr 13, 2024

this is because of a combination of two changes

  • the line pub use bstr; in the crate unfmt
  • the change use ::bstr::{ByteSlice, BStr}; -> use ::unfmt::bstr::{ByteSlice, BStr}; in the crate unfmt-impl

so the macro expansion now refers to the bstr that is made available from the unfmt crate which gets "linked" to the "real" bstr crate. this gets resolved because the user has specified a dependency on unfmt in Cargo.toml. and that would trigger rustc to download the crate bstr. previously, rustc searched for bstr in the path ::bstr which was undeclared.
in fact, doing it this way is more hygienic - you're now guaranteeing that the user is getting the correct bstr::ByteSlice and bstr::BStr, and not just whatever that path happens to resolve to in the user's code.

it would be nice if we could just put pub use bstr; in the original unfmt, but that's not allowed:

error: `proc-macro` crate types currently cannot export any items other than functions tagged with `#[proc_macro]`, `#[proc_macro_derive]`, or `#[proc_macro_attribute]`

so we gotta introduce that new crate.

@jun-sheaf
Copy link
Contributor

Makes sense. We'll look into it. The changes make sense, but we are a bit particular about restructures as large as this. We will check it out next week.

@jun-sheaf
Copy link
Contributor

Fixed in #14

@jun-sheaf jun-sheaf closed this Apr 14, 2024
@pickx pickx deleted the export branch April 14, 2024 11:31
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

Successfully merging this pull request may close these issues.

crate does not re-export bstr, therefore requires users to add this dependency
2 participants