Conversation
rsheeter
left a comment
There was a problem hiding this comment.
LGTM subject to confirmation --emit-ir still works, that is a useful capability. If it breaks it ... would have to think just a little about whether thats desirable.
|
so this does break That said, it feels like My preferred way of doing that would probably be to add some new method to the Is this an acceptable compromise? I don't want to spend too much more time on this, but also don't want to break anyone's workflow. Also fwiw the followup work I've done (enabled by this refactor) is genuinely useful to me. (It's about adding a new mechanism for debug-printing types while resolving GlyphId to names, because it's really hard to figure out what's going on in places like kerning when all we have access to is glyph ids) So my preferred solution, at this point:
|
This is a relic from back when we were thinking about incremental compilation, and has been causing me headaches recently. I was curious how hard it would be to remove and it turns out to not be that hard?
The specific motivation here is that I have twice this week wanted to move
GlyphOrderintofontdrasilbut have been prevented because of the implementation ofPersistablefor that type; the trait is defined infontir, which also has a blanket impl for write-fonts types, and that means I can't have another custom impl for a foreign type, and the solution would require either moving the whole trait intofontdrasil, having some ugly wrapper type, or having fontir be a dep of fontdrasil, which feels backwards.In any case, this was actually quite clean, and might unlock additional cleanups.