Move feature builders to otlLib

Refactors feaLib, moving code which builds OpenType lookups into otlLib. Note that this changes feaLib's concept of `location` from a tuple to an object.

Lib/fontTools/feaLib/ast.py

@@ -1,5 +1,6 @@
 from fontTools.misc.py23 import *
 from fontTools.feaLib.error import FeatureLibError
+from fontTools.feaLib.location import FeatureLibLocation
 from fontTools.misc.encodingTools import getEncoding
 from collections import OrderedDict
 import itertools
@@ -112,7 +113,9 @@ class Element(object):
     """A base class representing "something" in a feature file."""
 
     def __init__(self, location=None):
-        #: location of this element - tuple of ``(filename, line, column)``
+        #: location of this element as a `FeatureLibLocation` object.
+        if location and not isinstance(location, FeatureLibLocation):
+            location = FeatureLibLocation(*location)
         self.location = location
 
     def build(self, builder):
@@ -460,8 +463,7 @@ class MarkClass(object):
                 if otherLoc is None:
                     end = ""
                 else:
-                    end = " at %s:%d:%d" % (
+                    end = f" at {otherLoc}"
-                        otherLoc[0], otherLoc[1], otherLoc[2])
                 raise FeatureLibError(
                     "Glyph %s already defined%s" % (glyph, end),
                     definition.location)

Lib/fontTools/feaLib/builder.py

@@ -8,7 +8,24 @@ from fontTools.otlLib import builder as otl
 from fontTools.otlLib.maxContextCalc import maxCtxFont
 from fontTools.ttLib import newTable, getTableModule
 from fontTools.ttLib.tables import otBase, otTables
-from collections import defaultdict, OrderedDict
+from fontTools.otlLib.builder import (
+    AlternateSubstBuilder,
+    ChainContextPosBuilder,
+    ChainContextSubstBuilder,
+    LigatureSubstBuilder,
+    MultipleSubstBuilder,
+    CursivePosBuilder,
+    MarkBasePosBuilder,
+    MarkLigPosBuilder,
+    MarkMarkPosBuilder,
+    ReverseChainSingleSubstBuilder,
+    SingleSubstBuilder,
+    ClassPairPosSubtableBuilder,
+    PairPosBuilder,
+    SinglePosBuilder,
+)
+from fontTools.otlLib.error import OpenTypeLibError
+from collections import defaultdict
 import itertools
 import logging
 
@@ -560,7 +577,11 @@ class Builder(object):
                 continue
             lookup.lookup_index = len(lookups)
             lookups.append(lookup)
-        return [l.build() for l in lookups]
+        try:
+            otLookups = [l.build() for l in lookups]
+        except OpenTypeLibError as e:
+            raise FeatureLibError(str(e), e.location) from e
+        return otLookups
 
     def makeTable(self, tag):
         table = getattr(otTables, tag, None)()
@@ -774,8 +795,7 @@ class Builder(object):
                 _, loc = self.markAttach_[glyph]
                 raise FeatureLibError(
                     "Glyph %s already has been assigned "
-                    "a MarkAttachmentType at %s:%d:%d" % (
+                    "a MarkAttachmentType at %s" % (glyph, loc),
-                        glyph, loc[0], loc[1], loc[2]),
                     location)
             self.markAttach_[glyph] = (id_, location)
         return id_
@@ -942,7 +962,7 @@ class Builder(object):
                     'Removing duplicate multiple substitution from glyph'
                     ' "%s" to %s%s',
                     glyph, replacements,
