Skip to content

Conversation

weswigham
Copy link
Member

Heavily WIP with a bunch still TODO - I've spent a lot of time disentangling the circular host structure ID previously used that really doesn't port easily to go and inlining whatever unneeded abstractions I can. In the service of that, there is one new abstraction to decouple ID node lookup logic from the actual node/error generation - the psuedochecker (which, true to its' name, calculates psuedotypes), which mostly corresponds to the node lookup logic in expressionToTypeNode.ts without a lot of the intermingled error handling and node construction. This way the ID type node lookup logic is still "known checker-free", while avoiding trying to dip into the checker/node builder in it.

The primary thing I'm still working on is the replacement for (*nodeBuilderImpl).reuseNode that actually validates that the node is reusable (and triggers the node builder recovery scope otherwise), but this is a non-crashy checkpoint. It can't be merged as a WIP because without the reuseNode improvements, it's a fairly big step backwards in terms of output alignment (the ID logic very aggressively tries to reuse a ton of nodes that need some massaging to work at a different location, resulting in some pretty nonfunctional declarations (lots of uninstantiated Ts copied without regard for correctness)). I also need to add the sanity-checking that used to be (sometimes) done by the canReuseTypeNode chain of host calls (which should now just be a check in a single branch of the new psudoTypeToNode, ensuring it's more reliably checked).

The other-other thing that I really should do as part of this is reenable the node reuse baselines (to track where we do better/worse/different than strada); but that's probably better served as a followup PR, since that will add underlines to every diff in the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant