fix(isolated_declarations): fix potential memory leak (#6622)

`Scope` contains 2 x `FxHashMap`s, which own data outside of the arena. So this data will not be dropped when the allocator is dropped.

The scope for this becoming a memory leak in practice is limited for 2 reasons:

1. All `Scope`s except the root one are popped from the stack by the end of traversal. That last scope's hashmaps are always empty, unless there are unresolved references (references to globals).

2. `oxc_allocator::Vec` is currently `Drop`.

However, `oxc_allocator::Vec` will cease to be `Drop` in future, at which point this would become a real memory leak.

Additionally, it doesn't make sense to store temporary data in the arena, as the arena is intended to hold data that needs to live as long as the AST, which temporary data doesn't.
This commit is contained in:
overlookmotel 2024-10-16 15:05:54 +00:00
parent 3af08406b8
commit 2673397122
2 changed files with 6 additions and 8 deletions

View file

@ -71,7 +71,7 @@ impl<'a> IsolatedDeclarations<'a> {
ast: AstBuilder::new(allocator),
strip_internal,
internal_annotations: FxHashSet::default(),
scope: ScopeTree::new(allocator),
scope: ScopeTree::new(),
errors: RefCell::new(vec![]),
}
}

View file

@ -1,15 +1,14 @@
use std::cell::Cell;
use bitflags::bitflags;
use oxc_allocator::{Allocator, Vec};
use rustc_hash::FxHashMap;
#[allow(clippy::wildcard_imports)]
use oxc_ast::ast::*;
use oxc_ast::AstBuilder;
#[allow(clippy::wildcard_imports)]
use oxc_ast::{visit::walk::*, Visit};
use oxc_span::Atom;
use oxc_syntax::scope::{ScopeFlags, ScopeId};
use rustc_hash::FxHashMap;
bitflags! {
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@ -37,13 +36,12 @@ impl<'a> Scope<'a> {
/// Linear tree of declaration scopes.
#[derive(Debug)]
pub struct ScopeTree<'a> {
levels: Vec<'a, Scope<'a>>,
levels: Vec<Scope<'a>>,
}
impl<'a> ScopeTree<'a> {
pub fn new(allocator: &'a Allocator) -> Self {
let ast = AstBuilder::new(allocator);
let levels = ast.vec1(Scope::new(ScopeFlags::Top));
pub fn new() -> Self {
let levels = vec![Scope::new(ScopeFlags::Top)];
Self { levels }
}