oxc-project / oxc

⚓ A collection of JavaScript tools written in Rust.
https://oxc.rs
MIT License
11.52k stars 417 forks source link

Add `Deleted` flags for scopes, symbols, references #5097

Open overlookmotel opened 1 month ago

overlookmotel commented 1 month ago

In the transformer, we often delete scopes, symbols and references. But it's not possible to fully delete them from ScopeTree and SymbolTable, as cannot "shuffle up" entries, as it would invalidate IDs of later scopes/symbols/references.

Instead we can signal that a scope is deleted by setting its flags to ScopeFlags::Deleted. Or, probably better, use an absence of any flags (ScopeFlags::empty()) to signify a deleted scope. Introduce a ScopeFlags::Block flag which will cover the scopes which can currently be ScopeFlags::empty().

Same thing for SymbolFlags and ReferenceFlags.

Boshen commented 1 month ago

I see the SoA structure as database columns, a soft delete is another is_deleted column.

All the getter becomes SELECT * from symbols where is_deleted = true.

overlookmotel commented 1 month ago

I like that analogy. But don't you think better to re-use the existing flags columns, as they have spare bits, and often you'd be reading the flags anyway and so will have it in cache?

SELECT * from symbols where flags = 'Deleted'

but also when you're e.g. looking for functions:

SELECT * from symbols where flags = 'FunctionScopedVariable'

instead of:

SELECT * from symbols where flags = 'FunctionScopedVariable' AND is_deleted = false

Boshen commented 1 month ago

This may get complicated as all reads now need to access the deleted field, . Let's add this in multiple phases.

overlookmotel commented 1 month ago

I don't think it'll be so bad. If you get the SymbolId from a BindingIdentifier (binding_ident.symbol_id) then there's no need to check if the symbol is deleted - you know it's not deleted as you just read it from the AST. Ditto if you get the SymbolId from Bindings - because we remove bindings when we delete symbols.

You only need to check if the flag says "deleted" if you are looping through all symbols in SymbolTable. We don't often have reason to do that.