oxc-project / backlog

backlog for collborators only
1 stars 0 forks source link

Clone in scope API #127

Open overlookmotel opened 3 months ago

overlookmotel commented 3 months ago

oxc-project/oxc#4732 introduced CloneIn trait.

Currently it clones SymbolId, ScopeId and ReferenceId along with all other parts of AST being cloned.

I don't think this is correct behavior, but am not sure what the correct behavior should be.

If you clone part of an AST which has been through Semantic, and insert it elsewhere in same AST, that AST will now contain duplicate IDs.

If you insert it into another AST, the IDs will be invalid. It can cause panic if try to look up the symbol for a SymbolId, if the destination AST has less symbols than the source AST, so SymbolId is out of bounds. And if it doesn't panic, it'll be the wrong symbol.

Ideally we should provide another API where you provide the destination AST's ScopeTree and SymbolTable, and what scope the cloned AST fragment will be inserted into, and then it can create new scopes/symbols/references, and patch up all the IDs in the clone automatically for you.

trait CloneInScope<'a>: Sized {
    type Cloned;
    fn clone_in_scope(
        &self,
        parent_scope_id: ScopeId,
        alloc: &'a Allocator,
        scopes: &mut ScopeTree,
        symbols: &mut SymbolTable,
    ) -> Self::Cloned;
}

But in meantime, to avoid invalid IDs, perhaps CloneIn should set scope_id, symbol_id and reference_id fields to None?