Skip to content

Commit

Permalink
refactor(rust): remove AstSymbols and use SymbolTable directly (r…
Browse files Browse the repository at this point in the history
…olldown#2381)

<!-- Thank you for contributing! -->

### Description

<!-- Please insert your description here and provide especially info
about the "what" this PR is solving -->
  • Loading branch information
hyf0 authored Oct 7, 2024
1 parent d8b07cc commit 167f46a
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 87 deletions.
69 changes: 50 additions & 19 deletions crates/rolldown/src/ast_scanner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pub mod side_effect_detector;
use arcstr::ArcStr;
use oxc::ast::ast;
use oxc::index::IndexVec;
use oxc::semantic::{NodeId, SymbolFlags, SymbolTable};
use oxc::span::SPAN;
use oxc::{
ast::{
ast::{
Expand All @@ -30,8 +32,6 @@ use sugar_path::SugarPath;

use crate::types::symbol_ref_db::SymbolRefDbForModule;

use super::types::ast_symbols::AstSymbols;

#[derive(Debug)]
pub struct ScanResult {
pub named_imports: FxHashMap<SymbolRef, NamedImport>,
Expand All @@ -56,7 +56,7 @@ pub struct AstScanner<'me> {
file_path: &'me ModuleId,
scopes: &'me AstScopes,
trivias: &'me Trivias,
symbols: &'me mut AstSymbols,
symbol_table: SymbolTable,
current_stmt_info: StmtInfo,
result: ScanResult,
esm_export_keyword: Option<Span>,
Expand All @@ -79,7 +79,7 @@ impl<'me> AstScanner<'me> {
pub fn new(
idx: ModuleIdx,
scope: &'me AstScopes,
symbols: &'me mut AstSymbols,
mut symbol_table: SymbolTable,
repr_name: &'me str,
module_type: ModuleDefFormat,
source: &'me ArcStr,
Expand All @@ -88,12 +88,26 @@ impl<'me> AstScanner<'me> {
) -> Self {
// This is used for converting "export default foo;" => "var default_symbol = foo;"
let legitimized_repr_name = legitimize_identifier_name(repr_name);
let symbol_id_for_default_export_ref = symbols
.create_symbol(format!("{legitimized_repr_name}_default").into(), scope.root_scope_id());
let symbol_id_for_default_export_ref = symbol_table.create_symbol(
SPAN,
format!("{legitimized_repr_name}_default").into(),
SymbolFlags::empty(),
scope.root_scope_id(),
NodeId::DUMMY,
);

let name = format!("{legitimized_repr_name}_exports");
let namespace_object_ref: SymbolRef =
(idx, symbols.create_symbol(name.into(), scope.root_scope_id())).into();
let namespace_object_ref: SymbolRef = (
idx,
symbol_table.create_symbol(
SPAN,
name.into(),
SymbolFlags::empty(),
scope.root_scope_id(),
NodeId::DUMMY,
),
)
.into();

let result = ScanResult {
named_imports: FxHashMap::default(),
Expand All @@ -119,7 +133,7 @@ impl<'me> AstScanner<'me> {
Self {
idx,
scopes: scope,
symbols,
symbol_table,
current_stmt_info: StmtInfo::default(),
result,
esm_export_keyword: None,
Expand Down Expand Up @@ -196,7 +210,7 @@ impl<'me> AstScanner<'me> {
.collect::<FxHashSet<_>>();
for (name, symbol_id) in self.scopes.get_bindings(self.scopes.root_scope_id()) {
let symbol_ref: SymbolRef = (self.idx, *symbol_id).into();
let scope_id = self.symbols.scope_id_for(*symbol_id);
let scope_id = self.symbol_table.get_scope_id(*symbol_id);
if !scanned_top_level_symbols.remove(&symbol_ref) {
return Err(anyhow::format_err!(
"Symbol ({name:?}, {symbol_id:?}, {scope_id:?}) is declared in the top-level scope but doesn't get scanned by the scanner",
Expand All @@ -210,6 +224,7 @@ impl<'me> AstScanner<'me> {
// }
}
self.result.ast_usage = self.ast_usage;
self.result.symbol_ref_db.fill_classic_data(self.symbol_table);
Ok(self.result)
}

Expand All @@ -236,13 +251,16 @@ impl<'me> AstScanner<'me> {
// just create the symbol. If the symbol is finally used would be determined in the linking stage.
let namespace_ref: SymbolRef = (
self.idx,
self.symbols.create_symbol(
self.symbol_table.create_symbol(
SPAN,
format!(
"#LOCAL_NAMESPACE_IN_{}#",
itoa::Buffer::new().format(self.current_stmt_info.stmt_idx.unwrap_or_default())
)
.into(),
SymbolFlags::empty(),
self.scopes.root_scope_id(),
NodeId::DUMMY,
),
)
.into();
Expand Down Expand Up @@ -332,7 +350,8 @@ impl<'me> AstScanner<'me> {
// We will pretend `export { [imported] as [export_name] }` to be `import `
let generated_imported_as_ref = (
self.idx,
self.symbols.create_symbol(
self.symbol_table.create_symbol(
SPAN,
if export_name == "default" {
let importee_repr = self.result.import_records[record_id]
.module_request
Expand All @@ -343,7 +362,9 @@ impl<'me> AstScanner<'me> {
} else {
export_name.into()
},
SymbolFlags::empty(),
self.scopes.root_scope_id(),
NodeId::DUMMY,
),
)
.into();
Expand Down Expand Up @@ -371,9 +392,17 @@ impl<'me> AstScanner<'me> {
record_id: ImportRecordIdx,
span_for_export_name: Span,
) {
let generated_imported_as_ref =
(self.idx, self.symbols.create_symbol(export_name.into(), self.scopes.root_scope_id()))
.into();
let generated_imported_as_ref = (
self.idx,
self.symbol_table.create_symbol(
SPAN,
export_name.into(),
SymbolFlags::empty(),
self.scopes.root_scope_id(),
NodeId::DUMMY,
),
)
.into();
self.current_stmt_info.declared_symbols.push(generated_imported_as_ref);
let name_import = NamedImport {
imported: Specifier::Star,
Expand Down Expand Up @@ -568,20 +597,22 @@ impl<'me> AstScanner<'me> {
}

fn is_top_level(&self, symbol_id: SymbolId) -> bool {
self.scopes.root_scope_id() == self.symbols.scope_id_for(symbol_id)
self.scopes.root_scope_id() == self.symbol_table.get_scope_id(symbol_id)
}

fn try_diagnostic_forbid_const_assign(&mut self, id_ref: &IdentifierReference) {
match (self.resolve_symbol_from_reference(id_ref), id_ref.reference_id.get()) {
(Some(symbol_id), Some(ref_id)) if self.symbols.get_flag(symbol_id).is_const_variable() => {
(Some(symbol_id), Some(ref_id))
if self.symbol_table.get_flags(symbol_id).is_const_variable() =>
{
let reference = &self.scopes.references[ref_id];
if reference.is_write() {
self.result.warnings.push(
BuildDiagnostic::forbid_const_assign(
self.file_path.to_string(),
self.source.clone(),
self.symbols.get_name(symbol_id).into(),
self.symbols.get_span(symbol_id),
self.symbol_table.get_name(symbol_id).into(),
self.symbol_table.get_span(symbol_id),
id_ref.span(),
)
.with_severity_warning(),
Expand Down
15 changes: 6 additions & 9 deletions crates/rolldown/src/ecmascript/ecma_module_view_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use sugar_path::SugarPath;
use crate::{
ast_scanner::{AstScanner, ScanResult},
types::{
ast_symbols::AstSymbols,
module_factory::{CreateModuleContext, CreateModuleViewArgs},
symbol_ref_db::SymbolRefDbForModule,
},
Expand All @@ -33,16 +32,16 @@ fn scan_ast(
symbols: SymbolTable,
scopes: ScopeTree,
module_def_format: ModuleDefFormat,
) -> UnhandleableResult<(AstScopes, ScanResult, AstSymbols, SymbolRef)> {
let (mut ast_symbols, ast_scopes) = make_ast_scopes_and_symbols(symbols, scopes);
) -> UnhandleableResult<(AstScopes, ScanResult, SymbolRef)> {
let (symbol_table, ast_scopes) = make_ast_scopes_and_symbols(symbols, scopes);
let module_id = ModuleId::new(ArcStr::clone(id));
let repr_name = module_id.as_path().representative_file_name();
let repr_name = legitimize_identifier_name(&repr_name);

let scanner = AstScanner::new(
module_idx,
&ast_scopes,
&mut ast_symbols,
symbol_table,
&repr_name,
module_def_format,
ast.source(),
Expand All @@ -52,7 +51,7 @@ fn scan_ast(
let namespace_object_ref = scanner.namespace_object_ref;
let scan_result = scanner.scan(ast.program())?;

Ok((ast_scopes, scan_result, ast_symbols, namespace_object_ref))
Ok((ast_scopes, scan_result, namespace_object_ref))
}
pub struct CreateEcmaViewReturn {
pub view: EcmaView,
Expand Down Expand Up @@ -87,7 +86,7 @@ pub async fn create_ecma_view<'any>(
}
};

let (scope, scan_result, ast_symbol, namespace_object_ref) = scan_ast(
let (scope, scan_result, namespace_object_ref) = scan_ast(
ctx.module_index,
&ctx.resolved_id.id,
&mut ast,
Expand Down Expand Up @@ -116,7 +115,7 @@ pub async fn create_ecma_view<'any>(
has_eval,
errors,
ast_usage,
mut symbol_ref_db,
symbol_ref_db,
} = scan_result;
if !errors.is_empty() {
return Ok(Err(errors));
Expand Down Expand Up @@ -201,8 +200,6 @@ pub async fn create_ecma_view<'any>(
ast_usage,
};

symbol_ref_db.fill_classic_data(ast_symbol);

Ok(Ok(CreateEcmaViewReturn {
view,
resolved_deps,
Expand Down
15 changes: 5 additions & 10 deletions crates/rolldown/src/module_loader/runtime_module_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use super::Msg;
use crate::{
ast_scanner::{AstScanner, ScanResult},
runtime::{RuntimeModuleBrief, RUNTIME_MODULE_ID},
types::{ast_symbols::AstSymbols, symbol_ref_db::SymbolRefDbForModule},
types::symbol_ref_db::SymbolRefDbForModule,
utils::tweak_ast_for_scanning::tweak_ast_for_scanning,
};
pub struct RuntimeModuleTask {
Expand All @@ -33,7 +33,6 @@ pub struct MakeEcmaAstResult {
ast: EcmaAst,
ast_scope: AstScopes,
scan_result: ScanResult,
ast_symbols: AstSymbols,
namespace_object_ref: SymbolRef,
}

Expand All @@ -56,8 +55,7 @@ impl RuntimeModuleTask {
}
};

let MakeEcmaAstResult { ast, ast_scope, scan_result, ast_symbols, namespace_object_ref } =
ecma_ast_result;
let MakeEcmaAstResult { ast, ast_scope, scan_result, namespace_object_ref } = ecma_ast_result;

let runtime = RuntimeModuleBrief::new(self.module_id, &ast_scope);

Expand All @@ -74,11 +72,9 @@ impl RuntimeModuleTask {
has_eval,
errors: _,
ast_usage,
mut symbol_ref_db,
symbol_ref_db,
} = scan_result;

symbol_ref_db.fill_classic_data(ast_symbols);

let module = NormalModule {
idx: self.module_id,
repr_name: "rolldown_runtime".to_string(),
Expand Down Expand Up @@ -155,12 +151,11 @@ impl RuntimeModuleTask {
std::mem::take(&mut symbol_table.references),
std::mem::take(&mut symbol_table.resolved_references),
);
let mut ast_symbols = AstSymbols::from_symbol_table(symbol_table);
let facade_path = ModuleId::new("runtime");
let scanner = AstScanner::new(
self.module_id,
&ast_scope,
&mut ast_symbols,
symbol_table,
"rolldown_runtime",
ModuleDefFormat::EsmMjs,
source,
Expand All @@ -170,6 +165,6 @@ impl RuntimeModuleTask {
let namespace_object_ref = scanner.namespace_object_ref;
let scan_result = scanner.scan(ast.program())?;

Ok(Ok(MakeEcmaAstResult { ast, ast_scope, scan_result, ast_symbols, namespace_object_ref }))
Ok(Ok(MakeEcmaAstResult { ast, ast_scope, scan_result, namespace_object_ref }))
}
}
41 changes: 0 additions & 41 deletions crates/rolldown/src/types/ast_symbols.rs

This file was deleted.

1 change: 0 additions & 1 deletion crates/rolldown/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// and enums do not have complex logic, and are used to store data. They are not used to perform any
// operations on the data they store or only have simple getters and setters.

pub mod ast_symbols;
pub mod bundle_output;
pub mod bundler_fs;
pub mod generator;
Expand Down
5 changes: 3 additions & 2 deletions crates/rolldown/src/types/symbol_ref_db.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use oxc::index::IndexVec;
use oxc::semantic::SymbolTable;
use oxc::{semantic::SymbolId, span::CompactStr as CompactString};
use rolldown_common::{ChunkIdx, ModuleIdx, SymbolRef};
use rolldown_rstr::Rstr;
use rustc_hash::FxHashMap;

use super::{ast_symbols::AstSymbols, namespace_alias::NamespaceAlias};
use super::namespace_alias::NamespaceAlias;

#[derive(Debug)]
pub struct SymbolRefDataClassic {
Expand Down Expand Up @@ -32,7 +33,7 @@ pub struct SymbolRefDbForModule {
}

impl SymbolRefDbForModule {
pub fn fill_classic_data(&mut self, ast_symbols: AstSymbols) {
pub fn fill_classic_data(&mut self, ast_symbols: SymbolTable) {
self.classic_data = ast_symbols
.names
.into_iter()
Expand Down
7 changes: 2 additions & 5 deletions crates/rolldown/src/utils/make_ast_symbol_and_scope.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
use oxc::semantic::{ScopeTree, SymbolTable};
use rolldown_common::AstScopes;

use crate::types::ast_symbols::AstSymbols;

pub fn make_ast_scopes_and_symbols(
symbols: SymbolTable,
scopes: ScopeTree,
) -> (AstSymbols, AstScopes) {
) -> (SymbolTable, AstScopes) {
let mut symbols = symbols;
let ast_scope = AstScopes::new(
scopes,
std::mem::take(&mut symbols.references),
std::mem::take(&mut symbols.resolved_references),
);
let ast_symbols = AstSymbols::from_symbol_table(symbols);
(ast_symbols, ast_scope)
(symbols, ast_scope)
}

0 comments on commit 167f46a

Please sign in to comment.