perf: do not pass &Atom to functions (#3818)

`Atom` is just a wrapper around `&str`, so better not to pass `&Atom` to functions, as that's a double-reference. Prefer `Atom` or `&str` instead to avoid indirection.
oxc-project · Jun 22, 2024 · 4f7ff7e · 4f7ff7e
1 parent aaa944d
commit 4f7ff7e
Show file tree

Hide file tree

Showing 36 changed files with 173 additions and 171 deletions.
diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs
@@ -642,9 +642,12 @@ impl<'a> PropertyKey<'a> {
             Self::NumericLiteral(lit) => Some(lit.value.to_string().into()),
             Self::BigintLiteral(lit) => Some(lit.raw.to_compact_str()),
             Self::NullLiteral(_) => Some("null".into()),
-            Self::TemplateLiteral(lit) => {
-                lit.expressions.is_empty().then(|| lit.quasi()).flatten().map(Atom::to_compact_str)
-            }
+            Self::TemplateLiteral(lit) => lit
+                .expressions
+                .is_empty()
+                .then(|| lit.quasi())
+                .flatten()
+                .map(|quasi| quasi.to_compact_str()),
             _ => None,
         }
     }
@@ -661,16 +664,16 @@ impl<'a> PropertyKey<'a> {
         matches!(self, Self::PrivateIdentifier(_))
     }
 
-    pub fn private_name(&self) -> Option<&Atom<'a>> {
+    pub fn private_name(&self) -> Option<Atom<'a>> {
         match self {
-            Self::PrivateIdentifier(ident) => Some(&ident.name),
+            Self::PrivateIdentifier(ident) => Some(ident.name.clone()),
             _ => None,
         }
     }
 
     pub fn name(&self) -> Option<CompactStr> {
         if self.is_private_identifier() {
-            self.private_name().map(Atom::to_compact_str)
+            self.private_name().map(|name| name.to_compact_str())
         } else {
             self.static_name()
         }
@@ -717,8 +720,8 @@ impl<'a> TemplateLiteral<'a> {
     }
 
     /// Get single quasi from `template`
-    pub fn quasi(&self) -> Option<&Atom<'a>> {
-        self.quasis.first().and_then(|quasi| quasi.value.cooked.as_ref())
+    pub fn quasi(&self) -> Option<Atom<'a>> {
+        self.quasis.first().and_then(|quasi| quasi.value.cooked.clone())
     }
 }
 
@@ -2200,7 +2203,7 @@ impl<'a> BindingPattern<'a> {
         Self { kind, type_annotation: None, optional: false }
     }
 
-    pub fn get_identifier(&self) -> Option<&Atom<'a>> {
+    pub fn get_identifier(&self) -> Option<Atom<'a>> {
         self.kind.get_identifier()
     }
 
@@ -2228,9 +2231,9 @@ pub enum BindingPatternKind<'a> {
 }
 
 impl<'a> BindingPatternKind<'a> {
-    pub fn get_identifier(&self) -> Option<&Atom<'a>> {
+    pub fn get_identifier(&self) -> Option<Atom<'a>> {
         match self {
-            Self::BindingIdentifier(ident) => Some(&ident.name),
+            Self::BindingIdentifier(ident) => Some(ident.name.clone()),
             Self::AssignmentPattern(assign) => assign.left.get_identifier(),
             _ => None,
         }
@@ -3402,10 +3405,10 @@ impl<'a> fmt::Display for ModuleExportName<'a> {
 }
 
 impl<'a> ModuleExportName<'a> {
-    pub fn name(&self) -> &Atom<'a> {
+    pub fn name(&self) -> Atom<'a> {
         match self {
-            Self::Identifier(identifier) => &identifier.name,
-            Self::StringLiteral(literal) => &literal.value,
+            Self::Identifier(identifier) => identifier.name.clone(),
+            Self::StringLiteral(literal) => literal.value.clone(),
         }
     }
 }
diff --git a/crates/oxc_ast/src/ast/ts.rs b/crates/oxc_ast/src/ast/ts.rs
@@ -980,10 +980,10 @@ impl<'a> TSModuleDeclarationName<'a> {
         matches!(self, Self::StringLiteral(_))
     }
 
