[codegen] Always generate Eq/Ord/Hash where possible

In write-fonts, we will generate these traits for all types.

In read-fonts, the situation is more nuanced. In the case of tables,
derive is not an option, because tables in read-fonts wrap some chunk of
bytes, but that chunk may contain trailing data that is not necessarily
part of the table in question.

More generally, in read-fonts, the semantics of things like 'equality'
are ambiguous. If a table contains an offset to subtable, should
equality be based on the raw value of the offset, or based recursively
on the contents of the child tables?

At some point we may wish to revisit this qusetion, but for now I am
punting. We *will* derive extra traits in read-fonts, but only for
records, and only when they contain only non-offset scalar values.

font-codegen/README.md

@@ -157,13 +157,6 @@ The following annotations are supported on top-level objects:
   common tables that contain offsets which point to different concrete types
   depending on the containing table, such as the `Layout` subtable shared
   between GPOS and GSUB.
-- `#[capabilities(equality, order, hash)]` Provide a list of additional
-  functionality to be implemented for this type. In Rust this means deriving 
-  additional traits. In the case of records, the trait will be derived
-  in both `read-fonts` and `write-fonts`, but in the case of tables only for
-  `write-fonts` (since tables in `read-fonts` are just byte slices, without
-  semantic information)
-
 
 #### field attributes
 - `#[nullable]`: only allowed on offsets or arrays of offsets, and indicates

font-codegen/src/lib.rs