-                    ' at {}:{}:{}'.format(*location) if location else '',
+                    f' at {location}' if location else '',
                 )
             else:
                 raise FeatureLibError(
@@ -970,8 +990,8 @@ class Builder(object):
                 if to_glyph == lookup.mapping[from_glyph]:
                     log.info(
                         'Removing duplicate single substitution from glyph'
-                        ' "%s" to "%s" at %s:%i:%i',
+                        ' "%s" to "%s" at %s',
-                        from_glyph, to_glyph, *location,
+                        from_glyph, to_glyph, location,
                     )
                 else:
                     raise FeatureLibError(
@@ -1046,14 +1066,18 @@ class Builder(object):
     def add_class_pair_pos(self, location, glyphclass1, value1,
                            glyphclass2, value2):
         lookup = self.get_lookup_(location, PairPosBuilder)
-        lookup.addClassPair(location, glyphclass1, value1, glyphclass2, value2)
+        v1 = makeOpenTypeValueRecord(value1, pairPosContext=True)
+        v2 = makeOpenTypeValueRecord(value2, pairPosContext=True)
+        lookup.addClassPair(location, glyphclass1, v1, glyphclass2, v2)
 
     def add_subtable_break(self, location):
         self.cur_lookup_.add_subtable_break(location)
 
     def add_specific_pair_pos(self, location, glyph1, value1, glyph2, value2):
         lookup = self.get_lookup_(location, PairPosBuilder)
-        lookup.addGlyphPair(location, glyph1, value1, glyph2, value2)
+        v1 = makeOpenTypeValueRecord(value1, pairPosContext=True)
+        v2 = makeOpenTypeValueRecord(value2, pairPosContext=True)
+        lookup.addGlyphPair(location, glyph1, v1, glyph2, v2)
 
     def add_single_pos(self, location, prefix, suffix, pos, forceChain):
         if prefix or suffix or forceChain:
@@ -1061,8 +1085,12 @@ class Builder(object):
         else:
             lookup = self.get_lookup_(location, SinglePosBuilder)
             for glyphs, value in pos:
+                otValueRecord = makeOpenTypeValueRecord(value, pairPosContext=False)
                 for glyph in glyphs:
-                    lookup.add_pos(location, glyph, value)
+                    try:
+                        lookup.add_pos(location, glyph, otValueRecord)
+                    except OpenTypeLibError as e:
+                        raise FeatureLibError(str(e), e.location) from e
 
     def add_single_pos_chained_(self, location, prefix, suffix, pos):
         # https://github.com/fonttools/fonttools/issues/514
@@ -1075,13 +1103,13 @@ class Builder(object):
             if value is None:
                 subs.append(None)
                 continue
-            otValue, _ = makeOpenTypeValueRecord(value, pairPosContext=False)
+            otValue = makeOpenTypeValueRecord(value, pairPosContext=False)
             sub = chain.find_chainable_single_pos(targets, glyphs, otValue)
             if sub is None:
                 sub = self.get_chained_lookup_(location, SinglePosBuilder)
                 targets.append(sub)
             for glyph in glyphs:
-                sub.add_pos(location, glyph, value)
+                sub.add_pos(location, glyph, otValue)
             subs.append(sub)
         assert len(pos) == len(subs), (pos, subs)
         chain.rules.append(
@@ -1091,8 +1119,8 @@ class Builder(object):
         oldClass, oldLocation = self.glyphClassDefs_.get(glyph, (None, None))
         if oldClass and oldClass != glyphClass:
             raise FeatureLibError(
-                "Glyph %s was assigned to a different class at %s:%s:%s" %
+                "Glyph %s was assigned to a different class at %s" %
-                (glyph, oldLocation[0], oldLocation[1], oldLocation[2]),
+                (glyph, oldLocation),
                 location)
         self.glyphClassDefs_[glyph] = (glyphClass, location)
 
@@ -1152,9 +1180,9 @@ _VALUEREC_ATTRS = {
 
 
 def makeOpenTypeValueRecord(v, pairPosContext):
-    """ast.ValueRecord --> (otBase.ValueRecord, int ValueFormat)"""
+    """ast.ValueRecord --> otBase.ValueRecord"""
     if not v:
-        return None, 0
+        return None
 
     vr = {}
     for astName, (otName, isDevice) in _VALUEREC_ATTRS.items():
@@ -1164,569 +1192,7 @@ def makeOpenTypeValueRecord(v, pairPosContext):
     if pairPosContext and not vr:
         vr = {"YAdvance": 0} if v.vertical else {"XAdvance": 0}
     valRec = otl.buildValue(vr)
-    return valRec, valRec.getFormat()
+    return valRec
 
 
-class LookupBuilder(object):
-    SUBTABLE_BREAK_ = "SUBTABLE_BREAK"
 
-    def __init__(self, font, location, table, lookup_type):
-        self.font = font
-        self.glyphMap = font.getReverseGlyphMap()
-        self.location = location
-        self.table, self.lookup_type = table, lookup_type
-        self.lookupflag = 0
-        self.markFilterSet = None
-        self.lookup_index = None  # assigned when making final tables
-        assert table in ('GPOS', 'GSUB')
-
-    def equals(self, other):
-        return (isinstance(other, self.__class__) and
-                self.table == other.table and
-                self.lookupflag == other.lookupflag and
-                self.markFilterSet == other.markFilterSet)
-
-    def inferGlyphClasses(self):
-        """Infers glyph glasses for the GDEF table, such as {"cedilla":3}."""
-        return {}
-
-    def getAlternateGlyphs(self):
-        """Helper for building 'aalt' features."""
-        return {}
-
-    def buildLookup_(self, subtables):
-        return otl.buildLookup(subtables, self.lookupflag, self.markFilterSet)
-
-    def buildMarkClasses_(self, marks):
-        """{"cedilla": ("BOTTOM", ast.Anchor), ...} --> {"BOTTOM":0, "TOP":1}
-
-        Helper for MarkBasePostBuilder, MarkLigPosBuilder, and
-        MarkMarkPosBuilder. Seems to return the same numeric IDs
-        for mark classes as the AFDKO makeotf tool.
-        """
-        ids = {}
-        for mark in sorted(marks.keys(), key=self.font.getGlyphID):
-            markClassName, _markAnchor = marks[mark]
-            if markClassName not in ids:
-                ids[markClassName] = len(ids)
-        return ids
-
-    def setBacktrackCoverage_(self, prefix, subtable):
-        subtable.BacktrackGlyphCount = len(prefix)
-        subtable.BacktrackCoverage = []
-        for p in reversed(prefix):
-            coverage = otl.buildCoverage(p, self.glyphMap)
-            subtable.BacktrackCoverage.append(coverage)
-
-    def setLookAheadCoverage_(self, suffix, subtable):
-        subtable.LookAheadGlyphCount = len(suffix)
-        subtable.LookAheadCoverage = []
-        for s in suffix:
-            coverage = otl.buildCoverage(s, self.glyphMap)
-            subtable.LookAheadCoverage.append(coverage)
-
-    def setInputCoverage_(self, glyphs, subtable):
-        subtable.InputGlyphCount = len(glyphs)
-        subtable.InputCoverage = []
-        for g in glyphs:
-            coverage = otl.buildCoverage(g, self.glyphMap)
-            subtable.InputCoverage.append(coverage)
-
-    def build_subst_subtables(self, mapping, klass):
-        substitutions = [{}]
-        for key in mapping:
-            if key[0] == self.SUBTABLE_BREAK_:
-                substitutions.append({})
-            else:
-                substitutions[-1][key] = mapping[key]
-        subtables = [klass(s) for s in substitutions]
-        return subtables
-
-    def add_subtable_break(self, location):
-        log.warning(FeatureLibError(
-            'unsupported "subtable" statement for lookup type',
-            location
-        ))
-
-
-class AlternateSubstBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GSUB', 3)
-        self.alternates = OrderedDict()
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.alternates == other.alternates)
-
-    def build(self):
-        subtables = self.build_subst_subtables(self.alternates,
-                otl.buildAlternateSubstSubtable)
-        return self.buildLookup_(subtables)
-
-    def getAlternateGlyphs(self):
-        return self.alternates
-
-    def add_subtable_break(self, location):
-        self.alternates[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
-
-
-class ChainContextPosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 8)
-        self.rules = []  # (prefix, input, suffix, lookups)
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.rules == other.rules)
-
-    def build(self):
-        subtables = []
-        for (prefix, glyphs, suffix, lookups) in self.rules:
-            if prefix == self.SUBTABLE_BREAK_:
-                continue
-            st = otTables.ChainContextPos()
-            subtables.append(st)
-            st.Format = 3
-            self.setBacktrackCoverage_(prefix, st)
-            self.setLookAheadCoverage_(suffix, st)
-            self.setInputCoverage_(glyphs, st)
-
-            st.PosCount = 0
-            st.PosLookupRecord = []
-            for sequenceIndex, lookupList in enumerate(lookups):
-                if lookupList is not None:
-                    if not isinstance(lookupList, list):
-                        # Can happen with synthesised lookups
-                        lookupList = [ lookupList ]
-                    for l in lookupList:
-                        st.PosCount += 1
-                        if l.lookup_index is None:
-                            raise FeatureLibError('Missing index of the specified '
-                                'lookup, might be a substitution lookup',
-                                self.location)
-                        rec = otTables.PosLookupRecord()
-                        rec.SequenceIndex = sequenceIndex
-                        rec.LookupListIndex = l.lookup_index
-                        st.PosLookupRecord.append(rec)
-        return self.buildLookup_(subtables)
-
-    def find_chainable_single_pos(self, lookups, glyphs, value):
-        """Helper for add_single_pos_chained_()"""
-        res = None
-        for lookup in lookups[::-1]:
-            if lookup == self.SUBTABLE_BREAK_:
-                return res
-            if isinstance(lookup, SinglePosBuilder) and \
-                    all(lookup.can_add(glyph, value) for glyph in glyphs):
-                res = lookup
-        return res
-
-    def add_subtable_break(self, location):
-        self.rules.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
-                           self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_]))
-
-
-class ChainContextSubstBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GSUB', 6)
-        self.substitutions = []  # (prefix, input, suffix, lookups)
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.substitutions == other.substitutions)
-
-    def build(self):
-        subtables = []
-        for (prefix, input, suffix, lookups) in self.substitutions:
-            if prefix == self.SUBTABLE_BREAK_:
-                continue
-            st = otTables.ChainContextSubst()
-            subtables.append(st)
-            st.Format = 3
-            self.setBacktrackCoverage_(prefix, st)
-            self.setLookAheadCoverage_(suffix, st)
-            self.setInputCoverage_(input, st)
-
-            st.SubstCount = 0
-            st.SubstLookupRecord = []
-            for sequenceIndex, lookupList in enumerate(lookups):
-                if lookupList is not None:
-                    if not isinstance(lookupList, list):
-                        # Can happen with synthesised lookups
-                        lookupList = [ lookupList ]
-                    for l in lookupList:
-                        st.SubstCount += 1
-                        if l.lookup_index is None:
-                            raise FeatureLibError('Missing index of the specified '
-                                'lookup, might be a positioning lookup',
-                                self.location)
-                        rec = otTables.SubstLookupRecord()
-                        rec.SequenceIndex = sequenceIndex
-                        rec.LookupListIndex = l.lookup_index
-                        st.SubstLookupRecord.append(rec)
-        return self.buildLookup_(subtables)
-
-    def getAlternateGlyphs(self):
-        result = {}
-        for (_, _, _, lookuplist) in self.substitutions:
-            if lookuplist == self.SUBTABLE_BREAK_:
-                continue
-            for lookups in lookuplist:
-                if not isinstance(lookups, list):
-                    lookups = [lookups]
-                for lookup in lookups:
-                    if lookup is not None:
-                        alts = lookup.getAlternateGlyphs()
-                        for glyph, replacements in alts.items():
-                            result.setdefault(glyph, set()).update(replacements)
-        return result
-
-    def find_chainable_single_subst(self, glyphs):
-        """Helper for add_single_subst_chained_()"""
-        res = None
-        for _, _, _, substitutions in self.substitutions[::-1]:
-            if substitutions == self.SUBTABLE_BREAK_:
-                return res
-            for sub in substitutions:
-                if (isinstance(sub, SingleSubstBuilder) and
-                        not any(g in glyphs for g in sub.mapping.keys())):
-                    res = sub
-        return res
-
-    def add_subtable_break(self, location):
-        self.substitutions.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
-                                   self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_))
-
-
-class LigatureSubstBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GSUB', 4)
-        self.ligatures = OrderedDict()  # {('f','f','i'): 'f_f_i'}
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.ligatures == other.ligatures)
-
-    def build(self):
-        subtables = self.build_subst_subtables(self.ligatures,
-                otl.buildLigatureSubstSubtable)
-        return self.buildLookup_(subtables)
-
-    def add_subtable_break(self, location):
-        self.ligatures[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
-
-
-class MultipleSubstBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GSUB', 2)
-        self.mapping = OrderedDict()
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.mapping == other.mapping)
-
-    def build(self):
-        subtables = self.build_subst_subtables(self.mapping,
-                otl.buildMultipleSubstSubtable)
-        return self.buildLookup_(subtables)
-
-    def add_subtable_break(self, location):
-        self.mapping[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
-
-
-class CursivePosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 3)
-        self.attachments = {}
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.attachments == other.attachments)
-
-    def add_attachment(self, location, glyphs, entryAnchor, exitAnchor):
-        for glyph in glyphs:
-            self.attachments[glyph] = (entryAnchor, exitAnchor)
-
-    def build(self):
-        st = otl.buildCursivePosSubtable(self.attachments, self.glyphMap)
-        return self.buildLookup_([st])
-
-
-class MarkBasePosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 4)
-        self.marks = {}  # glyphName -> (markClassName, anchor)
-        self.bases = {}  # glyphName -> {markClassName: anchor}
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.marks == other.marks and
-                self.bases == other.bases)
-
-    def inferGlyphClasses(self):
-        result = {glyph: 1 for glyph in self.bases}
-        result.update({glyph: 3 for glyph in self.marks})
-        return result
-
-    def build(self):
-        markClasses = self.buildMarkClasses_(self.marks)
-        marks = {mark: (markClasses[mc], anchor)
-                 for mark, (mc, anchor) in self.marks.items()}
-        bases = {}
-        for glyph, anchors in self.bases.items():
-            bases[glyph] = {markClasses[mc]: anchor
-                            for (mc, anchor) in anchors.items()}
-        subtables = otl.buildMarkBasePos(marks, bases, self.glyphMap)
-        return self.buildLookup_(subtables)
-
-
-class MarkLigPosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 5)
-        self.marks = {}  # glyphName -> (markClassName, anchor)
-        self.ligatures = {}  # glyphName -> [{markClassName: anchor}, ...]
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.marks == other.marks and
-                self.ligatures == other.ligatures)
-
-    def inferGlyphClasses(self):
-        result = {glyph: 2 for glyph in self.ligatures}
-        result.update({glyph: 3 for glyph in self.marks})
-        return result
-
-    def build(self):
-        markClasses = self.buildMarkClasses_(self.marks)
-        marks = {mark: (markClasses[mc], anchor)
-                 for mark, (mc, anchor) in self.marks.items()}
-        ligs = {}
-        for lig, components in self.ligatures.items():
-            ligs[lig] = []
-            for c in components:
-                ligs[lig].append({markClasses[mc]: a for mc, a in c.items()})
-        subtables = otl.buildMarkLigPos(marks, ligs, self.glyphMap)
-        return self.buildLookup_(subtables)
-
-
-class MarkMarkPosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 6)
-        self.marks = {}      # glyphName -> (markClassName, anchor)
-        self.baseMarks = {}  # glyphName -> {markClassName: anchor}
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.marks == other.marks and
-                self.baseMarks == other.baseMarks)
-
-    def inferGlyphClasses(self):
-        result = {glyph: 3 for glyph in self.baseMarks}
-        result.update({glyph: 3 for glyph in self.marks})
-        return result
-
-    def build(self):
-        markClasses = self.buildMarkClasses_(self.marks)
-        markClassList = sorted(markClasses.keys(), key=markClasses.get)
-        marks = {mark: (markClasses[mc], anchor)
-                 for mark, (mc, anchor) in self.marks.items()}
-
-        st = otTables.MarkMarkPos()
-        st.Format = 1
-        st.ClassCount = len(markClasses)
-        st.Mark1Coverage = otl.buildCoverage(marks, self.glyphMap)
-        st.Mark2Coverage = otl.buildCoverage(self.baseMarks, self.glyphMap)
-        st.Mark1Array = otl.buildMarkArray(marks, self.glyphMap)
-        st.Mark2Array = otTables.Mark2Array()
-        st.Mark2Array.Mark2Count = len(st.Mark2Coverage.glyphs)
-        st.Mark2Array.Mark2Record = []
-        for base in st.Mark2Coverage.glyphs:
-            anchors = [self.baseMarks[base].get(mc) for mc in markClassList]
-            st.Mark2Array.Mark2Record.append(otl.buildMark2Record(anchors))
-        return self.buildLookup_([st])
-
-
-class ReverseChainSingleSubstBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GSUB', 8)
-        self.substitutions = []  # (prefix, suffix, mapping)
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.substitutions == other.substitutions)
-
-    def build(self):
-        subtables = []
-        for prefix, suffix, mapping in self.substitutions:
-            st = otTables.ReverseChainSingleSubst()
-            st.Format = 1
-            self.setBacktrackCoverage_(prefix, st)
-            self.setLookAheadCoverage_(suffix, st)
-            st.Coverage = otl.buildCoverage(mapping.keys(), self.glyphMap)
-            st.GlyphCount = len(mapping)
-            st.Substitute = [mapping[g] for g in st.Coverage.glyphs]
-            subtables.append(st)
-        return self.buildLookup_(subtables)
-
-    def add_subtable_break(self, location):
-        # Nothing to do here, each substitution is in its own subtable.
-        pass
-
-
-class SingleSubstBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GSUB', 1)
-        self.mapping = OrderedDict()
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.mapping == other.mapping)
-
-    def build(self):
-        subtables = self.build_subst_subtables(self.mapping,
-                otl.buildSingleSubstSubtable)
-        return self.buildLookup_(subtables)
-
-    def getAlternateGlyphs(self):
-        return {glyph: set([repl]) for glyph, repl in self.mapping.items()}
-
-    def add_subtable_break(self, location):
-        self.mapping[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
-
-
-class ClassPairPosSubtableBuilder(object):
-    def __init__(self, builder, valueFormat1, valueFormat2):
-        self.builder_ = builder
-        self.classDef1_, self.classDef2_ = None, None
-        self.values_ = {}  # (glyphclass1, glyphclass2) --> (value1, value2)
-        self.valueFormat1_, self.valueFormat2_ = valueFormat1, valueFormat2
-        self.forceSubtableBreak_ = False
-        self.subtables_ = []
-
-    def addPair(self, gc1, value1, gc2, value2):
-        mergeable = (not self.forceSubtableBreak_ and
-                     self.classDef1_ is not None and
-                     self.classDef1_.canAdd(gc1) and
-                     self.classDef2_ is not None and
-                     self.classDef2_.canAdd(gc2))
-        if not mergeable:
-            self.flush_()
-            self.classDef1_ = otl.ClassDefBuilder(useClass0=True)
-            self.classDef2_ = otl.ClassDefBuilder(useClass0=False)
-            self.values_ = {}
-        self.classDef1_.add(gc1)
-        self.classDef2_.add(gc2)
-        self.values_[(gc1, gc2)] = (value1, value2)
-
-    def addSubtableBreak(self):
-        self.forceSubtableBreak_ = True
-
-    def subtables(self):
-        self.flush_()
-        return self.subtables_
-
-    def flush_(self):
-        if self.classDef1_ is None or self.classDef2_ is None:
-            return
-        st = otl.buildPairPosClassesSubtable(self.values_,
-                                             self.builder_.glyphMap)
-        if st.Coverage is None:
-            return
-        self.subtables_.append(st)
-        self.forceSubtableBreak_ = False
-
-
-class PairPosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 2)
-        self.pairs = []  # [(gc1, value1, gc2, value2)*]
-        self.glyphPairs = {}  # (glyph1, glyph2) --> (value1, value2)
-        self.locations = {}  # (gc1, gc2) --> (filepath, line, column)
-
-    def addClassPair(self, location, glyphclass1, value1, glyphclass2, value2):
-        self.pairs.append((glyphclass1, value1, glyphclass2, value2))
-
-    def addGlyphPair(self, location, glyph1, value1, glyph2, value2):
-        key = (glyph1, glyph2)
-        oldValue = self.glyphPairs.get(key, None)
-        if oldValue is not None:
-            # the Feature File spec explicitly allows specific pairs generated
-            # by an 'enum' rule to be overridden by preceding single pairs
-            otherLoc = self.locations[key]
-            log.debug(
-                'Already defined position for pair %s %s at %s:%d:%d; '
-                'choosing the first value',
-                glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2])
-        else:
-            val1, _ = makeOpenTypeValueRecord(value1, pairPosContext=True)
-            val2, _ = makeOpenTypeValueRecord(value2, pairPosContext=True)
-            self.glyphPairs[key] = (val1, val2)
-            self.locations[key] = location
-
-    def add_subtable_break(self, location):
-        self.pairs.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
-                           self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_))
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.glyphPairs == other.glyphPairs and
-                self.pairs == other.pairs)
-
-    def build(self):
-        builders = {}
-        builder = None
-        for glyphclass1, value1, glyphclass2, value2 in self.pairs:
-            if glyphclass1 is self.SUBTABLE_BREAK_:
-                if builder is not None:
-                    builder.addSubtableBreak()
-                continue
-            val1, valFormat1 = makeOpenTypeValueRecord(
-                value1, pairPosContext=True)
-            val2, valFormat2 = makeOpenTypeValueRecord(
-                value2, pairPosContext=True)
-            builder = builders.get((valFormat1, valFormat2))
-            if builder is None:
-                builder = ClassPairPosSubtableBuilder(
-                    self, valFormat1, valFormat2)
-                builders[(valFormat1, valFormat2)] = builder
-            builder.addPair(glyphclass1, val1, glyphclass2, val2)
-        subtables = []
-        if self.glyphPairs:
-            subtables.extend(
-                otl.buildPairPosGlyphs(self.glyphPairs, self.glyphMap))
-        for key in sorted(builders.keys()):
-            subtables.extend(builders[key].subtables())
-        return self.buildLookup_(subtables)
-
-
-class SinglePosBuilder(LookupBuilder):
-    def __init__(self, font, location):
-        LookupBuilder.__init__(self, font, location, 'GPOS', 1)
-        self.locations = {}  # glyph -> (filename, line, column)
-        self.mapping = {}  # glyph -> otTables.ValueRecord
-
-    def add_pos(self, location, glyph, valueRecord):
-        otValueRecord, _ = makeOpenTypeValueRecord(
-            valueRecord, pairPosContext=False)
-        if not self.can_add(glyph, otValueRecord):
-            otherLoc = self.locations[glyph]
-            raise FeatureLibError(
-                'Already defined different position for glyph "%s" at %s:%d:%d'
-                % (glyph, otherLoc[0], otherLoc[1], otherLoc[2]),
-                location)
-        if otValueRecord:
-            self.mapping[glyph] = otValueRecord
-        self.locations[glyph] = location
-
-    def can_add(self, glyph, value):
-        assert isinstance(value, otl.ValueRecord)
-        curValue = self.mapping.get(glyph)
-        return curValue is None or curValue == value
-
-    def equals(self, other):
-        return (LookupBuilder.equals(self, other) and
-                self.mapping == other.mapping)
-
-    def build(self):
-        subtables = otl.buildSinglePos(self.mapping, self.glyphMap)
-        return self.buildLookup_(subtables)

