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

Add FromRepr, ToRepr, TryFromKnownRepr derives based on design from #22 #24

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

fu5ha
Copy link

@fu5ha fu5ha commented Oct 2, 2024

Does as the title says, implementing a few derives I want based on the design outlined in #22.

Since attributes we emit from the main attribute proc macro must be defined/consumed by something else (and we can't define helper attributes from an attribute proc macro, only from derive proc macros), we detect when derives are present on the main proc macro and only emit the necessary helper attributes that will actually be consumed.

@@ -102,6 +104,32 @@ fn emit_debug_impl<'a>(
})
}

fn path_matches_any_of_our_derives(got_path: &syn::Path, our_derives: &[&str]) -> bool {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and some other parts of the macros will have to change some if we ever want to allow providing a custom crate path like some other proc macros do...

Comment on lines +205 to +207
let mut should_emit_repr_helper = false;
let mut should_emit_variants_helper = false;
let mut should_emit_alias_helper = false;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking, it might be useful to have configs on the top level attribute macro to force emitting these helpers, in case third party derive macros would want to consume them? But I didn't think it was worth it for now, so I left it.

@kupiakos
Copy link
Owner

kupiakos commented Dec 9, 2024

I really appreciate you taking a crack at this, and I'm sorry for not seeing this PR - I've been on an extended vacation. The basic idea behind the code is right, but:

  • This code should not be introducing further dependencies on the current name of the crate (according to the user). I should have mentioned this more specifically, but a major motivation behind Use custom derive macros for all derive functionality #22 was in reducing the need for this sort of hack. In the next major version, the only hack will be in specially detecting and intercepting certain built-in derives like Debug.
  • I'd prefer to keep any necessary open-enum metadata in a single attribute, unless I hear a strong argument otherwise.

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