Crash in kernFeatureWriter

[simoncozens]

Here is a minimal Glyphs file which dies with an assertion failure:

{
.appVersion = "3177";
featurePrefixes = (
{
code = "languagesystem DFLT dflt;\012languagesystem gonm dflt;";
name = Languagesystems;
}
);
fontMaster = (
{
ascender = 827;
capHeight = 630;
descender = -200;
id = "55C6F8EE-B60C-49C4-8D15-2255C9205224";
weightValue = 90;
xHeight = 357;
}
);
glyphs = (
{
glyphname = Kssa.MGondi;
layers = (
{
layerId = "55C6F8EE-B60C-49C4-8D15-2255C9205224";
width = 1158;
}
);
unicode = 11D2E;
category = Letter;
},
{
glyphname = MatraAu.MGondi;
layers = (
{
layerId = "55C6F8EE-B60C-49C4-8D15-2255C9205224";
width = 155;
}
);
leftKerningGroup = MatraAu.MGondi;
unicode = 11D3F;
category = Letter;
},
{
glyphname = Anusvara.alt.MGondi;
layers = (
{
layerId = "55C6F8EE-B60C-49C4-8D15-2255C9205224";
width = 0;
}
);
category = Mark;
subCategory = Nonspacing;
}
);
kerning = {
"55C6F8EE-B60C-49C4-8D15-2255C9205224" = {
"Kssa.MGondi" = {
"@MMK_R_MatraAu.MGondi" = -20;
};
Anusvara.alt.MGondi = {
"@MMK_R_MatraAu.MGondi" = 180;
};
};
};
unitsPerEm = 1000;
}

and the error is:

  File "/Users/simon/others-repos/ufo2ft/Lib/ufo2ft/featureWriters/kernFeatureWriter.py", line 378, in _makeKerningLookups
    self._makeSplitScriptKernLookups(
  File "/Users/simon/others-repos/ufo2ft/Lib/ufo2ft/featureWriters/kernFeatureWriter.py", line 405, in _makeSplitScriptKernLookups
    assert not classDefs.keys() & self.context.kerning.classDefs.keys()
AssertionError

[simoncozens]

_makeKerningLookups is called twice. The first time, it runs through self._makeSplitScriptKernLookups(lookups, basePairs) with a base-base pair:

[KerningPair(side1='Kssa.MGondi', side2=('MatraAu.MGondi',), value=-20)]

Because it's the first time, self.context.kerning.classDefs is empty, so not classDefs.keys() & self.context.kerning.classDefs.keys() is of course fine. Then we add the class definition kern2.Gonm.MatraAu.MGondi to self.context.kerning.classDefs, so the next time around, it's in there and that causes the crash.

Next, it runs through the if markPairs on line 377, but this is a mark-base pair:

[KerningPair(side1='Anusvara.alt.MGondi', side2=('MatraAu.MGondi',), value=180)]

This is still a class definition, and that's absolutely fine, but since we have seen this class already before, in the "base pairs" phase, we explode.

I think the assertion (introduced in 23dcdd3b6) is bad. It's not unreasonable to kern a_base @some_bases and also kern a_mark @some_bases. One will end up in the bases kern lookup and the other in the marks kern lookup, and there shouldn't be an assumption that the class has not been seen before. So I would remove the assertion, but first I would cheek whether there was a specific issue that that commit was trying to address... @madig?

[madig]

I think I was making sure I wasn't accidentally redefining classes because I remembered that this code is run twice sometimes. I suppose a better assert would be to ensure that the intersection contains duplicates in fact, not redefinitions with different members.

[madig]

I found an instance where the seen-before class would actually be redefined and overwrite the previously created classDef AST with a different one, losing kerning.