Lib/fontTools/feaLib/error.py

@@ -8,8 +8,7 @@ class FeatureLibError(Exception):
     def __str__(self):
         message = Exception.__str__(self)
         if self.location:
-            path, line, column = self.location
+            return f"{self.location}: {message}"
-            return f"{path}:{line}:{column}: {message}"
         else:
             return message
 
@@ -22,5 +21,4 @@ class IncludedFeaNotFound(FeatureLibError):
             "The following feature file should be included but cannot be found: "
             f"{Exception.__str__(self)}"
         )
-        path, line, column = self.location
+        return f"{self.location}: {message}"
-        return f"{path}:{line}:{column}: {message}"

Lib/fontTools/feaLib/lexer.py

@@ -1,5 +1,6 @@
 from fontTools.misc.py23 import *
 from fontTools.feaLib.error import FeatureLibError, IncludedFeaNotFound
+from fontTools.feaLib.location import FeatureLibLocation
 import re
 import os
 
@@ -57,7 +58,7 @@ class Lexer(object):
 
     def location_(self):
         column = self.pos_ - self.line_start_ + 1
-        return (self.filename_ or "<features>", self.line_, column)
+        return FeatureLibLocation(self.filename_ or "<features>", self.line_, column)
 
     def next_(self):
         self.scan_over_(Lexer.CHAR_WHITESPACE_)