@@ -68,7 +68,7 @@ pub(crate) fn generate_parse_module(items: &Items) -> Result<proc_macro2::TokenS
     let mut code = Vec::new();
     for item in items.iter() {
         let item_code = match item {
-            Item::Record(item) => record::generate(item)?,
+            Item::Record(item) => record::generate(item, items)?,
             Item::Table(item) => table::generate(item)?,
             Item::GenericGroup(item) => table::generate_group(item)?,
             Item::Format(item) => table::generate_format_group(item)?,

font-codegen/src/parsing.rs

@@ -66,7 +66,6 @@ pub(crate) struct TableAttrs {
     pub(crate) tag: Option<Attr<syn::LitStr>>,
     /// Custom validation behaviour, must be a fn(&self, &mut ValidationCtx) for the type
     pub(crate) validate: Option<Attr<syn::Ident>>,
-    pub(crate) capabilities: Option<Attr<Capabilities>>,
 }
 
 #[derive(Debug, Clone)]
@@ -80,18 +79,6 @@ pub(crate) struct TableReadArg {
     pub(crate) typ: syn::Ident,
 }
 
-#[derive(Clone, Debug)]
-pub(crate) struct Capabilities {
-    pub(crate) capabilities: Vec<Capability>,
-}
-
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-pub(crate) enum Capability {
-    Order,
-    Equality,
-    Hash,
-}
-
 #[derive(Debug, Clone)]
 pub(crate) struct Record {
     pub(crate) attrs: TableAttrs,
@@ -1051,7 +1038,6 @@ static SKIP_FONT_WRITE: &str = "skip_font_write";
 static SKIP_CONSTRUCTOR: &str = "skip_constructor";
 static READ_ARGS: &str = "read_args";
 static GENERIC_OFFSET: &str = "generic_offset";
-static CAPABILITIES: &str = "capabilities";
 static TAG: &str = "tag";
 
 impl Parse for TableAttrs {
@@ -1079,8 +1065,6 @@ impl Parse for TableAttrs {
                 this.read_args = Some(Attr::new(ident.clone(), attr.parse_args()?));
             } else if ident == GENERIC_OFFSET {
                 this.generic_offset = Some(Attr::new(ident.clone(), attr.parse_args()?));
-            } else if ident == CAPABILITIES {
-                this.capabilities = Some(Attr::new(ident.clone(), attr.parse_args()?));
             } else if ident == TAG {
                 let tag: syn::LitStr = parse_attr_eq_value(&attr)?;
                 if let Err(e) = Tag::new_checked(tag.value().as_bytes()) {
@@ -1146,33 +1130,6 @@ impl Parse for TableReadArg {
     }
 }
 
-impl Parse for Capabilities {
-    fn parse(input: ParseStream) -> syn::Result<Self> {
-        let mut capabilities =
-            Punctuated::<Capability, Token![,]>::parse_separated_nonempty(input)?
-                .into_iter()
-                .collect::<Vec<_>>();
-        capabilities.sort_unstable();
-        capabilities.dedup();
-        Ok(Capabilities { capabilities })
-    }
-}
-
-impl Parse for Capability {
-    fn parse(input: ParseStream) -> syn::Result<Self> {
-        let ident = input.parse::<syn::Ident>()?;
-        match ident.to_string().as_str() {
-            "equality" => Ok(Self::Equality),
-            "order" => Ok(Self::Order),
-            "hash" => Ok(Self::Hash),
-            _ => Err(logged_syn_error(
-                ident.span(),
-                "expected one of 'equality', 'order', or 'hash'",
-            )),
-        }
-    }
-}
-
 fn resolve_ident<'a>(
     known: &'a HashMap<syn::Ident, FieldType>,
     field_name: &syn::Ident,
@@ -1700,37 +1657,8 @@ impl FromIterator<(syn::Ident, NeededWhen)> for ReferencedFields {
     }
 }
 
-impl Capabilities {
-    pub(crate) fn extra_traits(&self) -> TokenStream {
-        let iter = self.capabilities.iter().flat_map(Capability::traits);
-        quote! ( #( #iter, )* )
-    }
-}
-
-impl Capability {
-    fn traits(&self) -> impl Iterator<Item = syn::Ident> + '_ {
-        match self {
-            // in order for all branches to return the same type, they are all [Option<Ident>; 2]
-            // and then we filter
-            Capability::Order => [
-                Some(syn::Ident::new("PartialOrd", Span::call_site())),
-                Some(syn::Ident::new("Ord", Span::call_site())),
-            ]
-            .into_iter(),
-            Capability::Equality => [
-                Some(syn::Ident::new("PartialEq", Span::call_site())),
-                Some(syn::Ident::new("Eq", Span::call_site())),
-            ]
-            .into_iter(),
-            Capability::Hash => {
-                [Some(syn::Ident::new("Hash", Span::call_site())), None].into_iter()
-            }
-        }
-        .flatten()
-    }
-}
-
 fn parse_attr_eq_value<T: Parse>(attr: &syn::Attribute) -> syn::Result<T> {
+    /// the tokens '= T' where 'T' is any `Parse`
     struct EqualsThing<T>(T);
 
     impl<T: Parse> Parse for EqualsThing<T> {

font-codegen/src/record.rs

@@ -5,10 +5,13 @@ use quote::{quote, ToTokens};
 
 use crate::{
     fields::FieldConstructorInfo,
-    parsing::{logged_syn_error, CustomCompile, Field, Fields, Phase, Record, TableAttrs},
+    parsing::{
+        logged_syn_error, CustomCompile, Field, FieldType, Fields, Item, Items, Phase, Record,
+        TableAttrs,
+    },
 };
 
-pub(crate) fn generate(item: &Record) -> syn::Result<TokenStream> {
+pub(crate) fn generate(item: &Record, all_items: &Items) -> syn::Result<TokenStream> {
     let name = &item.name;
     let docs = &item.attrs.docs;
     let field_names = item.fields.iter().map(|fld| &fld.name).collect::<Vec<_>>();
@@ -50,10 +53,8 @@ pub(crate) fn generate(item: &Record) -> syn::Result<TokenStream> {
     });
     let maybe_impl_read_with_args = (has_read_args).then(|| generate_read_with_args(item));
     let maybe_extra_traits = item
-        .attrs
-        .capabilities
-        .as_ref()
-        .map(|cap| cap.extra_traits());
+        .gets_extra_traits(all_items)
+        .then(|| quote!(PartialEq, Eq, PartialOrd, Ord, Hash));
 
     Ok(quote! {
     #( #docs )*
@@ -266,7 +267,6 @@ pub(crate) fn generate_compile_impl(
         }
     });
 
-    let maybe_extra_traits = attrs.capabilities.as_ref().map(|cap| cap.extra_traits());
     let constructor_args_raw = fields.iter_constructor_info().collect::<Vec<_>>();
     let constructor_args = constructor_args_raw.iter().map(
         |FieldConstructorInfo {
@@ -322,7 +322,7 @@ pub(crate) fn generate_compile_impl(
 
     Ok(quote! {
         #( #docs )*
-        #[derive(Clone, Debug, #maybe_derive_default #maybe_extra_traits)]
+        #[derive(Clone, Debug, #maybe_derive_default PartialEq, Eq, PartialOrd, Ord, Hash)]
         pub struct #name <#generic_param> {
             #( #field_decls, )*
         }
@@ -382,4 +382,30 @@ impl Record {
     fn is_zerocopy(&self) -> bool {
         self.fields.iter().all(Field::is_zerocopy_compatible)
     }
+
+    fn gets_extra_traits(&self, all_items: &Items) -> bool {
+        self.fields
+            .iter()
+            .all(|fld| can_derive_extra_traits(&fld.typ, all_items))
+    }
+}
+
+/// Returns `true` if this field is composed only of non-offset scalars.
+///
+/// This means it can contain scalars, records which only contain scalars,
+/// and arrays of these two types.
+///
+/// we do not generate these traits if a record contains an offset,
+/// because the semantics are unclear: we would be comparing the raw bytes
+/// in the offset, instead of the thing that the offset points to.
+fn can_derive_extra_traits(field_type: &FieldType, all_items: &Items) -> bool {
+    match field_type {
+        FieldType::Scalar { .. } => true,
+        FieldType::Struct { typ } => match all_items.get(typ) {
+            Some(Item::Record(record)) => record.gets_extra_traits(all_items),
+            _ => false,
+        },
+        FieldType::Array { inner_typ } => can_derive_extra_traits(inner_typ, all_items),
+        _ => false,
+    }
 }

font-codegen/src/table.rs

@@ -406,7 +406,7 @@ pub(crate) fn generate_group_compile(
 
     Ok(quote! {
         #( #docs)*
-        #[derive(Debug, Clone)]
+        #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
         pub enum #name {
             #( #variant_decls, )*
         }
@@ -499,16 +499,10 @@ pub(crate) fn generate_format_compile(
         .then(|| generate_format_from_obj(item, parse_module))
         .transpose()?;
 
-    let maybe_extra_traits = item
-        .attrs
-        .capabilities
-        .as_ref()
-        .map(|cap| cap.extra_traits());
-
     let constructors = generate_format_constructors(item, items)?;
     Ok(quote! {
         #( #docs )*
-        #[derive(Clone, Debug, #maybe_extra_traits)]
+        #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
         pub enum #name {
             #( #variants ),*
         }

font-types/src/fword.rs

@@ -3,12 +3,12 @@
 use super::Fixed;
 
 /// 16-bit signed quantity in font design units.
-#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct FWord(i16);
 
 /// 16-bit unsigned quantity in font design units.
-#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct UfWord(u16);
 

font-types/src/version.rs

@@ -4,7 +4,7 @@
 ///
 /// This is a legacy type with an unusual representation. See [the spec][] for
 /// additional details.
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct Version16Dot16(u32);
 
@@ -14,7 +14,7 @@ pub struct Version16Dot16(u32);
 /// represented as a `major_version`, `minor_version` pair. This type encodes
 /// those as a single type, which is useful for some of the generated code that
 /// parses out a version.
-#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct MajorMinor {
     /// The major version number

read-fonts/generated/font.rs

@@ -126,7 +126,7 @@ impl<'a> std::fmt::Debug for TableDirectory<'a> {
 }
 
 /// Record for a table in a font.
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct TableRecord {

read-fonts/generated/generated_avar.rs

@@ -104,7 +104,7 @@ impl<'a> std::fmt::Debug for Avar<'a> {
 }
 
 /// [SegmentMaps](https://learn.microsoft.com/en-us/typography/opentype/spec/avar#table-formats) record
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct SegmentMaps<'a> {
     /// The number of correspondence pairs for this axis.
     pub position_map_count: BigEndian<u16>,
@@ -147,7 +147,7 @@ impl<'a> SomeRecord<'a> for SegmentMaps<'a> {
 }
 
 /// [AxisValueMap](https://learn.microsoft.com/en-us/typography/opentype/spec/avar#table-formats) record
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct AxisValueMap {

read-fonts/generated/generated_cmap.rs

@@ -473,7 +473,7 @@ impl<'a> std::fmt::Debug for Cmap2<'a> {
 }
 
 /// Part of [Cmap2]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct SubHeader {
@@ -1012,7 +1012,7 @@ impl<'a> std::fmt::Debug for Cmap8<'a> {
 }
 
 /// Used in [Cmap8] and [Cmap12]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct SequentialMapGroup {
@@ -1441,7 +1441,7 @@ impl<'a> std::fmt::Debug for Cmap13<'a> {
 }
 
 /// Part of [Cmap13]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct ConstantMapGroup {
@@ -1837,7 +1837,7 @@ impl<'a> std::fmt::Debug for NonDefaultUvs<'a> {
 }
 
 /// Part of [Cmap14]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct UvsMapping {
@@ -1886,7 +1886,7 @@ impl<'a> SomeRecord<'a> for UvsMapping {
 }
 
 /// Part of [Cmap14]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 #[repr(packed)]
 pub struct UnicodeRange {