Closed alxkzmn closed 2 weeks ago
I see that some tests are failing due to the new usages
field in the SymTableEntry, but I'd rather fix them only if the changes make it through the review 😃
so, now the changes to the analyser are missing?
@leolara maybe I misunderstood but I thought that the conclusion was to extract the usages in a separate pass and put them in a separate data structure. In this case it could all be done in the language server.
@alxkzmn That is not the conclusion: https://github.com/privacy-scaling-explorations/chiquito/pull/254#discussion_r1625953580
The conclusion is to remove the lines that check if the state symbol is there and create it if it is not. The state symbol will be there.
@leolara to make sure I'm on the right track:
Yes, single pass, you don't need a separate data structure. What you had before is ok, just needs to assume that the states are already in the symbol table, because they should be. As I explained, we are already doing kind of two passes, one to collect the state symbols and then to analyse each state after. So when you are analysing a transition you already should have the state in the symbol table
@leolara the PR is ready for review.
I put all my ideas here together to avoid more cycles:
find_symbol_by_offset
I think the if can be simplified by putting an and
of the first two conditions. (https://github.com/privacy-scaling-explorations/chiquito/pull/254/files#diff-feef6fedaf732a222ce51a0e815b9afe9d0386c1099a96d97cd3b6dca7ba8f60R374-R376)check_usage_at
the code can be simplified and more stateless by returning only the "closest" of the collected symbols. This can be done without a map. Keeping the closest found until now in a variable.find_symbol_by_offset
as well the code can be simplified by using a variable to keep the closest found until now, and don't use a map.extract_usages_recursively
is better rename to extract_usages_expression
and be called after the analysis like in the statements. This is more coherent with the current naming.extract_usages_expression
could call collect_id_usages([id])
but perhaps this is too nit pick.It was aprove already, you can merge
The following changes were necessary to integrate with the language server:
SymTableEntry
(and some WIP additional functionality in the Analyzer to record the usages during the analysis).SymTable