Skip to content

Commit

Permalink
Add support for deriving ContentHash for Enums
Browse files Browse the repository at this point in the history
This commit also updates the documentation regarding how ContentHash should be
implemented for enums to address the concerns raised in #3051.
  • Loading branch information
emesterhazy committed Feb 14, 2024
1 parent 87b767b commit 6ac1940
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 3 deletions.
1 change: 1 addition & 0 deletions lib/proc-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ proc-macro = true
proc-macro2 = { workspace=true }
quote = { workspace=true }
syn = { workspace=true }

51 changes: 49 additions & 2 deletions lib/proc-macros/src/content_hash.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use proc_macro2::TokenStream;
use quote::{quote, quote_spanned};
use quote::{format_ident, quote, quote_spanned};
use syn::spanned::Spanned;
use syn::{parse_quote, Data, Fields, GenericParam, Generics, Index};
use syn::{parse_quote, Data, Field, Fields, GenericParam, Generics, Index, Variant};

pub fn add_trait_bounds(mut generics: Generics) -> Generics {
for param in &mut generics.params {
Expand Down Expand Up @@ -44,6 +44,53 @@ pub fn generate_hash_impl(data: &Data) -> TokenStream {
quote! {}
}
},
Data::Enum(ref data) => {
let match_hash_statements =
data.variants
.iter()
.enumerate()
.map(|(i, v)| match &v.fields {
Fields::Named(fields) => match_arm_with_fields(i, v, fields.named.iter()),
Fields::Unnamed(fields) => {
match_arm_with_fields(i, v, fields.unnamed.iter())
}
Fields::Unit => {
let ix: u8 = 0;
quote_spanned! {v.span() =>
Self::#v => {state.update(&#ix.to_le_bytes());}
}
}
});
quote! {
match self {
#(#match_hash_statements)*
}
}
}
_ => unimplemented!("ContentHash can only be derived for structs."),
}
}

fn match_arm_with_fields<'a>(
index: usize,
variant: &Variant,
fields: impl IntoIterator<Item = &'a Field>,
) -> TokenStream {
let bindings = fields
.into_iter()
.enumerate()
.map(|(i, f)| {
// If the field is named, use the name, otherwise generate a placeholder name.
f.ident.clone().unwrap_or(format_ident!("field_{}", i))
})
.collect::<Vec<_>>();
let variant_id = &variant.ident;
let ix = u8::try_from(index).unwrap();

quote_spanned! {variant.span() =>
Self::#variant_id( #(#bindings),* ) => {
state.update(&#ix.to_le_bytes());
#( crate::content_hash::ContentHash::hash(#bindings, state); )*
}
}
}
14 changes: 13 additions & 1 deletion lib/src/content_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub(crate) use jj_lib_proc_macros::ContentHash;
/// Variable-length sequences should hash a 64-bit little-endian representation
/// of their length, then their elements in order. Unordered containers should
/// order their elements according to their `Ord` implementation. Enums should
/// hash a 32-bit little-endian encoding of the ordinal number of the enum
/// hash an 8-bit little-endian encoding of the ordinal number of the enum
/// variant, then the variant's fields in lexical order.
///
/// Structs can implement `ContentHash` by using `#[derive(ContentHash)]`.
Expand Down Expand Up @@ -234,6 +234,18 @@ mod tests {
);
}

// Test that the derived version of `ContentHash` matches the that's
// manually implemented for `std::Option`.
#[test]
fn derive_for_enum() {
#[derive(ContentHash)]
enum MyOption<T> {
None,
Some(T),
}
assert_eq!(hash(&Option::<i32>::None), hash(&MyOption::<i32>::None));
assert_eq!(hash(&Some(1)), hash(&MyOption::Some(1)));
}
// This will be removed once all uses of content_hash! are replaced by the
// derive version.
#[test]
Expand Down

0 comments on commit 6ac1940

Please sign in to comment.