-    pub fn name(&self) -> &Atom<'a> {
+    pub fn name(&self) -> Atom<'a> {
         match self {
-            Self::Identifier(ident) => &ident.name,
-            Self::StringLiteral(lit) => &lit.value,
+            Self::Identifier(ident) => ident.name.clone(),
+            Self::StringLiteral(lit) => lit.value.clone(),
         }
     }
 }

diff --git a/crates/oxc_codegen/src/gen.rs b/crates/oxc_codegen/src/gen.rs
@@ -1118,7 +1118,7 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for IdentifierName<'a> {
 
 impl<'a, const MINIFY: bool> Gen<MINIFY> for BindingIdentifier<'a> {
     fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
-        p.print_symbol(self.span, self.symbol_id.get(), &self.name);
+        p.print_symbol(self.span, self.symbol_id.get(), self.name.as_str());
     }
 }
 

diff --git a/crates/oxc_codegen/src/lib.rs b/crates/oxc_codegen/src/lib.rs
@@ -15,7 +15,7 @@ use oxc_ast::{
     ast::{BlockStatement, Directive, Expression, Program, Statement},
     Comment, Trivias,
 };
-use oxc_span::{Atom, Span};
+use oxc_span::Span;
 use oxc_syntax::{
     identifier::is_identifier_part,
     operator::{BinaryOperator, UnaryOperator, UpdateOperator},
@@ -394,7 +394,8 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
         }
     }
 
