From 62aaa57b7d64dda588e7728a1f2be84534aa2a4c Mon Sep 17 00:00:00 2001 From: Andrew Morris Date: Thu, 22 Jun 2023 17:46:05 +1000 Subject: [PATCH] Fix unnecessary binds --- valuescript_compiler/src/asm.rs | 2 +- .../src/expression_compiler.rs | 32 ++++++++----------- valuescript_compiler/src/function_compiler.rs | 27 ++++++++++++---- valuescript_compiler/src/scope.rs | 2 +- valuescript_compiler/src/scope_analysis.rs | 28 ++++++++++++++++ 5 files changed, 63 insertions(+), 28 deletions(-) diff --git a/valuescript_compiler/src/asm.rs b/valuescript_compiler/src/asm.rs index c8b1492..9bbceb9 100644 --- a/valuescript_compiler/src/asm.rs +++ b/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, } diff --git a/valuescript_compiler/src/expression_compiler.rs b/valuescript_compiler/src/expression_compiler.rs index 9717d58..ff30440 100644 --- a/valuescript_compiler/src/expression_compiler.rs +++ b/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, fn_value: &Value, - captures: &HashSet, + captures: &Vec, target_register: Option, ) -> CompiledExpression { let mut nested_registers = Vec::::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 { diff --git a/valuescript_compiler/src/function_compiler.rs b/valuescript_compiler/src/function_compiler.rs index 62da6d8..46e7c7f 100644 --- a/valuescript_compiler/src/function_compiler.rs +++ b/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()); diff --git a/valuescript_compiler/src/scope.rs b/valuescript_compiler/src/scope.rs index aa27f18..ebf879e 100644 --- a/valuescript_compiler/src/scope.rs +++ b/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), diff --git a/valuescript_compiler/src/scope_analysis.rs b/valuescript_compiler/src/scope_analysis.rs index a057118..e3009d8 100644 --- a/valuescript_compiler/src/scope_analysis.rs +++ b/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 { + // 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::::new(); + let mut res = Vec::::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 {