Simplify binder flow.#2832
Conversation
We now only recurse in a single place in the binder. The rest of the binding code is only concerned with how to bind a single node to a symbol and add that symbol to a symbol table. Recursion is handled as a separate concern, greatly simplifying binder flow.
|
Is this a general cleanup work or this needed for other work items? |
|
This greatly simplifies the work to do things like the JSDoc work Jason and I were looking at last week. The problem right now is that the binder is so complex that it makes any work in it much more expensive than necessary. A major difficulty is reasoning about flow in the binder, and which concerns are best handled where. This PR attempts to codify certain of those concerns, while also making the other parts operate in a more consistent manner. So it's both general cleanup, but also motivated specifically by recent work that has been hampered with the current state of the binder. 'Needed' is somewhat more difficult to answer. The entirety of this change is not absolutely needed (though portions were needed in my JSDoc branch). But it made a lot of sense to just address many of the problems we were running into when trying ot reason about things, and simplify/unify them while i was working in this code anyways. |
|
@CyrusNajmabadi i would not want to take a change to the core machinery of the binder at this point in the release cycle. so unless this is really blocking other feature work, i would not take it. |
|
I'm also thinking another enum would be better. It would be called something like BinderRecursionFlags to better signify what you're supposed to return and what impact it will have. |
|
👍 |
CR notes: I recommend reading this CR one commit at a time to better understand what's going on.
Previously, it was very difficult to reason about the flow of the binder. Different parts of the binder recursed in different ways. For example, some codepaths called
bindChildren. Others calledforEachChild(node, bind). Some codepaths did not recurse, but were called from paths that woudl then laterbreakout and recurse. This made it very easy to end up doing something wrong if you accidentallyreturned early. It was also very difficult to know if you should should recurse if you were inbindorbindDeclarationorbindAnonymousDeclaration. The answer was always "it depends".I've gone in and completely unified the recursion logic in the binder. The top level logic looks like this now:
That's it. Individual binding functions never need to worry about recursing. Instead, they just deal with the node they have, and they top level bind driver will always be responsible for recursing. This makes things much easier to reason about and provides a consistent framework for future binding code to follow.