-    fn print_symbol(&mut self, span: Span, _symbol_id: Option<SymbolId>, fallback: &Atom) {
+    #[allow(clippy::needless_pass_by_value)]
+    fn print_symbol(&mut self, span: Span, _symbol_id: Option<SymbolId>, fallback: &str) {
         // if let Some(mangler) = &self.mangler {
         // if let Some(symbol_id) = symbol_id {
         // let name = mangler.get_symbol_name(symbol_id);

diff --git a/crates/oxc_isolated_declarations/src/declaration.rs b/crates/oxc_isolated_declarations/src/declaration.rs
@@ -61,7 +61,7 @@ impl<'a> IsolatedDeclarations<'a> {
 
         if check_binding {
             if let Some(name) = decl.id.get_identifier() {
-                if !self.scope.has_reference(name) {
+                if !self.scope.has_reference(&name) {
                     return None;
                 }
             }

diff --git a/crates/oxc_isolated_declarations/src/diagnostics.rs b/crates/oxc_isolated_declarations/src/diagnostics.rs
@@ -1,5 +1,5 @@
 use oxc_diagnostics::OxcDiagnostic;
-use oxc_span::{Atom, Span};
+use oxc_span::Span;
 
 pub fn function_must_have_explicit_return_type(span: Span) -> OxcDiagnostic {
     OxcDiagnostic::error(
@@ -133,7 +133,8 @@ pub fn computed_property_name(span: Span) -> OxcDiagnostic {
         .with_label(span)
 }
 
-pub fn type_containing_private_name(name: &Atom<'_>, span: Span) -> OxcDiagnostic {
+#[allow(clippy::needless_pass_by_value)]
+pub fn type_containing_private_name(name: &str, span: Span) -> OxcDiagnostic {
     OxcDiagnostic::error(format!(
         "TS9039: Type containing private name '{name}' can't be used with --isolatedDeclarations."
     ))

diff --git a/crates/oxc_isolated_declarations/src/enum.rs b/crates/oxc_isolated_declarations/src/enum.rs
@@ -103,17 +103,17 @@ impl<'a> IsolatedDeclarations<'a> {
     fn computed_constant_value(
         &self,
         expr: &Expression<'a>,
-        enum_name: &Atom<'a>,
+        enum_name: &str,
         prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
     ) -> Option<ConstantValue> {
         self.evaluate(expr, enum_name, prev_members)
     }
 
-    #[allow(clippy::unused_self)]
+    #[allow(clippy::unused_self, clippy::needless_pass_by_value)]
     fn evaluate_ref(
         &self,
         expr: &Expression<'a>,
-        enum_name: &Atom<'a>,
+        enum_name: &str,
         prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
     ) -> Option<ConstantValue> {
         match expr {
@@ -147,7 +147,7 @@ impl<'a> IsolatedDeclarations<'a> {
     fn evaluate(
         &self,
         expr: &Expression<'a>,
-        enum_name: &Atom<'a>,
+        enum_name: &str,
         prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
     ) -> Option<ConstantValue> {
         match expr {
@@ -183,7 +183,7 @@ impl<'a> IsolatedDeclarations<'a> {
     fn eval_binary_expression(
         &self,
         expr: &BinaryExpression<'a>,
-        enum_name: &Atom<'a>,
+        enum_name: &str,
         prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
     ) -> Option<ConstantValue> {
         let left = self.evaluate(&expr.left, enum_name, prev_members)?;
@@ -249,7 +249,7 @@ impl<'a> IsolatedDeclarations<'a> {
     fn eval_unary_expression(
         &self,
         expr: &UnaryExpression<'a>,
-        enum_name: &Atom<'a>,
+        enum_name: &str,
         prev_members: &FxHashMap<Atom<'a>, ConstantValue>,
     ) -> Option<ConstantValue> {
         let value = self.evaluate(&expr.argument, enum_name, prev_members)?;

diff --git a/crates/oxc_isolated_declarations/src/module.rs b/crates/oxc_isolated_declarations/src/module.rs
@@ -111,7 +111,7 @@ impl<'a> IsolatedDeclarations<'a> {
                 self.scope.has_reference(&specifier.local.name)
             }
             ImportDeclarationSpecifier::ImportNamespaceSpecifier(_) => {
-                self.scope.has_reference(&self.ast.new_atom(&specifier.name()))
+                self.scope.has_reference(specifier.name().as_str())
             }
         });
         if specifiers.is_empty() {

diff --git a/crates/oxc_isolated_declarations/src/scope.rs b/crates/oxc_isolated_declarations/src/scope.rs
@@ -34,7 +34,7 @@ impl<'a> ScopeTree<'a> {
         self.flags.last().unwrap().contains(ScopeFlags::TsModuleBlock)
     }
 
-    pub fn has_reference(&self, name: &Atom<'a>) -> bool {
+    pub fn has_reference(&self, name: &str) -> bool {
         self.value_references.last().is_some_and(|rs| rs.contains(name))
             || self.type_references.last().is_some_and(|rs| rs.contains(name))
     }
@@ -43,20 +43,20 @@ impl<'a> ScopeTree<'a> {
         self.value_references.last().unwrap().len() + self.type_references.last().unwrap().len()
     }
 
-    fn add_value_binding(&mut self, ident: &Atom<'a>) {
-        self.value_bindings.last_mut().unwrap().insert(ident.clone());
+    fn add_value_binding(&mut self, ident: Atom<'a>) {
+        self.value_bindings.last_mut().unwrap().insert(ident);
     }
 
-    fn add_type_binding(&mut self, ident: &Atom<'a>) {
-        self.type_bindings.last_mut().unwrap().insert(ident.clone());
+    fn add_type_binding(&mut self, ident: Atom<'a>) {
+        self.type_bindings.last_mut().unwrap().insert(ident);
     }
 
-    fn add_value_reference(&mut self, ident: &Atom<'a>) {
-        self.value_references.last_mut().unwrap().insert(ident.clone());
+    fn add_value_reference(&mut self, ident: Atom<'a>) {
+        self.value_references.last_mut().unwrap().insert(ident);
     }
 
-    fn add_type_reference(&mut self, ident: &Atom<'a>) {
-        self.type_references.last_mut().unwrap().insert(ident.clone());
+    fn add_type_reference(&mut self, ident: Atom<'a>) {
+        self.type_references.last_mut().unwrap().insert(ident);
     }
 
     /// resolve references in the current scope
@@ -94,19 +94,19 @@ impl<'a> Visit<'a> for ScopeTree<'a> {
     }
 
     fn visit_identifier_reference(&mut self, ident: &IdentifierReference<'a>) {
-        self.add_value_reference(&ident.name);
+        self.add_value_reference(ident.name.clone());
     }
 
     fn visit_binding_pattern(&mut self, pattern: &BindingPattern<'a>) {
         if let BindingPatternKind::BindingIdentifier(ident) = &pattern.kind {
-            self.add_value_binding(&ident.name);
+            self.add_value_binding(ident.name.clone());
         }
         walk_binding_pattern(self, pattern);
     }
 
     fn visit_ts_type_name(&mut self, name: &TSTypeName<'a>) {
         if let TSTypeName::IdentifierReference(ident) = name {
-            self.add_type_reference(&ident.name);
+            self.add_type_reference(ident.name.clone());
         } else {
             walk_ts_type_name(self, name);
         }
@@ -115,7 +115,7 @@ impl<'a> Visit<'a> for ScopeTree<'a> {
     fn visit_ts_type_query(&mut self, ty: &TSTypeQuery<'a>) {
         if let Some(type_name) = ty.expr_name.as_ts_type_name() {
             let ident = TSTypeName::get_first_name(type_name);
-            self.add_value_reference(&ident.name);
+            self.add_value_reference(ident.name.clone());
         } else {
             walk_ts_type_query(self, ty);
         }
@@ -124,16 +124,16 @@ impl<'a> Visit<'a> for ScopeTree<'a> {
     fn visit_export_named_declaration(&mut self, decl: &ExportNamedDeclaration<'a>) {
         for specifier in &decl.specifiers {
             if let ModuleExportName::Identifier(ident) = &specifier.local {
-                self.add_type_reference(&ident.name);
-                self.add_value_reference(&ident.name);
+                self.add_type_reference(ident.name.clone());
+                self.add_value_reference(ident.name.clone());
             }
         }
     }
 
     fn visit_export_default_declaration(&mut self, decl: &ExportDefaultDeclaration<'a>) {
         if let ExportDefaultDeclarationKind::Identifier(ident) = &decl.declaration {
-            self.add_type_reference(&ident.name);
-            self.add_value_reference(&ident.name);
+            self.add_type_reference(ident.name.clone());
+            self.add_value_reference(ident.name.clone());
         } else {
             walk_export_default_declaration(self, decl);
         }
@@ -146,32 +146,32 @@ impl<'a> Visit<'a> for ScopeTree<'a> {
             }
             Declaration::FunctionDeclaration(decl) => {
                 if let Some(id) = decl.id.as_ref() {
-                    self.add_value_binding(&id.name);
+                    self.add_value_binding(id.name.clone());
                 }
             }
             Declaration::ClassDeclaration(decl) => {
                 if let Some(id) = decl.id.as_ref() {
-                    self.add_value_binding(&id.name);
+                    self.add_value_binding(id.name.clone());
                 }
             }
             Declaration::TSTypeAliasDeclaration(decl) => {
-                self.add_type_binding(&decl.id.name);
+                self.add_type_binding(decl.id.name.clone());
             }
             Declaration::TSInterfaceDeclaration(decl) => {
-                self.add_type_binding(&decl.id.name);
+                self.add_type_binding(decl.id.name.clone());
             }
             Declaration::TSEnumDeclaration(decl) => {
-                self.add_value_binding(&decl.id.name);
-                self.add_type_binding(&decl.id.name);
+                self.add_value_binding(decl.id.name.clone());
+                self.add_type_binding(decl.id.name.clone());
             }
             Declaration::TSModuleDeclaration(decl) => {
                 if let TSModuleDeclarationName::Identifier(ident) = &decl.id {
-                    self.add_value_binding(&ident.name);
-                    self.add_type_binding(&ident.name);
+                    self.add_value_binding(ident.name.clone());
+                    self.add_type_binding(ident.name.clone());
                 }
             }
             Declaration::TSImportEqualsDeclaration(decl) => {
-                self.add_value_binding(&decl.id.name);
+                self.add_value_binding(decl.id.name.clone());
             }
         }
         walk_declaration(self, declaration);
@@ -268,7 +268,7 @@ impl<'a> Visit<'a> for ScopeTree<'a> {
     fn visit_ts_mapped_type(&mut self, ty: &TSMappedType<'a>) {
         // copy from walk_ts_mapped_type
         self.enter_scope(ScopeFlags::empty());
-        self.add_type_binding(&ty.type_parameter.name.name);
+        self.add_type_binding(ty.type_parameter.name.name.clone());
         if let Some(name) = &ty.name_type {
             self.visit_ts_type(name);
         }
@@ -290,6 +290,6 @@ impl<'a> Visit<'a> for ScopeTree<'a> {
 
     fn visit_ts_infer_type(&mut self, ty: &TSInferType<'a>) {
         // copy from walk_ts_infer_type
-        self.add_type_binding(&ty.type_parameter.name.name);
+        self.add_type_binding(ty.type_parameter.name.name.clone());
     }
 }