Lib/fontTools/feaLib/location.py

@@ -0,0 +1,10 @@
+from typing import NamedTuple
+
+class FeatureLibLocation(NamedTuple):
+    """A location in a feature file"""
+    file: str
+    line: int
+    column: int
+
+    def __str__(self):
+    		return f"{self.file}:{self.line}:{self.column}"

Lib/fontTools/otlLib/builder.py

@@ -1,8 +1,14 @@
-from collections import namedtuple
+from collections import namedtuple, OrderedDict
 from fontTools.misc.fixedTools import fixedToFloat
 from fontTools import ttLib
 from fontTools.ttLib.tables import otTables as ot
 from fontTools.ttLib.tables.otBase import ValueRecord, valueRecordFormatDict
+from fontTools.ttLib.tables import otBase
+from fontTools.otlLib.error import OpenTypeLibError
+import logging
+
+
+log = logging.getLogger(__name__)
 
 
 def buildCoverage(glyphs, glyphMap):
@@ -47,6 +53,568 @@ def buildLookup(subtables, flags=0, markFilterSet=None):
     return self
 
 
+class LookupBuilder(object):
+    SUBTABLE_BREAK_ = "SUBTABLE_BREAK"
+
+    def __init__(self, font, location, table, lookup_type):
+        self.font = font
+        self.glyphMap = font.getReverseGlyphMap()
+        self.location = location
+        self.table, self.lookup_type = table, lookup_type
+        self.lookupflag = 0
+        self.markFilterSet = None
+        self.lookup_index = None  # assigned when making final tables
+        assert table in ('GPOS', 'GSUB')
+
+    def equals(self, other):
+        return (isinstance(other, self.__class__) and
+                self.table == other.table and
+                self.lookupflag == other.lookupflag and
+                self.markFilterSet == other.markFilterSet)
+
+    def inferGlyphClasses(self):
+        """Infers glyph glasses for the GDEF table, such as {"cedilla":3}."""
+        return {}
+
+    def getAlternateGlyphs(self):
+        """Helper for building 'aalt' features."""
+        return {}
+
+    def buildLookup_(self, subtables):
+        return buildLookup(subtables, self.lookupflag, self.markFilterSet)
+
+    def buildMarkClasses_(self, marks):
+        """{"cedilla": ("BOTTOM", ast.Anchor), ...} --> {"BOTTOM":0, "TOP":1}
+
+        Helper for MarkBasePostBuilder, MarkLigPosBuilder, and
+        MarkMarkPosBuilder. Seems to return the same numeric IDs
+        for mark classes as the AFDKO makeotf tool.
+        """
+        ids = {}
+        for mark in sorted(marks.keys(), key=self.font.getGlyphID):
+            markClassName, _markAnchor = marks[mark]
+            if markClassName not in ids:
+                ids[markClassName] = len(ids)
+        return ids
+
+    def setBacktrackCoverage_(self, prefix, subtable):
+        subtable.BacktrackGlyphCount = len(prefix)
+        subtable.BacktrackCoverage = []
+        for p in reversed(prefix):
+            coverage = buildCoverage(p, self.glyphMap)
+            subtable.BacktrackCoverage.append(coverage)
+
+    def setLookAheadCoverage_(self, suffix, subtable):
+        subtable.LookAheadGlyphCount = len(suffix)
+        subtable.LookAheadCoverage = []
+        for s in suffix:
+            coverage = buildCoverage(s, self.glyphMap)
+            subtable.LookAheadCoverage.append(coverage)
+
+    def setInputCoverage_(self, glyphs, subtable):
+        subtable.InputGlyphCount = len(glyphs)
+        subtable.InputCoverage = []
+        for g in glyphs:
+            coverage = buildCoverage(g, self.glyphMap)
+            subtable.InputCoverage.append(coverage)
+
+    def build_subst_subtables(self, mapping, klass):
+        substitutions = [{}]
+        for key in mapping:
+            if key[0] == self.SUBTABLE_BREAK_:
+                substitutions.append({})
+            else:
+                substitutions[-1][key] = mapping[key]
+        subtables = [klass(s) for s in substitutions]
+        return subtables
+
+    def add_subtable_break(self, location):
+        log.warning(OpenTypeLibError(
+            'unsupported "subtable" statement for lookup type',
+            location
+        ))
+
+
+class AlternateSubstBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GSUB', 3)
+        self.alternates = OrderedDict()
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.alternates == other.alternates)
+
+    def build(self):
+        subtables = self.build_subst_subtables(self.alternates,
+                buildAlternateSubstSubtable)
+        return self.buildLookup_(subtables)
+
+    def getAlternateGlyphs(self):
+        return self.alternates
+
+    def add_subtable_break(self, location):
+        self.alternates[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
+
+
+class ChainContextPosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 8)
+        self.rules = []  # (prefix, input, suffix, lookups)
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.rules == other.rules)
+
+    def build(self):
+        subtables = []
+        for (prefix, glyphs, suffix, lookups) in self.rules:
+            if prefix == self.SUBTABLE_BREAK_:
+                continue
+            st = ot.ChainContextPos()
+            subtables.append(st)
+            st.Format = 3
+            self.setBacktrackCoverage_(prefix, st)
+            self.setLookAheadCoverage_(suffix, st)
+            self.setInputCoverage_(glyphs, st)
+
+            st.PosCount = 0
+            st.PosLookupRecord = []
+            for sequenceIndex, lookupList in enumerate(lookups):
+                if lookupList is not None:
+                    if not isinstance(lookupList, list):
+                        # Can happen with synthesised lookups
+                        lookupList = [ lookupList ]
+                    for l in lookupList:
+                        st.PosCount += 1
+                        if l.lookup_index is None:
+                            raise OpenTypeLibError('Missing index of the specified '
+                                'lookup, might be a substitution lookup',
+                                self.location)
+                        rec = ot.PosLookupRecord()
+                        rec.SequenceIndex = sequenceIndex
+                        rec.LookupListIndex = l.lookup_index
+                        st.PosLookupRecord.append(rec)
+        return self.buildLookup_(subtables)
+
+    def find_chainable_single_pos(self, lookups, glyphs, value):
+        """Helper for add_single_pos_chained_()"""
+        res = None
+        for lookup in lookups[::-1]:
+            if lookup == self.SUBTABLE_BREAK_:
+                return res
+            if isinstance(lookup, SinglePosBuilder) and \
+                    all(lookup.can_add(glyph, value) for glyph in glyphs):
+                res = lookup
+        return res
+
+    def add_subtable_break(self, location):
+        self.rules.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
+                           self.SUBTABLE_BREAK_, [self.SUBTABLE_BREAK_]))
+
+
+class ChainContextSubstBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GSUB', 6)
+        self.substitutions = []  # (prefix, input, suffix, lookups)
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.substitutions == other.substitutions)
+
+    def build(self):
+        subtables = []
+        for (prefix, input, suffix, lookups) in self.substitutions:
+            if prefix == self.SUBTABLE_BREAK_:
+                continue
+            st = ot.ChainContextSubst()
+            subtables.append(st)
+            st.Format = 3
+            self.setBacktrackCoverage_(prefix, st)
+            self.setLookAheadCoverage_(suffix, st)
+            self.setInputCoverage_(input, st)
+
+            st.SubstCount = 0
+            st.SubstLookupRecord = []
+            for sequenceIndex, lookupList in enumerate(lookups):
+                if lookupList is not None:
+                    if not isinstance(lookupList, list):
+                        # Can happen with synthesised lookups
+                        lookupList = [ lookupList ]
+                    for l in lookupList:
+                        st.SubstCount += 1
+                        if l.lookup_index is None:
+                            raise OpenTypeLibError('Missing index of the specified '
+                                'lookup, might be a positioning lookup',
+                                self.location)
+                        rec = ot.SubstLookupRecord()
+                        rec.SequenceIndex = sequenceIndex
+                        rec.LookupListIndex = l.lookup_index
+                        st.SubstLookupRecord.append(rec)
+        return self.buildLookup_(subtables)
+
+    def getAlternateGlyphs(self):
+        result = {}
+        for (_, _, _, lookuplist) in self.substitutions:
+            if lookuplist == self.SUBTABLE_BREAK_:
+                continue
+            for lookups in lookuplist:
+                if not isinstance(lookups, list):
+                    lookups = [lookups]
+                for lookup in lookups:
+                    if lookup is not None:
+                        alts = lookup.getAlternateGlyphs()
+                        for glyph, replacements in alts.items():
+                            result.setdefault(glyph, set()).update(replacements)
+        return result
+
+    def find_chainable_single_subst(self, glyphs):
+        """Helper for add_single_subst_chained_()"""
+        res = None
+        for _, _, _, substitutions in self.substitutions[::-1]:
+            if substitutions == self.SUBTABLE_BREAK_:
+                return res
+            for sub in substitutions:
+                if (isinstance(sub, SingleSubstBuilder) and
+                        not any(g in glyphs for g in sub.mapping.keys())):
+                    res = sub
+        return res
+
+    def add_subtable_break(self, location):
+        self.substitutions.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
+                                   self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_))
+
+
+class LigatureSubstBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GSUB', 4)
+        self.ligatures = OrderedDict()  # {('f','f','i'): 'f_f_i'}
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.ligatures == other.ligatures)
+
+    def build(self):
+        subtables = self.build_subst_subtables(self.ligatures,
+                buildLigatureSubstSubtable)
+        return self.buildLookup_(subtables)
+
+    def add_subtable_break(self, location):
+        self.ligatures[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
+
+
+class MultipleSubstBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GSUB', 2)
+        self.mapping = OrderedDict()
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.mapping == other.mapping)
+
+    def build(self):
+        subtables = self.build_subst_subtables(self.mapping,
+                buildMultipleSubstSubtable)
+        return self.buildLookup_(subtables)
+
+    def add_subtable_break(self, location):
+        self.mapping[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
+
+
+class CursivePosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 3)
+        self.attachments = {}
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.attachments == other.attachments)
+
+    def add_attachment(self, location, glyphs, entryAnchor, exitAnchor):
+        for glyph in glyphs:
+            self.attachments[glyph] = (entryAnchor, exitAnchor)
+
+    def build(self):
+        st = buildCursivePosSubtable(self.attachments, self.glyphMap)
+        return self.buildLookup_([st])
+
+
+class MarkBasePosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 4)
+        self.marks = {}  # glyphName -> (markClassName, anchor)
+        self.bases = {}  # glyphName -> {markClassName: anchor}
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.marks == other.marks and
+                self.bases == other.bases)
+
+    def inferGlyphClasses(self):
+        result = {glyph: 1 for glyph in self.bases}
+        result.update({glyph: 3 for glyph in self.marks})
+        return result
+
+    def build(self):
+        markClasses = self.buildMarkClasses_(self.marks)
+        marks = {mark: (markClasses[mc], anchor)
+                 for mark, (mc, anchor) in self.marks.items()}
+        bases = {}
+        for glyph, anchors in self.bases.items():
+            bases[glyph] = {markClasses[mc]: anchor
+                            for (mc, anchor) in anchors.items()}
+        subtables = buildMarkBasePos(marks, bases, self.glyphMap)
+        return self.buildLookup_(subtables)
+
+
+class MarkLigPosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 5)
+        self.marks = {}  # glyphName -> (markClassName, anchor)
+        self.ligatures = {}  # glyphName -> [{markClassName: anchor}, ...]
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.marks == other.marks and
+                self.ligatures == other.ligatures)
+
+    def inferGlyphClasses(self):
+        result = {glyph: 2 for glyph in self.ligatures}
+        result.update({glyph: 3 for glyph in self.marks})
+        return result
+
+    def build(self):
+        markClasses = self.buildMarkClasses_(self.marks)
+        marks = {mark: (markClasses[mc], anchor)
+                 for mark, (mc, anchor) in self.marks.items()}
+        ligs = {}
+        for lig, components in self.ligatures.items():
+            ligs[lig] = []
+            for c in components:
+                ligs[lig].append({markClasses[mc]: a for mc, a in c.items()})
+        subtables = buildMarkLigPos(marks, ligs, self.glyphMap)
+        return self.buildLookup_(subtables)
+
+
+class MarkMarkPosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 6)
+        self.marks = {}      # glyphName -> (markClassName, anchor)
+        self.baseMarks = {}  # glyphName -> {markClassName: anchor}
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.marks == other.marks and
+                self.baseMarks == other.baseMarks)
+
+    def inferGlyphClasses(self):
+        result = {glyph: 3 for glyph in self.baseMarks}
+        result.update({glyph: 3 for glyph in self.marks})
+        return result
+
+    def build(self):
+        markClasses = self.buildMarkClasses_(self.marks)
+        markClassList = sorted(markClasses.keys(), key=markClasses.get)
+        marks = {mark: (markClasses[mc], anchor)
+                 for mark, (mc, anchor) in self.marks.items()}
+
+        st = ot.MarkMarkPos()
+        st.Format = 1
+        st.ClassCount = len(markClasses)
+        st.Mark1Coverage = buildCoverage(marks, self.glyphMap)
+        st.Mark2Coverage = buildCoverage(self.baseMarks, self.glyphMap)
+        st.Mark1Array = buildMarkArray(marks, self.glyphMap)
+        st.Mark2Array = ot.Mark2Array()
+        st.Mark2Array.Mark2Count = len(st.Mark2Coverage.glyphs)
+        st.Mark2Array.Mark2Record = []
+        for base in st.Mark2Coverage.glyphs:
+            anchors = [self.baseMarks[base].get(mc) for mc in markClassList]
+            st.Mark2Array.Mark2Record.append(buildMark2Record(anchors))
+        return self.buildLookup_([st])
+
+
+class ReverseChainSingleSubstBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GSUB', 8)
+        self.substitutions = []  # (prefix, suffix, mapping)
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.substitutions == other.substitutions)
+
+    def build(self):
+        subtables = []
+        for prefix, suffix, mapping in self.substitutions:
+            st = ot.ReverseChainSingleSubst()
+            st.Format = 1
+            self.setBacktrackCoverage_(prefix, st)
+            self.setLookAheadCoverage_(suffix, st)
+            st.Coverage = buildCoverage(mapping.keys(), self.glyphMap)
+            st.GlyphCount = len(mapping)
+            st.Substitute = [mapping[g] for g in st.Coverage.glyphs]
+            subtables.append(st)
+        return self.buildLookup_(subtables)
+
+    def add_subtable_break(self, location):
+        # Nothing to do here, each substitution is in its own subtable.
+        pass
+
+
+class SingleSubstBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GSUB', 1)
+        self.mapping = OrderedDict()
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.mapping == other.mapping)
+
+    def build(self):
+        subtables = self.build_subst_subtables(self.mapping,
+                buildSingleSubstSubtable)
+        return self.buildLookup_(subtables)
+
+    def getAlternateGlyphs(self):
+        return {glyph: set([repl]) for glyph, repl in self.mapping.items()}
+
+    def add_subtable_break(self, location):
+        self.mapping[(self.SUBTABLE_BREAK_, location)] = self.SUBTABLE_BREAK_
+
+
+class ClassPairPosSubtableBuilder(object):
+    def __init__(self, builder, valueFormat1, valueFormat2):
+        self.builder_ = builder
+        self.classDef1_, self.classDef2_ = None, None
+        self.values_ = {}  # (glyphclass1, glyphclass2) --> (value1, value2)
+        self.valueFormat1_, self.valueFormat2_ = valueFormat1, valueFormat2
+        self.forceSubtableBreak_ = False
+        self.subtables_ = []
+
+    def addPair(self, gc1, value1, gc2, value2):
+        mergeable = (not self.forceSubtableBreak_ and
+                     self.classDef1_ is not None and
+                     self.classDef1_.canAdd(gc1) and
+                     self.classDef2_ is not None and
+                     self.classDef2_.canAdd(gc2))
+        if not mergeable:
+            self.flush_()
+            self.classDef1_ = ClassDefBuilder(useClass0=True)
+            self.classDef2_ = ClassDefBuilder(useClass0=False)
+            self.values_ = {}
+        self.classDef1_.add(gc1)
+        self.classDef2_.add(gc2)
+        self.values_[(gc1, gc2)] = (value1, value2)
+
+    def addSubtableBreak(self):
+        self.forceSubtableBreak_ = True
+
+    def subtables(self):
+        self.flush_()
+        return self.subtables_
+
+    def flush_(self):
+        if self.classDef1_ is None or self.classDef2_ is None:
+            return
+        st = buildPairPosClassesSubtable(self.values_,
+                                             self.builder_.glyphMap)
+        if st.Coverage is None:
+            return
+        self.subtables_.append(st)
+        self.forceSubtableBreak_ = False
+
+
+class PairPosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 2)
+        self.pairs = []  # [(gc1, value1, gc2, value2)*]
+        self.glyphPairs = {}  # (glyph1, glyph2) --> (value1, value2)
+        self.locations = {}  # (gc1, gc2) --> (filepath, line, column)
+
+    def addClassPair(self, location, glyphclass1, value1, glyphclass2, value2):
+        self.pairs.append((glyphclass1, value1, glyphclass2, value2))
+
+    def addGlyphPair(self, location, glyph1, value1, glyph2, value2):
+        key = (glyph1, glyph2)
+        oldValue = self.glyphPairs.get(key, None)
+        if oldValue is not None:
+            # the Feature File spec explicitly allows specific pairs generated
+            # by an 'enum' rule to be overridden by preceding single pairs
+            otherLoc = self.locations[key]
+            log.debug(
+                'Already defined position for pair %s %s at %s; '
+                'choosing the first value',
+                glyph1, glyph2, otherLoc)
+        else:
+            self.glyphPairs[key] = (value1, value2)
+            self.locations[key] = location
+
+    def add_subtable_break(self, location):
+        self.pairs.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_,
+                           self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_))
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.glyphPairs == other.glyphPairs and
+                self.pairs == other.pairs)
+
+    def build(self):
+        builders = {}
+        builder = None
+        for glyphclass1, value1, glyphclass2, value2 in self.pairs:
+            if glyphclass1 is self.SUBTABLE_BREAK_:
+                if builder is not None:
+                    builder.addSubtableBreak()
+                continue
+            valFormat1, valFormat2 = 0, 0
+            if value1:
+                valFormat1 = value1.getFormat()
+            if value2:
+                valFormat2 = value2.getFormat()
+            builder = builders.get((valFormat1, valFormat2))
+            if builder is None:
+                builder = ClassPairPosSubtableBuilder(
+                    self, valFormat1, valFormat2)
+                builders[(valFormat1, valFormat2)] = builder
+            builder.addPair(glyphclass1, value1, glyphclass2, value2)
+        subtables = []
+        if self.glyphPairs:
+            subtables.extend(
+                buildPairPosGlyphs(self.glyphPairs, self.glyphMap))
+        for key in sorted(builders.keys()):
+            subtables.extend(builders[key].subtables())
+        return self.buildLookup_(subtables)
+
+
+class SinglePosBuilder(LookupBuilder):
+    def __init__(self, font, location):
+        LookupBuilder.__init__(self, font, location, 'GPOS', 1)
+        self.locations = {}  # glyph -> (filename, line, column)
+        self.mapping = {}  # glyph -> ot.ValueRecord
+
+    def add_pos(self, location, glyph, otValueRecord):
+        if not self.can_add(glyph, otValueRecord):
+            otherLoc = self.locations[glyph]
+            raise OpenTypeLibError(
+                'Already defined different position for glyph "%s" at %s'
+                % (glyph, otherLoc),
+                location)
+        if otValueRecord:
+            self.mapping[glyph] = otValueRecord
+        self.locations[glyph] = location
+
+    def can_add(self, glyph, value):
+        assert isinstance(value, ValueRecord)
+        curValue = self.mapping.get(glyph)
+        return curValue is None or curValue == value
+
+    def equals(self, other):
+        return (LookupBuilder.equals(self, other) and
+                self.mapping == other.mapping)
+
+    def build(self):
+        subtables = buildSinglePos(self.mapping, self.glyphMap)
+        return self.buildLookup_(subtables)
+
+
 # GSUB
 
 
