Implementation annoyances

[brendanzab]

This is a bit of a brain dump of things that I’m currently finding annoying and frustrating in the implementation of Fathom:

• self.scope.to_scope and self.scope.to_scope_from_iter are very noisy and make elaboration, quotation, etc. difficult to read.
• self.eval_env().eval(...), self.quote_env().quote(...) etc. is rather noisy
• can’t allocate AST nodes when contexts are borrowed mutably, meaning we need to split things up over multiple lines
• can’t report errors while and pretty printing terms/values in the same expression sure to multiple mutable borrows of the context
• pushing and popping contexts and environments adds lots of noise and is easy to mess up
• ranges vs. file ranges vs. spans - it’s annoying to have to convert between these
• it’s hard to match on primitives and their arguments
• initialising the types of primitives is verbose and error prone
• initialisation of primitive evaluation functions is clunky

The combined effect of these means that it is harder to implement new features and to experiment with new ideas, enough for me to ponder if it’s worth making a reference implementation that is aggressively focused on implementation clarity over user experience (perhaps in a higher level language like OCaml).

Some potential ideas for improvements (which may or may not help):

• delegate allocator and evaluation/quotation functions in the contexts that need them
• experiment with using immutable environments vs. pushing and popping environments (not sure about the performance impact of this - as the elaboration environment is rather large)
• when pushing to an environment, take a closure that then handles resetting the environment back to its original state
• pass the allocator Scope as a separate argument
• use allocators that use IDs vs. pointers
• move spans into a side-table that can be referred to by ID
• go back to using Boxes and and Vecs in surface and core syntaxes
• store spans in the surface syntax, and refer back to original surface nodes by pointer/ID in terms/values
• avoid the need to mutably access the list of diagnostics when reporting errors – perhaps by using shared memory
• investigate a “term builder” for defining the types of primitives like in https://gist.github.com/TOTBWF/9b2c071d2edb1c6596b785656c866fd6#file-micrott-ml-L133-L210 (seems challenging to implement this in Rust, however)

We might need to experiment with some of these outside of Fathom before committing to anything. Looking at how other Rust projects handle some of these issues might also be interesting.

[Kmeakin]

I would recommend against using integer IDs instead of pointers:

• Have to pass the arena to any code that wants to dereference an ID
• fmt::Debug output is less readable because subexpressions are just printed as an opaque ID
• Instead of a single scoped_arena::Scope, would need a separate arena for each type being allocated
• Introduces runtime bounds checks when de-referencing
• Possible to index an arena with an ID that was allocated from a different area (eg if each module has its own arena, would be possible to index module 1 with a TermId from module 2)

I think rust-analyzer uses IDs over pointers because they want to have side tables and caches indexed by IDs. But should be possible to do the same thing with pointers, by using a wrapper struct that implements Eq/Hash based on the address of the pointer, rather than the contents of the underlying type.