From 5fd2a046e22b52a387c6c7eea1b7d7efc07895ef Mon Sep 17 00:00:00 2001
From: Andrew Morris <voltrevo@gmail.com>
Date: Tue, 25 Jul 2023 12:00:43 +1000
Subject: [PATCH] Simplify function compiling

---
 .../src/expression_compiler.rs                |  44 ++----
 valuescript_compiler/src/function_compiler.rs | 130 +++++++-----------
 valuescript_compiler/src/module_compiler.rs   |  57 +++-----
 .../src/static_expression_compiler.rs         |   4 +-
 4 files changed, 81 insertions(+), 154 deletions(-)

diff --git a/valuescript_compiler/src/expression_compiler.rs b/valuescript_compiler/src/expression_compiler.rs
index 766b7ec..275dbf4 100644
--- a/valuescript_compiler/src/expression_compiler.rs
+++ b/valuescript_compiler/src/expression_compiler.rs
@@ -1,11 +1,10 @@
 use std::mem::take;
 
-use queues::*;
 use swc_common::Spanned;
 
 use crate::asm::{Array, Instruction, Label, Number, Object, Register, Value};
 use crate::diagnostic::{Diagnostic, DiagnosticContainer, DiagnosticReporter};
-use crate::function_compiler::{FunctionCompiler, Functionish, QueuedFunction};
+use crate::function_compiler::{FunctionCompiler, Functionish};
 use crate::ident::Ident as CrateIdent;
 use crate::scope::{NameId, OwnerId};
 use crate::scope_analysis::{fn_to_owner_id, NameType};
@@ -527,15 +526,10 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
               None => self.fnc.allocate_defn_numbered("_anon"),
             };
 
-            self
-              .fnc
-              .queue
-              .add(QueuedFunction {
-                definition_pointer: p.clone(),
-                fn_name: fn_name.clone(),
-                functionish: Functionish::Fn(fn_ident, method.function.clone()),
-              })
-              .expect("Failed to queue function");
+            FunctionCompiler::new(self.fnc.mc).compile(
+              p.clone(),
+              Functionish::Fn(fn_ident, method.function.clone()),
+            );
 
             object_asm.properties.push((prop_key, Value::Pointer(p)));
           }
@@ -967,15 +961,10 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
       .scope_analysis
       .get_register_captures(&fn_to_owner_id(&fn_.ident, &fn_.function));
 
