[read-fonts] Stop asking me for FontData! Which one do you even mean?!

[qxliu76]

Noticed when reading a wrong Feature table, no errors are issued. Below's an example code from gsub.rs in klippa:

impl<'a> NameIdClosure for Gsub<'a> {
    fn collect_name_ids(&self, plan: &mut Plan) {
        let Ok(feature_list) = self.feature_list() else {
            return;
        };
        for (i, feature_record) in feature_list.feature_records().iter().enumerate() {
            if !plan.gsub_features.contains_key(&(i as u16)) {
                continue;
            }
            let Ok(feature) = feature_record.feature(self.offset_data()) else {
                continue;
            };
            feature.collect_name_ids(plan);
        }
    }
}

This line let Ok(feature) = feature_record.feature(self.offset_data()) is reading Feature from a wrong FontData, but I can still get an Ok() result and continue to collect_name_ids from the wrong Feature table.
We should try to fix this.

[rsheeter]

This is a pita that applies in many places. Types would be nice, e.g. if we could make newtypes to make the relationship between the thing that gives you the correct FontData and the thing that consumes it more obvious.

[cmyr]

Agree this is a bit annoying. For what it's worth, any method that requires you to pass in the data explicitly should also document where that data is expected to come from (see docs here) and we can't pass the data ourselves during codegen because the record that contains the offset does not have a reference to the parent table which owns the offset data.

We could get fancier in codegen (as rod says) with adding types to the offset data and that might be worth investigating at some point, but it will add a fair bit of codegen code and might have other complications I'm not foreseeing, so I don't think it's an immediate priority.

[rsheeter]

It has burned me and Ginger. I consider that fairly strong evidence of a papercut. Chad had some ideas about making ncier accessors.

[dfrg]

The basic idea is that we'd add a new type which I called RecordWithOffset (or maybe RecordRef to match TableRef) that would look something like:

pub struct RecordRef<'a, T>(FontData<'a>, T);

// Easy access to underlying record
impl Deref for RecordRef<'_, T> {
    type Target = T;
    fn deref(&self) -> &T { &self.1 }
}

And codegen would need to generate methods helper RecordRef for various types:

impl<'a> RecordRef<'a, ScriptRecord> {
    pub fn script(&self) -> Result<Script<'a>, ReadError> {
        self.1.script(self.0)
    }
}

The parent table would have to detect when a field has a record (or array of record) type that contains an offset. We could then codegen accessors that return RecordRef instead. This could be done with some semantic analysis or with a new attribute.

Note that it's not always clear which offset data to provide and some types might require manual impls. For example, the name table has the separate string data area which is used to resolve name records and each record has a length that is used to slice the relevant data.