Resolve offsets as well as arrays

font-codegen/src/fields.rs

@@ -1,6 +1,6 @@
 //! methods on fields
 
-use std::ops::Deref;
+use std::{backtrace::Backtrace, ops::Deref};
 
 use proc_macro2::TokenStream;
 use quote::{quote, ToTokens};
@@ -345,6 +345,23 @@ fn traversal_arm_for_field(
     }
 }
 
+fn check_resolution(phase: Phase, field_type: &FieldType) -> syn::Result<()> {
+    if let Phase::Parse = phase {
+        return Ok(());
+    }
+    if let FieldType::PendingResolution { typ } = field_type {
+        eprintln!("{}", Backtrace::capture()); // very helpful when troubleshooting
+        return Err(syn::Error::new(
+            typ.span(),
+            format!(
+                "{}: be ye struct or scalar? - we certainly don't know.",
+                typ.to_string()
+            ),
+        ));
+    }
+    Ok(())
+}
+
 impl Field {
     pub(crate) fn type_for_record(&self) -> TokenStream {
         match &self.typ {
@@ -412,6 +429,7 @@ impl Field {
 
     /// Sanity check we are in a sane state for the end of phase
     fn sanity_check(&self, phase: Phase) -> syn::Result<()> {
+        check_resolution(phase, &self.typ)?;
         if let FieldType::Array { inner_typ } = &self.typ {
             if matches!(
                 inner_typ.as_ref(),
@@ -422,6 +440,12 @@ impl Field {
                     "nested arrays are not allowed",
                 ));
             }
+            check_resolution(phase, &inner_typ)?;
+        }
+        if let FieldType::Offset { target, .. } = &self.typ {
+            if let Some(OffsetTarget::Array(inner_typ)) = target {
+                check_resolution(phase, &inner_typ)?;
+            }
         }
         if self.is_array() && self.attrs.count.is_none() {
             return Err(syn::Error::new(
@@ -446,14 +470,7 @@ impl Field {
                 _ => (),
             }
         }
-        if let Phase::Analysis = phase {
-            if let FieldType::PendingResolution { typ } = &self.typ {
-                return Err(syn::Error::new(
-                    typ.span(),
-                    "be ye struct or scalar? - we certainly don't know",
-                ));
-            }
-        }
+
         Ok(())
     }
 
@@ -540,10 +557,10 @@ impl Field {
             FieldType::Array { inner_typ } => match inner_typ.as_ref() {
                 FieldType::Offset { typ, .. } if self.is_nullable() => {
                     quote!(&'a [BigEndian<Nullable<#typ>>])
-                },
+                }
                 FieldType::Offset { typ, .. } | FieldType::Scalar { typ } => {
                     quote!(&'a [BigEndian<#typ>])
-                },
+                }
                 FieldType::Struct { typ } => quote!(&'a [#typ]),
                 FieldType::PendingResolution { typ } => quote!( &'a [#typ] ),
                 _ => unreachable!("An array should never contain {:#?}", inner_typ),

font-codegen/src/lib.rs

@@ -11,7 +11,7 @@ mod parsing;
 mod record;
 mod table;
 
-use parsing::{FieldType, Item, Items};
+use parsing::{FieldType, Item, Items, OffsetTarget};
 
 pub use error::ErrorReport;
 
@@ -61,15 +61,17 @@ pub fn generate_code(code_str: &str, mode: Mode) -> Result<String, syn::Error> {
 pub fn generate_parse_module(code: &str) -> Result<proc_macro2::TokenStream, syn::Error> {
     // Generation is done in 3 phases (https://github.com/googlefonts/fontations/issues/71):
     // 1. Parse
+    eprintln!("Parse");
     let mut items: Items = syn::parse_str(code)?;
     items.sanity_check(Phase::Parse)?;
 
     // 2. Contemplate (semantic analysis)
+    eprintln!("Analyze");
     resolve_pending(&mut items)?;
     items.sanity_check(Phase::Analysis)?;
 
     // 3. Generate
-
+    eprintln!("Generate");
     let mut code = Vec::new();
     for item in &items.items {
         let item_code = match item {
@@ -148,8 +150,9 @@ fn resolve_ident<'a>(
         return item;
     } else {
         panic!(
-            "I don't know what this is, do you? {}",
-            field_name.to_string().as_str()
+            "I don't know what this is, do you? {}: {}",
+            field_name.to_string().as_str(),
+            ident.to_string(),
         );
     }
 }
@@ -163,7 +166,7 @@ fn resolve_field(known: &HashMap<String, FieldType>, field: &mut Field) {
         }
     }
 
-    // Array nests FieldType, pursue the rabbit
+    // Array and offsets can nest FieldType, pursue the rabbit
     if let FieldType::Array { inner_typ } = &field.typ {
         if let FieldType::PendingResolution { typ } = inner_typ.as_ref() {
             let resolved_typ = resolve_ident(known, &field.name, typ);
@@ -175,6 +178,22 @@ fn resolve_field(known: &HashMap<String, FieldType>, field: &mut Field) {
             }
         }
     }
+
+    if let FieldType::Offset { typ, target } = &field.typ {
+        let offset_typ = typ;
+        if let Some(OffsetTarget::Array(array_of)) = target {
+            if let FieldType::PendingResolution { typ } = array_of.as_ref() {
+                let resolved_typ = resolve_ident(known, &field.name, typ);
+                *field = Field {
+                    typ: FieldType::Offset {
+                        typ: offset_typ.clone(),
+                        target: Some(OffsetTarget::Array(Box::new(resolved_typ.clone()))),
+                    },
+                    ..field.clone()
+                }
+            }
+        }
+    }
 }
 
 fn resolve_pending(items: &mut Items) -> syn::Result<()> {

font-codegen/src/parsing.rs

@@ -614,12 +614,10 @@ fn is_wellknown_scalar(path: &syn::PathSegment) -> bool {
         return false;
     }
     // TODO use spec types not Rust types
+    // TODO feels like this should be an enum
     match path.ident.to_string().as_str() {
-        "u8" => true,
-        "u16" => true,
-        "Uint24" => true,
-        "u32" => true,
-        "i16" => true,
+        "u8" | "u16" | "Uint24" | "u32" | "i16" | "GlyphId" | "Fixed" | "FWord" | "UfWord"
+        | "F2Dot14" => true,
         _ => false,
     }
 }
@@ -701,7 +699,12 @@ fn get_offset_target(input: &syn::PathArguments) -> syn::Result<Option<OffsetTar
     match get_single_generic_arg(input)? {
         Some(syn::GenericArgument::Type(syn::Type::Slice(t))) => {
             let inner = FieldType::from_syn_type(&t.elem)?;
-            if matches!(inner, FieldType::Scalar { .. } | FieldType::Struct { .. }) {
+            if matches!(
+                inner,
+                FieldType::Scalar { .. }
+                    | FieldType::Struct { .. }
+                    | FieldType::PendingResolution { .. }
+            ) {
                 Ok(Some(OffsetTarget::Array(Box::new(inner))))
             } else {
                 Err(syn::Error::new(