-    self
-      .fnc
-      .queue
-      .add(QueuedFunction {
-        definition_pointer: definition_pointer.clone(),
-        fn_name: fn_name.clone(),
-        functionish: Functionish::Fn(fn_.ident.clone(), fn_.function.clone()),
-      })
-      .expect("Failed to queue function");
+    FunctionCompiler::new(self.fnc.mc).compile(
+      definition_pointer.clone(),
+      Functionish::Fn(fn_.ident.clone(), fn_.function.clone()),
+    );
 
     match capture_params.len() {
       0 => Value::Pointer(definition_pointer).to_ce(),
@@ -1005,15 +994,10 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
       .scope_analysis
       .get_register_captures(&OwnerId::Span(arrow_expr.span));
 
-    self
-      .fnc
-      .queue
-      .add(QueuedFunction {
-        definition_pointer: definition_pointer.clone(),
-        fn_name: None,
-        functionish: Functionish::Arrow(arrow_expr.clone()),
-      })
-      .expect("Failed to queue function");
+    FunctionCompiler::new(self.fnc.mc).compile(
+      definition_pointer.clone(),
+      Functionish::Arrow(arrow_expr.clone()),
+    );
 
     match capture_params.len() {
       0 => Value::Pointer(definition_pointer).to_ce(),
@@ -1078,7 +1062,7 @@ impl<'a, 'fnc> ExpressionCompiler<'a, 'fnc> {
       // produce incorrect results, see captureShadowed.ts).
       let mut is_param = false;
 
-      for p in &self.fnc.current.parameters {
+      for p in &self.fnc.fn_.parameters {
         if p == &cap_reg {
           is_param = true;
         }
diff --git a/valuescript_compiler/src/function_compiler.rs b/valuescript_compiler/src/function_compiler.rs
index 6447291..353634b 100644
--- a/valuescript_compiler/src/function_compiler.rs
+++ b/valuescript_compiler/src/function_compiler.rs
@@ -1,4 +1,3 @@
-use queues::*;
 use std::collections::BTreeSet;
 use std::mem::take;
 
@@ -62,11 +61,10 @@ pub struct CatchSetting {
 
 pub struct FunctionCompiler<'a> {
   pub mc: &'a mut ModuleCompiler,
-  pub current: Function,
+  pub fn_: Function,
   pub owner_id: OwnerId,
   pub reg_allocator: RegAllocator,
   pub label_allocator: NameAllocator,
-  pub queue: Queue<QueuedFunction>,
   pub loop_labels: Vec<LoopLabels>,
   pub catch_settings: Vec<CatchSetting>,
   pub end_label: Option<Label>,
@@ -81,19 +79,13 @@ impl<'a> DiagnosticContainer for FunctionCompiler<'a> {
 }
 
 impl<'a> FunctionCompiler<'a> {
-  pub fn new(mc: &'a mut ModuleCompiler, owner_id: OwnerId) -> Self {
-    let reg_allocator = match mc.scope_analysis.reg_allocators.get(&owner_id) {
-      Some(reg_allocator) => reg_allocator.clone(),
-      None => RegAllocator::default(),
-    };
-
+  pub fn new(mc: &'a mut ModuleCompiler) -> Self {
     FunctionCompiler {
       mc,
-      current: Function::default(),
-      owner_id,
-      reg_allocator,
+      fn_: Function::default(),
+      owner_id: OwnerId::Span(swc_common::DUMMY_SP),
+      reg_allocator: RegAllocator::default(),
       label_allocator: NameAllocator::default(),
-      queue: Queue::new(),
       loop_labels: vec![],
       catch_settings: vec![],
       end_label: None,
@@ -111,16 +103,16 @@ impl<'a> FunctionCompiler<'a> {
   }
 
   pub fn push_raw(&mut self, instruction: Instruction) {
-    self.current.body.push(FnLine::Instruction(instruction));
+    self.fn_.body.push(FnLine::Instruction(instruction));
   }
 
   pub fn label(&mut self, label: Label) {
-    self.current.body.push(FnLine::Label(label));
+    self.fn_.body.push(FnLine::Label(label));
   }
 
   #[allow(dead_code)]
   pub fn comment(&mut self, message: String) {
-    self.current.body.push(FnLine::Comment(message));
+    self.fn_.body.push(FnLine::Comment(message));
   }
 
   pub fn lookup(&mut self, ident: &Ident) -> Option<&Name> {
@@ -188,45 +180,28 @@ impl<'a> FunctionCompiler<'a> {
     // `NameAllocator::release` for more information.
     // self.reg_allocator.release(reg);
 
-    self.current.body.push(FnLine::Release(reg.clone()));
+    self.fn_.body.push(FnLine::Release(reg.clone()));
   }
 
   pub fn insert_all_releases(&mut self) {
     for reg in self.reg_allocator.all_used() {
       if !reg.is_special() {
-        self.current.body.push(FnLine::Release(reg));
+        self.fn_.body.push(FnLine::Release(reg));
       }
     }
   }
 
-  pub fn compile(
-    mc: &'a mut ModuleCompiler,
-    definition_pointer: Pointer,
-    fn_name: Option<String>,
-    functionish: Functionish,
-  ) {
-    let mut self_ = FunctionCompiler::new(mc, functionish.owner_id());
-
-    self_
-      .queue
-      .add(QueuedFunction {
-        definition_pointer,
-        fn_name,
-        functionish,
-      })
-      .expect("Failed to queue function");
-
-    self_.process_queue();
-  }
+  pub fn set_owner_id(&mut self, owner_id: OwnerId) {
+    self.reg_allocator = match self.mc.scope_analysis.reg_allocators.get(&owner_id) {
+      Some(reg_allocator) => reg_allocator.clone(),
+      None => RegAllocator::default(),
+    };
 
-  pub fn process_queue(&mut self) {
-    while let Ok(qfn) = self.queue.remove() {
-      self.compile_functionish(qfn.definition_pointer, &qfn.functionish);
-    }
+    self.owner_id = owner_id;
   }
 
-  fn compile_functionish(&mut self, definition_pointer: Pointer, functionish: &Functionish) {
-    self.current.is_generator = match functionish {
+  pub fn compile(&mut self, definition_pointer: Pointer, functionish: Functionish) {
+    self.fn_.is_generator = match &functionish {
       Functionish::Fn(_, fn_) => fn_.is_generator,
 
       // Note: It isn't currently possible to have an arrow generator, but SWC includes the
@@ -236,18 +211,7 @@ impl<'a> FunctionCompiler<'a> {
       Functionish::Constructor(..) => false,
     };
 
-    // TODO: Use a new FunctionCompiler per function instead of this hack
-    self.reg_allocator = match self
-      .mc
-      .scope_analysis
-      .reg_allocators
-      .get(&functionish.owner_id())
-    {
-      Some(reg_allocator) => reg_allocator.clone(),
-      None => RegAllocator::default(),
-    };
-
-    self.owner_id = functionish.owner_id();
+    self.set_owner_id(functionish.owner_id());
 
     let capture_params = self
       .mc
@@ -270,16 +234,16 @@ impl<'a> FunctionCompiler<'a> {
         }
       };
 
-      self.current.parameters.push(reg.clone());
+      self.fn_.parameters.push(reg.clone());
     }
 
-    let param_registers = self.get_param_registers(functionish);
+    let param_registers = self.get_param_registers(&functionish);
 
     for reg in param_registers.iter().flatten() {
-      self.current.parameters.push(reg.clone());
+      self.fn_.parameters.push(reg.clone());
     }
 
-    self.add_param_code(functionish, &param_registers);
+    self.add_param_code(&functionish, &param_registers);
 
     match functionish {
       Functionish::Fn(_, fn_) => {
@@ -305,7 +269,7 @@ impl<'a> FunctionCompiler<'a> {
       },
       Functionish::Constructor(member_initializers_assembly, _class_span, constructor) => {
         let mut mia_copy = member_initializers_assembly.clone();
-        self.current.body.append(&mut mia_copy);
+        self.fn_.body.append(&mut mia_copy);
 
         match &constructor.body {
           Some(block) => {
@@ -319,7 +283,7 @@ impl<'a> FunctionCompiler<'a> {
     self.insert_all_releases();
 
     if let Some(end_label) = self.end_label.as_ref() {
-      self.current.body.push(FnLine::Label(end_label.clone()));
+      self.fn_.body.push(FnLine::Label(end_label.clone()));
 
       self.end_label = None;
       self.is_returning_register = None;
@@ -327,7 +291,7 @@ impl<'a> FunctionCompiler<'a> {
 
     self.mc.module.definitions.push(Definition {
       pointer: definition_pointer,
-      content: DefinitionContent::Function(take(&mut self.current)),
+      content: DefinitionContent::Function(take(&mut self.fn_)),
     });
   }
 
@@ -1151,28 +1115,26 @@ impl<'a> FunctionCompiler<'a> {
     match decl {
       Class(class) => self.todo(class.span(), "Class declaration"),
       Fn(fn_decl) => {
-        self
-          .queue
-          .add(QueuedFunction {
-            definition_pointer: match self.lookup_value(&Ident::from_swc_ident(&fn_decl.ident)) {
-              Some(Value::Pointer(p)) => p,
-              _ => {
-                self.internal_error(
-                  fn_decl.ident.span,
-                  &format!(
-                    "Lookup of function {} was not a pointer, lookup_result: {:?}",
-                    fn_decl.ident.sym,
-                    self.lookup_value(&Ident::from_swc_ident(&fn_decl.ident))
-                  ),
-                );
-
-                return;
-              }
-            },
-            fn_name: Some(fn_decl.ident.sym.to_string()),
-            functionish: Functionish::Fn(Some(fn_decl.ident.clone()), fn_decl.function.clone()),
-          })
-          .expect("Failed to add function to queue");
+        let p = match self.lookup_value(&Ident::from_swc_ident(&fn_decl.ident)) {
+          Some(Value::Pointer(p)) => p,
+          _ => {
+            self.internal_error(
+              fn_decl.ident.span,
+              &format!(
+                "Lookup of function {} was not a pointer, lookup_result: {:?}",
+                fn_decl.ident.sym,
+                self.lookup_value(&Ident::from_swc_ident(&fn_decl.ident))
+              ),
+            );
+
+            return;
+          }
+        };
+
+        FunctionCompiler::new(self.mc).compile(
+          p,
+          Functionish::Fn(Some(fn_decl.ident.clone()), fn_decl.function.clone()),
+        );
       }
       Var(var_decl) => self.var_declaration(var_decl),
       TsInterface(interface_decl) => self.todo(interface_decl.span, "TsInterface declaration"),
diff --git a/valuescript_compiler/src/module_compiler.rs b/valuescript_compiler/src/module_compiler.rs
index 3155324..b8db0e1 100644
--- a/valuescript_compiler/src/module_compiler.rs
+++ b/valuescript_compiler/src/module_compiler.rs
@@ -323,7 +323,6 @@ impl ModuleCompiler {
 
     self.compile_fn(
       pointer,
-      Some(fn_name),
       Functionish::Fn(Some(fn_.ident.clone()), fn_.function.clone()),
     );
   }
@@ -368,35 +367,28 @@ impl ModuleCompiler {
         self.module.export_default = Value::Pointer(pointer);
       }
       DefaultDecl::Fn(fn_) => {
-        let (fn_name, defn) = match &fn_.ident {
-          Some(ident) => {
-            let fn_name = ident.sym.to_string();
-
-            let defn = match self
-              .scope_analysis
-              .lookup_value(&OwnerId::Module, &Ident::from_swc_ident(ident))
-            {
-              Some(Value::Pointer(p)) => p,
-              _ => {
-                self.internal_error(
-                  ident.span,
-                  &format!("Definition for {} should have been in scope", fn_name),
-                );
-
-                return;
-              }
-            };
+        let defn = match &fn_.ident {
+          Some(ident) => match self
+            .scope_analysis
+            .lookup_value(&OwnerId::Module, &Ident::from_swc_ident(ident))
+          {
+            Some(Value::Pointer(p)) => p,
+            _ => {
+              self.internal_error(
+                ident.span,
+                &format!("Definition for {} should have been in scope", ident.sym),
+              );
 
-            (Some(fn_name), defn)
-          }
-          None => (None, self.allocate_defn_numbered("_anon")),
+              return;
+            }
+          },
+          None => self.allocate_defn_numbered("_anon"),
         };
 
         self.module.export_default = Value::Pointer(defn.clone());
 
         self.compile_fn(
           defn,
-          fn_name,
           Functionish::Fn(fn_.ident.clone(), fn_.function.clone()),
         );
       }
@@ -692,13 +684,8 @@ impl ModuleCompiler {
     }
   }
 
-  pub fn compile_fn(
-    &mut self,
-    defn_pointer: Pointer,
-    fn_name: Option<String>,
-    functionish: Functionish,
-  ) {
-    FunctionCompiler::compile(self, defn_pointer, fn_name, functionish);
+  pub fn compile_fn(&mut self, defn_pointer: Pointer, functionish: Functionish) {
+    FunctionCompiler::new(self).compile(defn_pointer, functionish);
   }
 
   pub fn compile_class(
@@ -737,7 +724,8 @@ impl ModuleCompiler {
     }
 
     // member initializers function compiler
-    let mut mi_fnc = FunctionCompiler::new(self, OwnerId::Span(class.span));
+    let mut mi_fnc = FunctionCompiler::new(self);
+    mi_fnc.set_owner_id(OwnerId::Span(class.span));
 
     for class_member in &class.body {
       match class_member {
@@ -779,10 +767,7 @@ impl ModuleCompiler {
     }
 
     let mut member_initializers_assembly = Vec::<FnLine>::new();
-    member_initializers_assembly.append(&mut mi_fnc.current.body);
-
-    // Include any other definitions that were created by the member initializers
-    mi_fnc.process_queue();
+    member_initializers_assembly.append(&mut mi_fnc.fn_.body);
 
     let mut has_constructor = false;
 
@@ -794,7 +779,6 @@ impl ModuleCompiler {
 
         self.compile_fn(
           ctor_defn_name.clone(),
-          None,
           Functionish::Constructor(
             member_initializers_assembly.clone(),
             class.span,
@@ -840,7 +824,6 @@ impl ModuleCompiler {
 
           self.compile_fn(
             method_defn_name.clone(),
-            None,
             Functionish::Fn(None, method.function.clone()),
           );
 
diff --git a/valuescript_compiler/src/static_expression_compiler.rs b/valuescript_compiler/src/static_expression_compiler.rs
index e251cdf..2c06a66 100644
--- a/valuescript_compiler/src/static_expression_compiler.rs
+++ b/valuescript_compiler/src/static_expression_compiler.rs
@@ -110,7 +110,6 @@ impl<'a> StaticExpressionCompiler<'a> {
 
                 self.mc.compile_fn(
                   p.clone(),
-                  fn_name.clone(),
                   Functionish::Fn(fn_ident, method.function.clone()),
                 );
 
@@ -161,7 +160,6 @@ impl<'a> StaticExpressionCompiler<'a> {
 
         self.mc.compile_fn(
           p.clone(),
-          fn_name,
           Functionish::Fn(fn_.ident.clone(), fn_.function.clone()),
         );
 
@@ -172,7 +170,7 @@ impl<'a> StaticExpressionCompiler<'a> {
 
         self
           .mc
-          .compile_fn(p.clone(), None, Functionish::Arrow(arrow.clone()));
+          .compile_fn(p.clone(), Functionish::Arrow(arrow.clone()));
 
         Value::Pointer(p)
       }