First pass at usage refactor

[Acepie]

First attempt to swap out the old name based usage detection. Every new variable declaration is given a unique id and stored. Every variable get stores the location of the get. Anything that has no get locations is unused.

Closes #3552

This gets us a bit closer what we ultimately need for references and renames but there would still be some more plumbing required. We'd probably want to attach the unique id of the variable to both the variable declaration and the variable usage to make the look ups from ast node to scope variable easy. Also, we'd still have to update the module_interface to make cross module references work

[lpil]

Sorry, what's this change? This system is going to be replaced with a conventional call graph traversal so I don't think we should spend more time on it.

[Acepie]

For a conventional call graph traversal, do you explicitly want to reuse the CallGraphBuilder, add new node variants for local variables, record constructors, and return/public sink nodes, switch from using names to unique identifiers, and then do dead code analysis?

[lpil]

It would be good to reuse it, though I'm not sure how you would distinguish between module and record access as that's pre-analysis. Perhaps you'd store a collection of ambiguous edges and update them during analysis.

Why would you need to change names?

[Acepie]

Yeah I think ideally we would want information from after/during the rest of analysis which is why I was focused on doing something inline with the existing system. Maybe it would make more sense to use the same overall graph data structure stored on the environment but a separate instance with a different node enum?

Using just names causes issues with shadowing since we need all local variable instances across the module and not scope specific instances if we wanna reuse this for references.

[lpil]

I'm not following what you mean about names still, sorry. I was thinking the graph would identify source locations. Is there anything else we'd need?

[Acepie]

Yeah the nodes would still be exprs that include the locations (as well as I think the edges would be tagged with the usage locations). I guess if we reuse that graph structure then the name->node-id map accomplishes the same thing as long as we change how the start node is set instead of that current_function

[lpil]

Sorry, I'm having a slow day. When you say store exprs, do you mean store more than just the source locations?

[Acepie]

We probably still wanna store the actual type in some way so we can easily display the right unused message right? In this pr I had used the value constructor since that had enough info/was convenient

[lpil]

I think we want to keep the graph as small as possible and avoid duplicating information.

I think I would like to move over to a flat array AST rather than a graph one and then do bisection searches on it, but that seems like an unrealistic amount of work.

Having as little as practical in the graph and then doing AST traversals for more information when needed could be good. Do we have uses other than unused code detection, completion, and renaming presently?

[Acepie]

That's fair. We can make down the actual stored data so that we just serialize the "graph" but as long as we can map from the graph indices to the nodes easily. A flat search would work once we had that but until then I was thinking to attach node I'd to the ast in some way

Yeah I think the main usages of the graph would be

1. Dead code elimination
2. Rename/refactor/reference

For 1 we really just need the existence of the edges and the publicity/variant so we can know if something has no edges and is not public + what warning variant to show

For 2 we need the actual locations on the edges and a way to map cursor position/found expr to node index

[lpil]

Both 1 and 2 seem like srcspans to me? As in once you have them you have enough to get anything else from the AST, and caches could be added later if we decide the traversal is impractical or expensive for any particular operation.

[Acepie]

Yeah we can start with just recording the src spans and see how the perf is after

[Acepie]

I think I can probably rework what I wrote here to match what we want and use the graph crate. Idt I'll have time today but I'll try and take a look

[Acepie]

Hey, wanted to post an update on this at least since idt I'm gonna have time to work on this again anytime soon. I managed to get an initial PoC going that shows how unused values can be chained up using the stable graph so that if a value is only used by other unused values it is also considered unused. There are a couple notable gaps/points to improve on this

1. Right now local variables that are used in assignments are considered referenced by their parent function and not by the actual assignment. This means that you can't actually chain the unused locals. Making this work would require preregistering the pattern variables prior to type unification. I put a todo for where this would go
2. I didn't add any logic for pulling out unused cycles. Probably need to write something similar to the leaf_or_cycle logic in the src/graph module to pull out each leaf or cycle rather than using the externals method from the graph library itself
3. I still haven't converted types to use the same mechanism though I think its "mostly" the same idea?
4. Storing just the src locations in the environment made creating the unused warnings really annoying so I think it might make more sense to store the value constructors in the environment but map the constructors to just the src locations when we actually convert and attach to the module interface

[lpil]

Hello! Are you still working on this one? Thanks

[Acepie]

Hey Louis, I haven't gotten around to this and probably won't any time in the near future. Could be useful to have as reference for somebody who wants to continue this/work on a call graph mechanism but you can close this if you want