Skip to content

Commit

Permalink
Don't generate match expressions for unit structs
Browse files Browse the repository at this point in the history
While the code generated for unit structs would *technically* work, it
accidentally created a variable binding the value to the struct name.
That struct name is naturally CamelCase, which would then create
compiler warnings inside the macro about `snake_case`.

I don't think this was directly caused by my PR, but empty structs are
now treated as unit structs, and unit structs were broken, so this
surfaced the issue.
  • Loading branch information
bgw committed Jun 4, 2024
1 parent f9c77d7 commit 4c1ca5a
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 18 deletions.
23 changes: 19 additions & 4 deletions crates/turbo-tasks-macros-shared/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ use syn::{
pub fn match_expansion<
EN: Fn(&Ident, &FieldsNamed) -> (TokenStream, TokenStream),
EU: Fn(&Ident, &FieldsUnnamed) -> (TokenStream, TokenStream),
U: Fn(&Ident) -> (TokenStream, TokenStream),
U: Fn(&Ident) -> TokenStream,
>(
derive_input: &DeriveInput,
expand_named: &EN,
expand_unnamed: &EU,
expand_unit: &U,
) -> TokenStream {
let ident = &derive_input.ident;
let expand_unit = move |ident| (TokenStream::new(), expand_unit(ident));
match &derive_input.data {
Data::Enum(DataEnum { variants, .. }) => {
let (variants_idents, (variants_fields_capture, expansion)): (
Expand Down Expand Up @@ -63,9 +64,19 @@ pub fn match_expansion<
let (captures, expansion) =
expand_fields(ident, fields, expand_named, expand_unnamed, expand_unit);

quote! {
match self {
#ident #captures => #expansion
if fields.is_empty() {
assert!(captures.is_empty());
// a match expression here doesn't make sense as there's no fields to capture,
// just pass through the inner expression.
expansion
} else {
match fields {
Fields::Named(_) | Fields::Unnamed(_) => quote! {
match self {
#ident #captures => #expansion
}
},
Fields::Unit => unreachable!(),
}
}
}
Expand All @@ -82,6 +93,10 @@ pub fn match_expansion<
}

/// Formats the fields of any structure or enum variant.
///
/// Empty lists of named or unnamed fields are treated as unit structs, as they
/// are semantically identical, and the `expand_unit` codepath can usually
/// generate better code.
pub fn expand_fields<
'ident,
'fields,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ fn hash_unnamed(_ident: &Ident, fields: &FieldsUnnamed) -> (TokenStream2, TokenS
}

/// Hashes a unit struct or enum variant (e.g. `struct Foo;`, `Foo::Bar`).
fn hash_unit(_ident: &Ident) -> (TokenStream2, TokenStream2) {
(quote! {}, quote! { { } })
fn hash_unit(_ident: &Ident) -> TokenStream2 {
quote! { { } }
}
4 changes: 2 additions & 2 deletions crates/turbo-tasks-macros/src/derive/trace_raw_vcs_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ fn trace_unnamed(_ident: &Ident, fields: &FieldsUnnamed) -> (TokenStream2, Token
)
}

fn trace_unit(_ident: &Ident) -> (TokenStream2, TokenStream2) {
(quote! {}, quote! { { } })
fn trace_unit(_ident: &Ident) -> TokenStream2 {
quote! { { } }
}
17 changes: 7 additions & 10 deletions crates/turbo-tasks-macros/src/derive/value_debug_format_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,11 @@ fn format_unnamed(ident: &Ident, fields: &FieldsUnnamed) -> (TokenStream2, Token
}

/// Formats a unit struct or enum variant (e.g. `struct Foo;`, `Foo::Bar`).
fn format_unit(ident: &Ident) -> (TokenStream2, TokenStream2) {
(
quote! {},
quote! {
FormattingStruct::new_unnamed(
stringify!(#ident),
vec![],
)
},
)
fn format_unit(ident: &Ident) -> TokenStream2 {
quote! {
FormattingStruct::new_unnamed(
stringify!(#ident),
vec![],
)
}
}

0 comments on commit 4c1ca5a

Please sign in to comment.