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

More attribute refactoring #134

Merged
merged 6 commits into from Oct 15, 2020
Merged

More attribute refactoring #134

merged 6 commits into from Oct 15, 2020

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Oct 11, 2020

This has two effects that may not be obvious from the commit messages / on first sight:

  • strum(default = "true") and strum(disabled = "true") no longer produce a dedicated "deprecation" compiler error
  • the strum_discriminants "passthrough" of other attributes now supports attributes that don't parse as syn::Meta, e.g. foo(key = some::Path), since syn only supports literals in the value position of MetaLists but the compiler allows arbitrary token streams in attributes

I'll probably work on converting all of the panics to syn::Errors next :)

Copy link
Owner

@Peternator7 Peternator7 left a comment

Choose a reason for hiding this comment

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

Looks really great! Thanks for putting all this work in. Couple of minor points, and the TryFrom back-compat issue, but otherwise not much.

strum_macros/src/helpers/case_style.rs Outdated Show resolved Hide resolved
strum_macros/src/helpers/metadata.rs Show resolved Hide resolved
strum_macros/src/helpers/type_props.rs Show resolved Hide resolved
@@ -130,7 +126,7 @@ pub fn enum_discriminants_inner(ast: &DeriveInput) -> syn::Result<TokenStream> {
Ok(quote! {
/// Auto-generated discriminant enum variants
#derives
#(#pass_though_attributes)*
#(#[ #pass_though_attributes ])*
Copy link
Owner

Choose a reason for hiding this comment

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

Could you help me understand the difference here?

Copy link
Contributor Author

@jplatte jplatte Oct 12, 2020

Choose a reason for hiding this comment

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

#() interpolations can contain arbitrary extra tokens in addition to an iterable of impl ToTokens. So instead of

let values = vec![1, 2, 3];
let push_values = values.into_iter().map(|v| quote! { vec.push(#v); });
quote! {
    let vec = Vec::new();
    #(#push_values)
}

you can just write

let values = vec![1, 2, 3];
quote! {
    let vec = Vec::new();
    #( vec.push(#values); )
}

I did the same kind of simplification here (context).

@Peternator7
Copy link
Owner

I don't think Self alias for enums was stablized until 1.37. Otherwise, still seems good.

@jplatte
Copy link
Contributor Author

jplatte commented Oct 15, 2020

Sorry, not getting notifications about Travis CI failure. Should be fixed now :)

@Peternator7 Peternator7 merged commit 25eaa5a into Peternator7:master Oct 15, 2020
@Peternator7
Copy link
Owner

Everything looks good. Thanks for all the changes! Do you have more coming?

@jplatte
Copy link
Contributor Author

jplatte commented Oct 15, 2020

Yeah, still planning to replace panic!s by syn::Error and implement most or all of the features I have recently requested. Can't make any promises as for when that'll happen, but I hope to have another PR up in the coming days / weeks.

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.

None yet

2 participants