search

Nadrieril / dhall-rust

Maintainable configuration files, for Rust users
Other
303 stars 27 forks source link

Refactor import resolution to use a global store #204

Closed Nadrieril closed 3 years ago

Nadrieril commented 3 years ago

This is the first step towards concurrent/async import resolution, as needed for wasm support but also more generally would be cool. This is an intense PR. The idea is that instead of storing the result of an import in the Hir directly, instead we store and index into a global store. Which means I had to make the global store accessible to any code that would need to read an import. I ended up adding a lot of lifetimes x). So now import resolution runs in two phases: first traverse the ast and replace the imports by their indices, and then go through the imports and resolve them. By making that second step smaller, we'll make it possible to add async and to batch imports.

I got excited with the global store. There's a few more cool things I want to do with it. I hope this is not making the code too much more complicated. Among other things, once non-import errors don't count for import alternatives (https://github.com/dhall-lang/dhall-lang/issues/1113), we can decouple entirely import resolution from the rest of the code. That would not have been possible before this PR without introducing lots of duplicate work.

cc @basile-henry

Nadrieril commented 3 years ago

Yeah, I considered the global mutable store but it made me sad :(. The pain of lifetimes is mostly adding them, but once they're there they're not too painful I find. Typechecking will still be on a per-file basis, that's not an issue. What dhall-lang/dhall-lang#1113 would give me is that none of the resolve code even needs to know that such a thing as typechecking and normalization exist. Which would be awesome. Parallel typechecking would also be possible if we want, whereas currently it would have to be entangled with import fetching and alternatives decition-making.

Nadrieril commented 3 years ago

EDIt: I confused myself, for a moment I thought there were no dependencies between different files when we typecheck :( The point I wanted to make still stands though: trying to schedule parallelism for both typechecking and imports sounds harder than first doing imports and second doing typechecking (and third normalization). For example imports benefit from being batched and async but typechecking doesn't.

Nadrieril commented 3 years ago

Eh, I want to run with this and see where it leads. I think we really want some sort of store, we can always make it static later if the lifetimes-based one is too painful