Fix unnecessary binds

valuescript_compiler/src/asm.rs

@@ -677,7 +677,7 @@ impl std::fmt::Display for Lazy {
   }
 }
 
-#[derive(Debug, Clone, Hash, PartialEq, Eq)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Builtin {
   pub name: String,
 }

valuescript_compiler/src/expression_compiler.rs

@@ -1,8 +1,6 @@
-use std::collections::HashSet;
 use std::mem::take;
 
 use queues::*;
-
 use swc_common::Spanned;
 
 use crate::asm::{Array, Builtin, Instruction, Label, Object, Register, Value};
@@ -942,9 +940,7 @@ impl<'a> ExpressionCompiler<'a> {
     let capture_params = self
       .fnc
       .scope_analysis
-      .captures
-      .get(&fn_to_owner_id(&fn_.ident, &fn_.function))
-      .cloned();
+      .get_register_captures(&fn_to_owner_id(&fn_.ident, &fn_.function));
 
     self
       .fnc
@@ -956,9 +952,9 @@ impl<'a> ExpressionCompiler<'a> {
       })
       .expect("Failed to queue function");
 
-    match capture_params {
-      None => Value::Pointer(definition_pointer).to_ce(),
-      Some(capture_params) => self.capturing_fn_ref(
+    match capture_params.len() {
+      0 => Value::Pointer(definition_pointer).to_ce(),
+      _ => self.capturing_fn_ref(
         match fn_.ident {
           Some(ref ident) => ident.span,
           None => fn_.function.span,
@@ -981,9 +977,7 @@ impl<'a> ExpressionCompiler<'a> {
     let capture_params = self
       .fnc
       .scope_analysis
-      .captures
-      .get(&OwnerId::Span(arrow_expr.span))
-      .cloned();
+      .get_register_captures(&OwnerId::Span(arrow_expr.span));
 
     self
       .fnc
@@ -995,9 +989,9 @@ impl<'a> ExpressionCompiler<'a> {
       })
       .expect("Failed to queue function");
 
-    match capture_params {
-      None => Value::Pointer(definition_pointer).to_ce(),
-      Some(capture_params) => self.capturing_fn_ref(
+    match capture_params.len() {
+      0 => Value::Pointer(definition_pointer).to_ce(),
+      _ => self.capturing_fn_ref(
         arrow_expr.span,
         None,
         &Value::Pointer(definition_pointer),
@@ -1012,7 +1006,7 @@ impl<'a> ExpressionCompiler<'a> {
     span: swc_common::Span,
     fn_name: Option<String>,
     fn_value: &Value,
-    captures: &HashSet<NameId>,
+    captures: &Vec<NameId>,
     target_register: Option<Register>,
   ) -> CompiledExpression {
     let mut nested_registers = Vec::<Register>::new();
@@ -1360,17 +1354,17 @@ impl<'a> ExpressionCompiler<'a> {
 
     match fn_as_owner_id {
       Some(owner_id) => {
-        let capture_params = self.fnc.scope_analysis.captures.get(&owner_id).cloned();
+        let capture_params = self.fnc.scope_analysis.get_register_captures(&owner_id);
 
-        match capture_params {
-          Some(capture_params) => self.capturing_fn_ref(
+        match capture_params.len() {
+          0 => value.to_ce(),
+          _ => self.capturing_fn_ref(
             ident.span,
             Some(ident.sym.to_string()),
             &value,
             &capture_params,
             target_register,
           ),
-          None => value.to_ce(),
         }
       }
       None => match value {

valuescript_compiler/src/function_compiler.rs

@@ -162,6 +162,14 @@ impl FunctionCompiler {
     });
   }
 
+  pub fn internal_error(&mut self, span: swc_common::Span, message: &str) {
+    self.diagnostics.push(Diagnostic {
+      level: DiagnosticLevel::InternalError,
+      message: format!("{}", message),
+      span,
+    });
+  }
+
   pub fn allocate_defn(&mut self, name: &str) -> Pointer {
     let allocated_name = self
       .definition_allocator
@@ -268,18 +276,23 @@ impl FunctionCompiler {
 
     self.owner_id = functionish.owner_id();
 
-    let capture_params = self.scope_analysis.captures.get(&functionish.owner_id());
+    let capture_params = self
+      .scope_analysis
+      .get_register_captures(&functionish.owner_id());
 
-    for cap_param in capture_params.unwrap_or(&HashSet::new()) {
+    for cap_param in capture_params {
       let reg = match self
         .scope_analysis
-        .lookup_capture(&self.owner_id, cap_param)
+        .lookup_capture(&self.owner_id, &cap_param)
       {
         Some(Value::Register(reg)) => reg,
-
-        // Technically we can capture definitions, and maybe in future variables that have been
-        // reduced to constant values. We can just ignore these here.
-        _ => continue,
+        _ => {
+          self.internal_error(
+            cap_param.span(),
+            "Unexpected non-register in captured_registers",
+          );
+          continue;
+        }
       };
 
       self.current.parameters.push(reg.clone());

valuescript_compiler/src/scope.rs

@@ -7,7 +7,7 @@ use valuescript_common::BUILTIN_NAMES;
 use crate::diagnostic::{Diagnostic, DiagnosticLevel};
 use crate::{asm::Builtin, constants::CONSTANTS};
 
-#[derive(Hash, PartialEq, Eq, Clone, Debug)]
+#[derive(Hash, PartialEq, Eq, Clone, Debug, PartialOrd, Ord)]
 pub enum NameId {
   Span(swc_common::Span),
   Builtin(Builtin),

valuescript_compiler/src/scope_analysis.rs

@@ -2127,6 +2127,34 @@ impl ScopeAnalysis {
 
     self.diagnostics.append(&mut diagnostics);
   }
+
+  pub fn get_register_captures(&self, captor_id: &OwnerId) -> Vec<NameId> {
+    // It's important to return NameId and not the registers here. The register that is needed
+    // depends on the scope.
+    //
+    // Remember: A NameId usually maps to the same register everywhere, but that's not always true.
+    // Resolving to a register must always depend on scope so that conflicting names can be
+    // disambiguated. Eg `inputs/passing/captureShadowed.ts`.
+
+    let mut seen = HashSet::<NameId>::new();
+    let mut res = Vec::<NameId>::new();
+
+    if let Some(caps) = self.captures.get(captor_id) {
+      for cap in caps {
+        if let Value::Register(_) = self.names.get(cap).expect("Failed to get name").value {
+          if seen.insert(cap.clone()) {
+            res.push(cap.clone());
+          }
+        }
+      }
+    }
+
+    // Ensure a consistent ordering. It's also nice to include the captures in the order that they
+    // appear in the source code.
+    res.sort();
+
+    res
+  }
 }
 
 fn is_declare(decl: &swc_ecma_ast::Decl) -> bool {