From 1b4412fdaaf8c359265abd212e8e4d749f17bb67 Mon Sep 17 00:00:00 2001
From: overlookmotel <theoverlookmotel@gmail.com>
Date: Wed, 11 Sep 2024 18:26:13 +0100
Subject: [PATCH] refactor(semantic): move `Counts` code into counter module

---
 Cargo.lock                         |  7 +++++++
 Cargo.toml                         |  1 +
 crates/oxc_semantic/Cargo.toml     |  1 +
 crates/oxc_semantic/src/builder.rs | 23 +++++++++++------------
 crates/oxc_semantic/src/counter.rs | 27 ++++++++++++++++++++++++++-
 5 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index e5219370b6d363..735a956c26c373 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1232,6 +1232,12 @@ dependencies = [
  "windows-sys 0.52.0",
 ]
 
+[[package]]
+name = "more-asserts"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1fafa6961cabd9c63bcd77a45d7e3b7f3b552b70417831fb0f56db717e72407e"
+
 [[package]]
 name = "napi"
 version = "3.0.0-alpha.8"
@@ -1852,6 +1858,7 @@ dependencies = [
  "indexmap",
  "insta",
  "itertools",
+ "more-asserts",
  "oxc_allocator",
  "oxc_ast",
  "oxc_cfg",
diff --git a/Cargo.toml b/Cargo.toml
index 34b3604d0962c5..dc20a3461becb8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -147,6 +147,7 @@ memoffset = "0.9.1"
 miette = { version = "7.2.0", features = ["fancy-no-syscall"] }
 mimalloc = "0.1.43"
 mime_guess = "2.0.5"
+more-asserts = "0.3.1"
 nonmax = "0.5.5"
 num-bigint = "0.4.6"
 num-traits = "0.2.19"
diff --git a/crates/oxc_semantic/Cargo.toml b/crates/oxc_semantic/Cargo.toml
index 446bd110717112..1f0d9834bda988 100644
--- a/crates/oxc_semantic/Cargo.toml
+++ b/crates/oxc_semantic/Cargo.toml
@@ -30,6 +30,7 @@ oxc_syntax = { workspace = true }
 assert-unchecked = { workspace = true }
 indexmap = { workspace = true }
 itertools = { workspace = true }
+more-asserts = { workspace = true }
 phf = { workspace = true, features = ["macros"] }
 rustc-hash = { workspace = true }
 serde = { workspace = true, features = ["derive"], optional = true }
diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs
index 24e3444ca5fda3..52cdcade7dc2c6 100644
--- a/crates/oxc_semantic/src/builder.rs
+++ b/crates/oxc_semantic/src/builder.rs
@@ -231,8 +231,7 @@ impl<'a> SemanticBuilder<'a> {
             // Avoiding this growth produces up to 30% perf boost on our benchmarks.
             // TODO: It would be even more efficient to calculate counts in parser to avoid
             // this extra AST traversal.
-            let mut counts = Counts::default();
-            counts.visit_program(program);
+            let counts = Counts::count(program);
             self.nodes.reserve(counts.nodes);
             self.scope.reserve(counts.scopes);
             self.symbols.reserve(counts.symbols, counts.references);
@@ -241,16 +240,16 @@ impl<'a> SemanticBuilder<'a> {
             self.visit_program(program);
 
             // Check that estimated counts accurately
-            debug_assert_eq!(self.nodes.len(), counts.nodes);
-            debug_assert_eq!(self.scope.len(), counts.scopes);
-            debug_assert_eq!(self.symbols.references.len(), counts.references);
-            // `Counts` may overestimate number of symbols, because multiple `BindingIdentifier`s
-            // can result in only a single symbol.
-            // e.g. `var x; var x;` = 2 x `BindingIdentifier` but 1 x symbol.
-            // This is not a big problem - allocating a `Vec` with excess capacity is cheap.
-            // It's allocating with *not enough* capacity which is costly, as then the `Vec`
-            // will grow and reallocate.
-            debug_assert!(self.symbols.len() <= counts.symbols);
+            #[cfg(debug_assertions)]
+            {
+                let actual_counts = Counts {
+                    nodes: self.nodes.len(),
+                    scopes: self.scope.len(),
+                    symbols: self.symbols.len(),
+                    references: self.symbols.references.len(),
+                };
+                Counts::assert_accurate(&actual_counts, &counts);
+            }
 
             // Checking syntax error on module record requires scope information from the previous AST pass
             if self.check_syntax_error {
diff --git a/crates/oxc_semantic/src/counter.rs b/crates/oxc_semantic/src/counter.rs
index 345633464ebe67..52c2bb688c98cc 100644
--- a/crates/oxc_semantic/src/counter.rs
+++ b/crates/oxc_semantic/src/counter.rs
@@ -4,8 +4,12 @@
 
 use std::cell::Cell;
 
+use more_asserts::assert_le;
+
 use oxc_ast::{
-    ast::{BindingIdentifier, IdentifierReference, TSEnumMemberName, TSModuleDeclarationName},
+    ast::{
+        BindingIdentifier, IdentifierReference, Program, TSEnumMemberName, TSModuleDeclarationName,
+    },
     visit::walk::{walk_ts_enum_member_name, walk_ts_module_declaration_name},
     AstKind, Visit,
 };
@@ -19,6 +23,27 @@ pub(crate) struct Counts {
     pub references: usize,
 }
 
+impl Counts {
+    pub fn count(program: &Program) -> Self {
+        let mut counts = Counts::default();
+        counts.visit_program(program);
+        counts
+    }
+
+    pub fn assert_accurate(actual: &Self, estimated: &Self) {
+        assert_eq!(actual.nodes, estimated.nodes, "nodes count mismatch");
+        assert_eq!(actual.scopes, estimated.scopes, "scopes count mismatch");
+        assert_eq!(actual.references, estimated.references, "references count mismatch");
+        // `Counts` may overestimate number of symbols, because multiple `BindingIdentifier`s
+        // can result in only a single symbol.
+        // e.g. `var x; var x;` = 2 x `BindingIdentifier` but 1 x symbol.
+        // This is not a big problem - allocating a `Vec` with excess capacity is cheap.
+        // It's allocating with *not enough* capacity which is costly, as then the `Vec`
+        // will grow and reallocate.
+        assert_le!(actual.symbols, estimated.symbols, "symbols count mismatch");
+    }
+}
+
 impl<'a> Visit<'a> for Counts {
     #[inline]
     fn enter_node(&mut self, _: AstKind<'a>) {