@@ -137,14 +705,14 @@ def buildBaseArray(bases, numMarkClasses, glyphMap):
 
 
 def buildBaseRecord(anchors):
-    """[otTables.Anchor, otTables.Anchor, ...] --> otTables.BaseRecord"""
+    """[ot.Anchor, ot.Anchor, ...] --> ot.BaseRecord"""
     self = ot.BaseRecord()
     self.BaseAnchor = anchors
     return self
 
 
 def buildComponentRecord(anchors):
-    """[otTables.Anchor, otTables.Anchor, ...] --> otTables.ComponentRecord"""
+    """[ot.Anchor, ot.Anchor, ...] --> ot.ComponentRecord"""
     if not anchors:
         return None
     self = ot.ComponentRecord()
@@ -153,7 +721,7 @@ def buildComponentRecord(anchors):
 
 
 def buildCursivePosSubtable(attach, glyphMap):
-    """{"alef": (entry, exit)} --> otTables.CursivePos"""
+    """{"alef": (entry, exit)} --> ot.CursivePos"""
     if not attach:
         return None
     self = ot.CursivePos()
@@ -171,7 +739,7 @@ def buildCursivePosSubtable(attach, glyphMap):
 
 
 def buildDevice(deltas):
-    """{8:+1, 10:-3, ...} --> otTables.Device"""
+    """{8:+1, 10:-3, ...} --> ot.Device"""
     if not deltas:
         return None
     self = ot.Device()
