From cd6467c42358e33a43219a7e3700ff0b20f38bca Mon Sep 17 00:00:00 2001 From: Evan Mesterhazy Date: Tue, 20 Feb 2024 09:44:03 -0500 Subject: [PATCH] Output better error messages when deriving ContentHash for an enum fails Consider this code: ``` struct NoContentHash {} #[derive(ContentHash)] enum Hashable { NoCanHash(NoContentHash), Empty, } ``` Before this commit, it generates an error like this: ``` error[E0277]: the trait bound `NoContentHash: ContentHash` is not satisfied --> lib/src/content_hash.rs:150:10 | 150 | #[derive(ContentHash)] | ^^^^^^^^^^^ the trait `ContentHash` is not implemented for `NoContentHash` 151 | enum Hashable { 152 | NoCanHash(NoContentHash), | --------- required by a bound introduced by this call | = help: the following other types implement trait `ContentHash`: bool i32 i64 u8 u32 u64 std::collections::HashMap BTreeMap and 35 others For more information about this error, try `rustc --explain E0277`. ``` After this commit, it generates a better error message: ``` error[E0277]: the trait bound `NoContentHash: ContentHash` is not satisfied --> lib/src/content_hash.rs:152:15 | 152 | NoCanHash(NoContentHash), | ^^^^^^^^^^^^^ the trait `ContentHash` is not implemented for `NoContentHash` | = help: the following other types implement trait `ContentHash`: bool i32 i64 u8 u32 u64 std::collections::HashMap BTreeMap and 35 others For more information about this error, try `rustc --explain E0277`. error: could not compile `jj-lib` (lib) due to 1 previous error ``` It also works for enum variants with named fields: ``` error[E0277]: the trait bound `NoContentHash: ContentHash` is not satisfied --> lib/src/content_hash.rs:152:23 | 152 | NoCanHash { named: NoContentHash }, | ^^^^^^^^^^^^^ the trait `ContentHash` is not implemented for `NoContentHash` | = help: the following other types implement trait `ContentHash`: bool i32 i64 u8 u32 u64 std::collections::HashMap BTreeMap and 35 others For more information about this error, try `rustc --explain E0277`. ``` --- lib/proc-macros/src/content_hash.rs | 45 +++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/proc-macros/src/content_hash.rs b/lib/proc-macros/src/content_hash.rs index faa4ea04ecb..8b38ba7791e 100644 --- a/lib/proc-macros/src/content_hash.rs +++ b/lib/proc-macros/src/content_hash.rs @@ -1,7 +1,7 @@ use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote, quote_spanned}; use syn::spanned::Spanned; -use syn::{parse_quote, Data, Field, Fields, GenericParam, Generics, Index}; +use syn::{parse_quote, Data, Field, Fields, GenericParam, Generics, Index, Type}; pub fn add_trait_bounds(mut generics: Generics) -> Generics { for param in &mut generics.params { @@ -54,21 +54,21 @@ pub fn generate_hash_impl(data: &Data) -> TokenStream { match &v.fields { Fields::Named(fields) => { let bindings = enum_bindings(fields.named.iter()); - let ix = index_to_ordinal(i); + let hash_statements = + hash_statements_for_enum_fields(i, fields.named.iter()); quote_spanned! {v.span() => Self::#variant_id{ #(#bindings),* } => { - ::jj_lib::content_hash::ContentHash::hash(&#ix, state); - #( ::jj_lib::content_hash::ContentHash::hash(#bindings, state); )* + #(#hash_statements)* } } } Fields::Unnamed(fields) => { let bindings = enum_bindings(fields.unnamed.iter()); - let ix = index_to_ordinal(i); + let hash_statements = + hash_statements_for_enum_fields(i, fields.unnamed.iter()); quote_spanned! {v.span() => Self::#variant_id( #(#bindings),* ) => { - ::jj_lib::content_hash::ContentHash::hash(&#ix, state); - #( ::jj_lib::content_hash::ContentHash::hash(#bindings, state); )* + #(#hash_statements)* } } } @@ -99,13 +99,40 @@ fn index_to_ordinal(ix: usize) -> u32 { u32::try_from(ix).expect("The number of enum variants overflows a u32.") } -fn enum_bindings<'a>(fields: impl IntoIterator) -> Vec { +fn enum_bindings_with_type<'a>(fields: impl IntoIterator) -> Vec<(Type, Ident)> { 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)) + ( + f.ty.clone(), + f.ident.clone().unwrap_or(format_ident!("field_{}", i)), + ) }) .collect::>() } + +fn enum_bindings<'a>(fields: impl IntoIterator) -> Vec { + enum_bindings_with_type(fields) + .into_iter() + .map(|(_, b)| b) + .collect() +} + +fn hash_statements_for_enum_fields<'a>( + index: usize, + fields: impl IntoIterator, +) -> Vec { + let ix = index_to_ordinal(index); + let typed_bindings = enum_bindings_with_type(fields); + let mut hash_statements = Vec::with_capacity(typed_bindings.len() + 1); + hash_statements.push(quote! {::jj_lib::content_hash::ContentHash::hash(&#ix, state);}); + for (ty, b) in typed_bindings.iter() { + hash_statements.push(quote_spanned! {b.span() => + <#ty as ::jj_lib::content_hash::ContentHash>::hash(&#b, state); + }); + } + + hash_statements +}