Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Deleted flags for scopes, symbols, references #5097

Open
overlookmotel opened this issue Aug 22, 2024 · 4 comments
Open

Add Deleted flags for scopes, symbols, references #5097

overlookmotel opened this issue Aug 22, 2024 · 4 comments
Assignees
Labels
A-semantic Area - Semantic

Comments

@overlookmotel
Copy link
Collaborator

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.

@overlookmotel overlookmotel added the A-semantic Area - Semantic label Aug 22, 2024
@Boshen
Copy link
Member

Boshen commented Aug 22, 2024

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
Copy link
Collaborator Author

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
Copy link
Member

Boshen commented Aug 23, 2024

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

@Boshen Boshen assigned Boshen and overlookmotel and unassigned Boshen Aug 23, 2024
@overlookmotel
Copy link
Collaborator Author

overlookmotel commented Aug 23, 2024

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.

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

No branches or pull requests

2 participants