@@ -215,7 +783,7 @@ def buildLigatureAttach(components):
 
 
 def buildMarkArray(marks, glyphMap):
-    """{"acute": (markClass, otTables.Anchor)} --> otTables.MarkArray"""
+    """{"acute": (markClass, ot.Anchor)} --> ot.MarkArray"""
     self = ot.MarkArray()
     self.MarkRecord = []
     for mark in sorted(marks.keys(), key=glyphMap.__getitem__):
@@ -305,7 +873,7 @@ def buildMarkRecord(classID, anchor):
 
 
 def buildMark2Record(anchors):
-    """[otTables.Anchor, otTables.Anchor, ...] --> otTables.Mark2Record"""
+    """[ot.Anchor, ot.Anchor, ...] --> ot.Mark2Record"""
     self = ot.Mark2Record()
     self.Mark2Anchor = anchors
     return self
@@ -394,7 +962,7 @@ def buildPairPosGlyphsSubtable(pairs, glyphMap,
 
 
 def buildSinglePos(mapping, glyphMap):
-    """{"glyph": ValueRecord} --> [otTables.SinglePos*]"""
+    """{"glyph": ValueRecord} --> [ot.SinglePos*]"""
     result, handled = [], set()
     # In SinglePos format 1, the covered glyphs all share the same ValueRecord.
     # In format 2, each glyph has its own ValueRecord, but these records
@@ -448,7 +1016,7 @@ def buildSinglePos(mapping, glyphMap):
 
 
 def buildSinglePosSubtable(values, glyphMap):
-    """{glyphName: otBase.ValueRecord} --> otTables.SinglePos"""
+    """{glyphName: otBase.ValueRecord} --> ot.SinglePos"""
     self = ot.SinglePos()
     self.Coverage = buildCoverage(values.keys(), glyphMap)
     valueRecords = [values[g] for g in self.Coverage.glyphs]
@@ -493,7 +1061,7 @@ _DeviceTuple = namedtuple("_DeviceTuple", "DeltaFormat StartSize EndSize DeltaVa
 
 
 def _makeDeviceTuple(device):
-    """otTables.Device --> tuple, for making device tables unique"""
+    """ot.Device --> tuple, for making device tables unique"""
     return _DeviceTuple(
         device.DeltaFormat,
         device.StartSize,
@@ -522,7 +1090,7 @@ def buildValue(value):
 # GDEF
 
 def buildAttachList(attachPoints, glyphMap):
-    """{"glyphName": [4, 23]} --> otTables.AttachList, or None"""
+    """{"glyphName": [4, 23]} --> ot.AttachList, or None"""
     if not attachPoints:
         return None
     self = ot.AttachList()
@@ -534,7 +1102,7 @@ def buildAttachList(attachPoints, glyphMap):
 
 
 def buildAttachPoint(points):
-    """[4, 23, 41] --> otTables.AttachPoint"""
+    """[4, 23, 41] --> ot.AttachPoint"""
     if not points:
         return None
     self = ot.AttachPoint()
@@ -544,7 +1112,7 @@ def buildAttachPoint(points):
 
 
 def buildCaretValueForCoord(coord):
-    """500 --> otTables.CaretValue, format 1"""
+    """500 --> ot.CaretValue, format 1"""
     self = ot.CaretValue()
     self.Format = 1
     self.Coordinate = coord
@@ -552,7 +1120,7 @@ def buildCaretValueForCoord(coord):
 
 
 def buildCaretValueForPoint(point):
-    """4 --> otTables.CaretValue, format 2"""
+    """4 --> ot.CaretValue, format 2"""
     self = ot.CaretValue()
     self.Format = 2
     self.CaretValuePoint = point
@@ -560,7 +1128,7 @@ def buildCaretValueForPoint(point):
 
 
 def buildLigCaretList(coords, points, glyphMap):
-    """{"f_f_i":[300,600]}, {"c_t":[28]} --> otTables.LigCaretList, or None"""
+    """{"f_f_i":[300,600]}, {"c_t":[28]} --> ot.LigCaretList, or None"""
     glyphs = set(coords.keys()) if coords else set()
     if points:
         glyphs.update(points.keys())
@@ -576,7 +1144,7 @@ def buildLigCaretList(coords, points, glyphMap):
 
 
 def buildLigGlyph(coords, points):
-    """([500], [4]) --> otTables.LigGlyph; None for empty coords/points"""
+    """([500], [4]) --> ot.LigGlyph; None for empty coords/points"""
     carets = []
     if coords:
         carets.extend([buildCaretValueForCoord(c) for c in sorted(coords)])
@@ -591,7 +1159,7 @@ def buildLigGlyph(coords, points):
 
 
 def buildMarkGlyphSetsDef(markSets, glyphMap):
-    """[{"acute","grave"}, {"caron","grave"}] --> otTables.MarkGlyphSetsDef"""
+    """[{"acute","grave"}, {"caron","grave"}] --> ot.MarkGlyphSetsDef"""
     if not markSets:
         return None
     self = ot.MarkGlyphSetsDef()

Lib/fontTools/otlLib/error.py

@@ -0,0 +1,13 @@
+
+
+class OpenTypeLibError(Exception):
+    def __init__(self, message, location):
+        Exception.__init__(self, message)
+        self.location = location
+
+    def __str__(self):
+        message = Exception.__str__(self)
+        if self.location:
+            return f"{self.location}: {message}"
+        else:
+            return message

Tests/feaLib/ast_test.py

@@ -13,6 +13,12 @@ class AstTest(unittest.TestCase):
         statement = ast.ValueRecord(xPlacement=10, xAdvance=20)
         self.assertEqual(statement.asFea(), "<10 0 20 0>")
 
+    def test_non_object_location(self):
+        el = ast.Element(location=("file.fea", 1, 2))
+        self.assertEqual(el.location.file, "file.fea")
+        self.assertEqual(el.location.line, 1)
+        self.assertEqual(el.location.column, 2)
+
 
 if __name__ == "__main__":
     import sys

Tests/feaLib/builder_test.py

@@ -225,7 +225,7 @@ class BuilderTest(unittest.TestCase):
 
     def test_pairPos_redefinition_warning(self):
         # https://github.com/fonttools/fonttools/issues/1147
-        logger = logging.getLogger("fontTools.feaLib.builder")
+        logger = logging.getLogger("fontTools.otlLib.builder")
         with CapturingLogHandler(logger, "DEBUG") as captor:
             # the pair "yacute semicolon" is redefined in the enum pos
             font = self.build(
@@ -571,7 +571,7 @@ class BuilderTest(unittest.TestCase):
         assert "GSUB" in font
 
     def test_unsupported_subtable_break(self):
-        logger = logging.getLogger("fontTools.feaLib.builder")
+        logger = logging.getLogger("fontTools.otlLib.builder")
         with CapturingLogHandler(logger, level='WARNING') as captor:
             self.build(
                 "feature test {"
@@ -598,6 +598,26 @@ class BuilderTest(unittest.TestCase):
         self.assertIn("GSUB", font)
         self.assertNotIn("name", font)
 
+    def test_singlePos_multiplePositionsForSameGlyph(self):
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Already defined different position for glyph",
+            self.build,
+            "lookup foo {"
+            "    pos A -45; "
+            "    pos A 45; "
+            "} foo;")
+
+    def test_pairPos_enumRuleOverridenBySinglePair_DEBUG(self):
+        logger = logging.getLogger("fontTools.otlLib.builder")
+        with CapturingLogHandler(logger, "DEBUG") as captor:
+            self.build(
+                "feature test {"
+                "    enum pos A [V Y] -80;"
+                "    pos A V -75;"
+                "} test;")
+        captor.assertRegex('Already defined position for pair A V at')
+
 
 def generate_feature_file_test(name):
     return lambda self: self.check_feature_file(name)

Tests/feaLib/error_test.py

@@ -1,10 +1,11 @@
 from fontTools.feaLib.error import FeatureLibError
+from fontTools.feaLib.location import FeatureLibLocation
 import unittest
 
 
 class FeatureLibErrorTest(unittest.TestCase):
     def test_str(self):
-        err = FeatureLibError("Squeak!", ("foo.fea", 23, 42))
+        err = FeatureLibError("Squeak!", FeatureLibLocation("foo.fea", 23, 42))
         self.assertEqual(str(err), "foo.fea:23:42: Squeak!")
 
     def test_str_nolocation(self):

Tests/feaLib/lexer_test.py

@@ -107,7 +107,7 @@ class LexerTest(unittest.TestCase):
 
     def test_newline(self):
         def lines(s):
-            return [loc[1] for (_, _, loc) in Lexer(s, "test.fea")]
+            return [loc.line for (_, _, loc) in Lexer(s, "test.fea")]
         self.assertEqual(lines("FOO\n\nBAR\nBAZ"), [1, 3, 4])  # Unix
         self.assertEqual(lines("FOO\r\rBAR\rBAZ"), [1, 3, 4])  # Macintosh
         self.assertEqual(lines("FOO\r\n\r\n BAR\r\nBAZ"), [1, 3, 4])  # Windows
@@ -115,7 +115,7 @@ class LexerTest(unittest.TestCase):
 
     def test_location(self):
         def locs(s):
-            return ["%s:%d:%d" % loc for (_, _, loc) in Lexer(s, "test.fea")]
+            return [str(loc) for (_, _, loc) in Lexer(s, "test.fea")]
         self.assertEqual(locs("a b # Comment\n12 @x"), [
             "test.fea:1:1", "test.fea:1:3", "test.fea:1:5", "test.fea:2:1",
             "test.fea:2:4"
@@ -150,7 +150,7 @@ class IncludingLexerTest(unittest.TestCase):
 
     def test_include(self):
         lexer = IncludingLexer(self.getpath("include/include4.fea"))
-        result = ['%s %s:%d' % (token, os.path.split(loc[0])[1], loc[1])
+        result = ['%s %s:%d' % (token, os.path.split(loc.file)[1], loc.line)
                   for _, token, loc in lexer]
         self.assertEqual(result, [
             "I4a include4.fea:1",
@@ -186,7 +186,7 @@ class IncludingLexerTest(unittest.TestCase):
     def test_featurefilepath_None(self):
         lexer = IncludingLexer(UnicodeIO("# foobar"))
         self.assertIsNone(lexer.featurefilepath)
-        files = set(loc[0] for _, _, loc in lexer)
+        files = set(loc.file for _, _, loc in lexer)
         self.assertIn("<features>", files)
 
     def test_include_absolute_path(self):
@@ -199,7 +199,7 @@ class IncludingLexerTest(unittest.TestCase):
         including = UnicodeIO("include(%s);" % included.name)
         try:
             lexer = IncludingLexer(including)
-            files = set(loc[0] for _, _, loc in lexer)
+            files = set(loc.file for _, _, loc in lexer)
             self.assertIn(included.name, files)
         finally:
             os.remove(included.name)
@@ -225,7 +225,7 @@ class IncludingLexerTest(unittest.TestCase):
             # an in-memory stream, so it will use the current working
             # directory to resolve relative include statements
             lexer = IncludingLexer(UnicodeIO("include(included.fea);"))
-            files = set(os.path.realpath(loc[0]) for _, _, loc in lexer)
+            files = set(os.path.realpath(loc.file) for _, _, loc in lexer)
             expected = os.path.realpath(included.name)
             self.assertIn(expected, files)
         finally:

Tests/otlLib/mock_builder_test.py

@@ -0,0 +1,85 @@
+from fontTools.otlLib.builder import (
+    AlternateSubstBuilder,
+    ChainContextPosBuilder,
+    ChainContextSubstBuilder,
+    LigatureSubstBuilder,
+    MultipleSubstBuilder,
+    CursivePosBuilder,
+    MarkBasePosBuilder,
+    MarkLigPosBuilder,
+    MarkMarkPosBuilder,
+    ReverseChainSingleSubstBuilder,
+    SingleSubstBuilder,
+    ClassPairPosSubtableBuilder,
+    PairPosBuilder,
+    SinglePosBuilder,
+)
+from fontTools.otlLib.error import OpenTypeLibError
+from fontTools.ttLib import TTFont
+from fontTools.misc.loggingTools import CapturingLogHandler
+import logging
+import pytest
+
+
+@pytest.fixture
+def ttfont():
+    glyphs = """
+        .notdef space slash fraction semicolon period comma ampersand
+        quotedblleft quotedblright quoteleft quoteright
+        zero one two three four five six seven eight nine
+        zero.oldstyle one.oldstyle two.oldstyle three.oldstyle
+        four.oldstyle five.oldstyle six.oldstyle seven.oldstyle
+        eight.oldstyle nine.oldstyle onequarter onehalf threequarters
+        onesuperior twosuperior threesuperior ordfeminine ordmasculine
+        A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
+        a b c d e f g h i j k l m n o p q r s t u v w x y z
+        A.sc B.sc C.sc D.sc E.sc F.sc G.sc H.sc I.sc J.sc K.sc L.sc M.sc
+        N.sc O.sc P.sc Q.sc R.sc S.sc T.sc U.sc V.sc W.sc X.sc Y.sc Z.sc
+        A.alt1 A.alt2 A.alt3 B.alt1 B.alt2 B.alt3 C.alt1 C.alt2 C.alt3
+        a.alt1 a.alt2 a.alt3 a.end b.alt c.mid d.alt d.mid
+        e.begin e.mid e.end m.begin n.end s.end z.end
+        Eng Eng.alt1 Eng.alt2 Eng.alt3
+        A.swash B.swash C.swash D.swash E.swash F.swash G.swash H.swash
+        I.swash J.swash K.swash L.swash M.swash N.swash O.swash P.swash
+        Q.swash R.swash S.swash T.swash U.swash V.swash W.swash X.swash
+        Y.swash Z.swash
+        f_l c_h c_k c_s c_t f_f f_f_i f_f_l f_i o_f_f_i s_t f_i.begin
+        a_n_d T_h T_h.swash germandbls ydieresis yacute breve
+        grave acute dieresis macron circumflex cedilla umlaut ogonek caron
+        damma hamza sukun kasratan lam_meem_jeem noon.final noon.initial
+        by feature lookup sub table uni0327 uni0328 e.fina
+    """.split()
+    glyphs.extend("cid{:05d}".format(cid) for cid in range(800, 1001 + 1))
+    font = TTFont()
+    font.setGlyphOrder(glyphs)
+    return font
+
+
+class MockBuilderLocation(object):
+    def __init__(self, location):
+        self.location = location
+
+    def __str__(self):
+        return "%s:%s" % self.location
+
+
+def test_unsupported_subtable_break_1(ttfont):
+    location = MockBuilderLocation((0, "alpha"))
+
+    logger = logging.getLogger("fontTools.otlLib.builder")
+
+    with CapturingLogHandler(logger, "INFO") as captor:
+        builder = SinglePosBuilder(ttfont, location)
+        builder.add_subtable_break(MockBuilderLocation((5, "beta")))
+        builder.build()
+
+    captor.assertRegex('5:beta: unsupported "subtable" statement for lookup type')
+
+def test_chain_pos_references_GSUB_lookup(ttfont):
+    location = MockBuilderLocation((0, "alpha"))
+    builder = ChainContextPosBuilder(ttfont, location)
+    builder2 = SingleSubstBuilder(ttfont, location)
+    builder.rules.append(([], [], [], [[builder2]]))
+
+    with pytest.raises(OpenTypeLibError, match="0:alpha: Missing index of the specified lookup, might be a substitution lookup"):
+        builder.build()