Skip to content

Commit

Permalink
feat!(ast): make Trivias clonable by adding Arc (#3638)
Browse files Browse the repository at this point in the history
This makes `Trivias` cloneable and stops us from using `Rc::new` and
`Rc::clone` everywhere.

`Trivias` is rarely cloned so an `Arc` should suffice.
  • Loading branch information
Boshen committed Jun 12, 2024
1 parent 84304b4 commit f6752b4
Show file tree
Hide file tree
Showing 31 changed files with 89 additions and 86 deletions.
20 changes: 16 additions & 4 deletions crates/oxc_ast/src/trivia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

use std::{
collections::btree_map::{BTreeMap, Range},
ops::RangeBounds,
ops::{Deref, RangeBounds},
sync::Arc,
};

use oxc_span::Span;
Expand Down Expand Up @@ -38,17 +39,28 @@ impl CommentKind {

pub type TriviasMap = BTreeMap<u32, Comment>;

#[derive(Debug, Clone, Default)]
pub struct Trivias(Arc<TriviasImpl>);

#[derive(Debug, Default)]
pub struct Trivias {
pub struct TriviasImpl {
/// Keyed by span.start
comments: TriviasMap,

irregular_whitespaces: Vec<Span>,
}

impl Deref for Trivias {
type Target = TriviasImpl;

fn deref(&self) -> &Self::Target {
self.0.as_ref()
}
}

impl Trivias {
pub fn new(comments: TriviasMap, irregular_whitespaces: Vec<Span>) -> Self {
Self { comments, irregular_whitespaces }
pub fn new(comments: TriviasMap, irregular_whitespaces: Vec<Span>) -> Trivias {
Self(Arc::new(TriviasImpl { comments, irregular_whitespaces }))
}

pub fn comments(&self) -> impl Iterator<Item = (CommentKind, Span)> + '_ {
Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ pub struct Codegen<'a, const MINIFY: bool> {
indentation: u8,

sourcemap_builder: Option<SourcemapBuilder>,
comment_gen_related: Option<CommentGenRelated<'a>>,
comment_gen_related: Option<CommentGenRelated>,
source_code: &'a str,
}

pub struct CommentGenRelated<'a> {
pub trivials: &'a Trivias,
pub struct CommentGenRelated {
pub trivials: Trivias,
/// The key of map is the node start position,
/// the first element of value is the start of the comment
/// the second element of value includes the end of the comment and comment kind.
Expand All @@ -115,7 +115,7 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
source_name: &str,
source_code: &'a str,
options: CodegenOptions,
comment_gen_related: Option<CommentGenRelated<'a>>,
comment_gen_related: Option<CommentGenRelated>,
) -> Self {
// Initialize the output code buffer to reduce memory reallocation.
// Minification will reduce by at least half of the original size.
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_codegen/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn test(source_text: &str, expected: &str, options: Option<CodegenOptions>) {
source_text,
options,
Some(oxc_codegen::CommentGenRelated {
trivials: &parse_return.trivias,
trivials: parse_return.trivias,
move_comment_map: FxHashMap::default(),
}),
)
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_language_server/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl IsolatedLintHandler {

let program = allocator.alloc(ret.program);
let semantic_ret = SemanticBuilder::new(javascript_source_text, source_type)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.with_check_syntax_error(true)
.build(program);

Expand Down
7 changes: 3 additions & 4 deletions crates/oxc_linter/examples/linter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! The simplest linter

use std::{env, path::Path, rc::Rc};
use std::{env, path::Path};

use oxc_allocator::Allocator;
use oxc_ast::AstKind;
Expand Down Expand Up @@ -29,9 +29,8 @@ fn main() -> std::io::Result<()> {
}

let program = allocator.alloc(ret.program);
let semantic_ret = SemanticBuilder::new(&source_text, source_type)
.with_trivias(Rc::new(ret.trivias))
.build(program);
let semantic_ret =
SemanticBuilder::new(&source_text, source_type).with_trivias(ret.trivias).build(program);

let mut errors: Vec<OxcDiagnostic> = vec![];

Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_linter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ pub struct LintContext<'a> {
impl<'a> LintContext<'a> {
pub fn new(file_path: Box<Path>, semantic: Rc<Semantic<'a>>) -> Self {
let disable_directives =
DisableDirectivesBuilder::new(semantic.source_text(), semantic.trivias()).build();
DisableDirectivesBuilder::new(semantic.source_text(), semantic.trivias().clone())
.build();
Self {
semantic,
diagnostics: RefCell::new(vec![]),
Expand Down
10 changes: 5 additions & 5 deletions crates/oxc_linter/src/disable_directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ impl<'a> DisableDirectives<'a> {
}
}

pub struct DisableDirectivesBuilder<'a, 'b> {
pub struct DisableDirectivesBuilder<'a> {
source_text: &'a str,
trivias: &'b Trivias,
trivias: Trivias,
/// All the disabled rules with their corresponding covering spans
intervals: Lapper<u32, DisabledRule<'a>>,
/// Start of `eslint-disable` or `oxlint-disable`
Expand All @@ -61,8 +61,8 @@ pub struct DisableDirectivesBuilder<'a, 'b> {
disable_rule_comments: Vec<DisableRuleComment<'a>>,
}

impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
pub fn new(source_text: &'a str, trivias: &'b Trivias) -> Self {
impl<'a> DisableDirectivesBuilder<'a> {
pub fn new(source_text: &'a str, trivias: Trivias) -> Self {
Self {
source_text,
trivias,
Expand Down Expand Up @@ -93,7 +93,7 @@ impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
// This algorithm iterates through the comments and builds all intervals
// for matching disable and enable pairs.
// Wrongly ordered matching pairs are not taken into consideration.
for (_, span) in self.trivias.comments() {
for (_, span) in self.trivias.clone().comments() {
let text = span.source_text(self.source_text);
let text = text.trim_start();

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl Runtime {

let program = allocator.alloc(ret.program);

let trivias = Rc::new(ret.trivias);
let trivias = ret.trivias;

// Build the module record to unblock other threads from waiting for too long.
// The semantic model is not built at this stage.
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_prettier/examples/prettier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ fn main() -> std::io::Result<()> {
let output = Prettier::new(
&allocator,
&source_text,
&ret.trivias,
ret.trivias,
PrettierOptions { semi, trailing_comma: TrailingComma::All, ..PrettierOptions::default() },
)
.build(&ret.program);
Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_prettier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,11 @@ impl<'a> DocBuilder<'a> for Prettier<'a> {
}

impl<'a> Prettier<'a> {
#[allow(clippy::needless_pass_by_value)]
pub fn new(
allocator: &'a Allocator,
source_text: &'a str,
trivias: &Trivias,
trivias: Trivias,
options: PrettierOptions,
) -> Self {
Self {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/examples/cfg.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{collections::HashMap, env, path::Path, rc::Rc, sync::Arc};
use std::{collections::HashMap, env, path::Path, sync::Arc};

use itertools::Itertools;
use oxc_allocator::Allocator;
Expand Down Expand Up @@ -38,7 +38,7 @@ fn main() -> std::io::Result<()> {

let semantic = SemanticBuilder::new(&source_text, source_type)
.with_check_syntax_error(true)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.build(program);

if !semantic.errors.is_empty() {
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/examples/simple.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{env, path::Path, rc::Rc, sync::Arc};
use std::{env, path::Path, sync::Arc};

use itertools::Itertools;
use oxc_allocator::Allocator;
Expand All @@ -23,7 +23,7 @@ fn main() -> std::io::Result<()> {

let semantic = SemanticBuilder::new(&source_text, source_type)
.with_check_syntax_error(true)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.build(program);

if !semantic.errors.is_empty() {
Expand Down
14 changes: 7 additions & 7 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Semantic Builder

use std::{cell::RefCell, path::PathBuf, rc::Rc, sync::Arc};
use std::{cell::RefCell, path::PathBuf, sync::Arc};

#[allow(clippy::wildcard_imports)]
use oxc_ast::{ast::*, AstKind, Trivias, Visit};
Expand Down Expand Up @@ -35,7 +35,7 @@ pub struct SemanticBuilder<'a> {

pub source_type: SourceType,

trivias: Rc<Trivias>,
trivias: Trivias,

/// Semantic early errors such as redeclaration errors.
errors: RefCell<Vec<OxcDiagnostic>>,
Expand Down Expand Up @@ -82,11 +82,11 @@ impl<'a> SemanticBuilder<'a> {
let scope = ScopeTree::default();
let current_scope_id = scope.root_scope_id();

let trivias = Rc::new(Trivias::default());
let trivias = Trivias::default();
Self {
source_text,
source_type,
trivias: Rc::clone(&trivias),
trivias: trivias.clone(),
errors: RefCell::new(vec![]),
current_node_id: AstNodeId::new(0),
current_node_flags: NodeFlags::empty(),
Expand All @@ -108,9 +108,9 @@ impl<'a> SemanticBuilder<'a> {
}

#[must_use]
pub fn with_trivias(mut self, trivias: Rc<Trivias>) -> Self {
self.trivias = trivias;
self.jsdoc = JSDocBuilder::new(self.source_text, Rc::clone(&self.trivias));
pub fn with_trivias(mut self, trivias: Trivias) -> Self {
self.trivias = trivias.clone();
self.jsdoc = JSDocBuilder::new(self.source_text, trivias);
self
}

Expand Down
9 changes: 3 additions & 6 deletions crates/oxc_semantic/src/jsdoc/builder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::collections::BTreeMap;
use std::rc::Rc;

use super::parser::JSDoc;
use crate::jsdoc::JSDocFinder;
Expand All @@ -9,13 +8,13 @@ use rustc_hash::FxHashSet;

pub struct JSDocBuilder<'a> {
source_text: &'a str,
trivias: Rc<Trivias>,
trivias: Trivias,
attached_docs: BTreeMap<Span, Vec<JSDoc<'a>>>,
leading_comments_seen: FxHashSet<u32>,
}

impl<'a> JSDocBuilder<'a> {
pub fn new(source_text: &'a str, trivias: Rc<Trivias>) -> Self {
pub fn new(source_text: &'a str, trivias: Trivias) -> Self {
Self {
source_text,
trivias,
Expand Down Expand Up @@ -230,8 +229,6 @@ fn should_attach_jsdoc(kind: &AstKind) -> bool {

#[cfg(test)]
mod test {
use std::rc::Rc;

use oxc_allocator::Allocator;
use oxc_parser::Parser;
use oxc_span::{SourceType, Span};
Expand All @@ -248,7 +245,7 @@ mod test {
let ret = Parser::new(allocator, source_text, source_type).parse();
let program = allocator.alloc(ret.program);
let semantic = SemanticBuilder::new(source_text, source_type)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.build(program)
.semantic;
semantic
Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_semantic/src/jsdoc/parser/jsdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ impl<'a> JSDoc<'a> {

#[cfg(test)]
mod test {
use std::rc::Rc;

use crate::{Semantic, SemanticBuilder};
use oxc_allocator::Allocator;
use oxc_parser::Parser;
Expand All @@ -48,7 +46,7 @@ mod test {
let ret = Parser::new(allocator, source_text, source_type).parse();
let program = allocator.alloc(ret.program);
let semantic = SemanticBuilder::new(source_text, source_type)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.build(program)
.semantic;
semantic
Expand Down
6 changes: 2 additions & 4 deletions crates/oxc_semantic/src/jsdoc/parser/jsdoc_tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ impl<'a> JSDocTag<'a> {

#[cfg(test)]
mod test {
use std::rc::Rc;

use crate::{Semantic, SemanticBuilder};
use oxc_allocator::Allocator;
use oxc_parser::Parser;
Expand All @@ -194,7 +192,7 @@ mod test {
let ret = Parser::new(allocator, source_text, source_type).parse();
let program = allocator.alloc(ret.program);
let semantic = SemanticBuilder::new(source_text, source_type)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.build(program)
.semantic;
semantic
Expand Down Expand Up @@ -356,7 +354,7 @@ mod test {
("/** @k */", None, ("", " ")),
("/** @k1 {t1} c1 */", Some(("t1", "{t1}")), ("c1", " c1 ")),
(
"/** @k2
"/** @k2
{t2} */",
Some(("t2", "{t2}")),
("", " "),
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod reference;
mod scope;
mod symbol;

use std::{rc::Rc, sync::Arc};
use std::sync::Arc;

pub use petgraph;
pub use petgraph::algo;
Expand Down Expand Up @@ -57,7 +57,7 @@ pub struct Semantic<'a> {

classes: ClassTable,

trivias: Rc<Trivias>,
trivias: Trivias,

module_record: Arc<ModuleRecord>,

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/module_record/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod module_record_tests {
use oxc_span::{SourceType, Span};
#[allow(clippy::wildcard_imports)]
use oxc_syntax::module_record::*;
use std::{path::PathBuf, rc::Rc, sync::Arc};
use std::{path::PathBuf, sync::Arc};

use crate::SemanticBuilder;

Expand All @@ -19,7 +19,7 @@ mod module_record_tests {
let ret = Parser::new(&allocator, source_text, source_type).parse();
let program = allocator.alloc(ret.program);
let semantic_ret = SemanticBuilder::new(source_text, source_type)
.with_trivias(Rc::new(ret.trivias))
.with_trivias(ret.trivias)
.build_module_record(PathBuf::new(), program)
.build(program);
Arc::clone(&semantic_ret.semantic.module_record)
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/tests/integration/util/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
mod class_tester;
mod expect;
mod symbol_tester;
use std::{path::PathBuf, rc::Rc, sync::Arc};
use std::{path::PathBuf, sync::Arc};

use itertools::Itertools;
use oxc_allocator::Allocator;
Expand Down Expand Up @@ -80,7 +80,7 @@ impl<'a> SemanticTester<'a> {
let program = self.allocator.alloc(parse.program);
let semantic_ret = SemanticBuilder::new(self.source_text, self.source_type)
.with_check_syntax_error(true)
.with_trivias(Rc::new(parse.trivias))
.with_trivias(parse.trivias)
.build_module_record(PathBuf::new(), program)
.build(program);

Expand Down
Loading

0 comments on commit f6752b4

Please sign in to comment.