Skip to content

Conversation

@zicklag
Copy link
Contributor

@zicklag zicklag commented Oct 18, 2023

Hey! How do you feel about re-exporting gc_arena. I've needed to use a couple of it's types such as Mutation and the Collect trait, but when adding piccolo as a git dependency is trickier to make sure I have the same version of gc_arena, and I think it'd be nice not to have to worry about adding it to your Cargo.toml and making sure the versions match under normal circumstances.

Not sure if we should worry about the gc_arena derive macro or not.

@kyren
Copy link
Owner

kyren commented Oct 19, 2023

I've tried this before and there's no way to re-export gc-arena where it preserves the derive macro correctly.

Since you quickly run into needing the gc-arena derive macro, I just decided not to bother. It's unfortunate, but currently afaics unavoidable.

We could re-export the gc_arena crate just for uses that don't involve the derive macro, but that seems to make the non-trivial case (needing the derive macro) more confusing rather than less because you'd have potentially split imports... it just doesn't seem worth it.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 19, 2023

Oh, yeah, I forgot about that. I've run into the same issue. There's only a couple ways I've found to solve it:

  • Have the derive macro use gc_arena without the leading :: so that it will find gc_arena wherever it is in scope, and then you can use piccolo::gc_arena to satisfy the derive macro. In bones I also put it in the prelude, so that using, for example, piccolo::prelude::*, will bring it into scope for the derive macro.
  • Have an extra attribute on the derive macro that lets you specify which gc_arena path to use. This one is not cool for users.

Edit: But I'm not super strong on having this anyway, so I don't care what we do.

@kyren kyren force-pushed the master branch 2 times, most recently from 194308a to b0e9412 Compare November 22, 2023 05:43
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.

2 participants