Merge pull request #607 from rben01/code-cleanup

Reduced allocations relating to `ast::Expression` + `typed_ast::Expression` and reduced allocations in `Transformer::transform_*` functions

numbat/src/prefix_transformer.rs

@@ -31,9 +31,9 @@ impl Transformer {
         }
     }
 
-    fn transform_expression<'a>(&self, expression: Expression<'a>) -> Expression<'a> {
+    fn transform_expression(&self, expression: &mut Expression) {
         match expression {
-            expr @ Expression::Scalar(..) => expr,
+            Expression::Scalar(..) | Expression::Boolean(_, _) | Expression::TypedHole(_) => {}
             Expression::Identifier(span, identifier) => {
                 if let PrefixParserResult::UnitIdentifier(
                     _definition_span,
@@ -42,91 +42,51 @@ impl Transformer {
                     full_name,
                 ) = self.prefix_parser.parse(identifier)
                 {
-                    Expression::UnitIdentifier(span, prefix, unit_name, full_name)
+                    *expression = Expression::UnitIdentifier(*span, prefix, unit_name, full_name);
                 } else {
-                    Expression::Identifier(span, identifier)
+                    *expression = Expression::Identifier(*span, identifier);
                 }
             }
             Expression::UnitIdentifier(_, _, _, _) => {
                 unreachable!("Prefixed identifiers should not exist prior to this stage")
             }
-            Expression::UnaryOperator { op, expr, span_op } => Expression::UnaryOperator {
-                op,
-                expr: Box::new(self.transform_expression(*expr)),
-                span_op,
-            },
-            Expression::BinaryOperator {
-                op,
-                lhs,
-                rhs,
-                span_op,
-            } => Expression::BinaryOperator {
-                op,
-                lhs: Box::new(self.transform_expression(*lhs)),
-                rhs: Box::new(self.transform_expression(*rhs)),
-                span_op,
-            },
-            Expression::FunctionCall(span, full_span, name, args) => Expression::FunctionCall(
-                span,
-                full_span,
-                name,
-                args.into_iter()
-                    .map(|arg| self.transform_expression(arg))
-                    .collect(),
-            ),
-            expr @ Expression::Boolean(_, _) => expr,
-            Expression::Condition(span, condition, then, else_) => Expression::Condition(
-                span,
-                Box::new(self.transform_expression(*condition)),
-                Box::new(self.transform_expression(*then)),
-                Box::new(self.transform_expression(*else_)),
-            ),
-            Expression::String(span, parts) => Expression::String(
-                span,
-                parts
-                    .into_iter()
-                    .map(|p| match p {
-                        f @ StringPart::Fixed(_) => f,
-                        StringPart::Interpolation {
-                            span,
-                            expr,
-                            format_specifiers,
-                        } => StringPart::Interpolation {
-                            span,
-                            expr: Box::new(self.transform_expression(*expr)),
-                            format_specifiers,
-                        },
-                    })
-                    .collect(),
-            ),
-            Expression::InstantiateStruct {
-                full_span,
-                ident_span,
-                name,
-                fields,
-            } => Expression::InstantiateStruct {
-                full_span,
-                ident_span,
-                name,
-                fields: fields
-                    .into_iter()
-                    .map(|(span, attr, arg)| (span, attr, self.transform_expression(arg)))
-                    .collect(),
-            },
-            Expression::AccessField(full_span, ident_span, expr, attr) => Expression::AccessField(
-                full_span,
-                ident_span,
-                Box::new(self.transform_expression(*expr)),
-                attr,
-            ),
-            Expression::List(span, elements) => Expression::List(
-                span,
-                elements
-                    .into_iter()
-                    .map(|e| self.transform_expression(e))
-                    .collect(),
-            ),
-            hole @ Expression::TypedHole(_) => hole,
+            Expression::UnaryOperator { expr, .. } => self.transform_expression(expr),
+
+            Expression::BinaryOperator { lhs, rhs, .. } => {
+                self.transform_expression(lhs);
+                self.transform_expression(rhs);
+            }
+            Expression::FunctionCall(_, _, _, args) => {
+                for arg in args {
+                    self.transform_expression(arg);
+                }
+            }
+            Expression::Condition(_, condition, then_expr, else_expr) => {
+                self.transform_expression(condition);
+                self.transform_expression(then_expr);
+                self.transform_expression(else_expr);
+            }
+            Expression::String(_, parts) => {
+                for p in parts {
+                    match p {
+                        StringPart::Fixed(_) => {}
+                        StringPart::Interpolation { expr, .. } => self.transform_expression(expr),
+                    }
+                }
+            }
+            Expression::InstantiateStruct { fields, .. } => {
+                for (_, _, arg) in fields {
+                    self.transform_expression(arg);
+                }
+            }
+            Expression::AccessField(_, _, expr, _) => {
+                self.transform_expression(expr);
+            }
+            Expression::List(_, elements) => {
+                for e in elements {
+                    self.transform_expression(e);
+                }
+            }
         }
     }
 
@@ -167,73 +127,59 @@ impl Transformer {
         Ok(())
     }
 
-    fn transform_define_variable<'a>(
-        &mut self,
-        define_variable: DefineVariable<'a>,
-    ) -> Result<DefineVariable<'a>> {
+    fn transform_define_variable(&mut self, define_variable: &mut DefineVariable) -> Result<()> {
         let DefineVariable {
             identifier_span,
             identifier,
             expr,
-            type_annotation,
+            type_annotation: _,
             decorators,
         } = define_variable;
 
-        for (name, _) in decorator::name_and_aliases(identifier, &decorators) {
+        for (name, _) in decorator::name_and_aliases(identifier, decorators) {
             self.variable_names.push(name.to_compact_string());
         }
         self.prefix_parser
-            .add_other_identifier(identifier, identifier_span)?;
-        Ok(DefineVariable {
-            identifier_span,
-            identifier,
-            expr: self.transform_expression(expr),
-            type_annotation,
-            decorators,
-        })
+            .add_other_identifier(identifier, *identifier_span)?;
+        self.transform_expression(expr);
+
+        Ok(())
     }
 
-    fn transform_statement<'a>(&mut self, statement: Statement<'a>) -> Result<Statement<'a>> {
-        Ok(match statement {
-            Statement::Expression(expr) => Statement::Expression(self.transform_expression(expr)),
-            Statement::DefineBaseUnit(span, name, dexpr, decorators) => {
-                self.register_name_and_aliases(name, span, &decorators)?;
-                Statement::DefineBaseUnit(span, name, dexpr, decorators)
+    fn transform_statement(&mut self, statement: &mut Statement) -> Result<()> {
+        match statement {
+            Statement::DefineStruct { .. } | Statement::ModuleImport(_, _) => {}
+
+            Statement::Expression(expr) => {
+                self.transform_expression(expr);
+            }
+            Statement::DefineBaseUnit(span, name, _, decorators) => {
+                self.register_name_and_aliases(name, *span, decorators)?;
             }
             Statement::DefineDerivedUnit {
                 identifier_span,
                 identifier,
                 expr,
-                type_annotation_span,
-                type_annotation,
                 decorators,
+                ..
             } => {
-                self.register_name_and_aliases(identifier, identifier_span, &decorators)?;
-                Statement::DefineDerivedUnit {
-                    identifier_span,
-                    identifier,
-                    expr: self.transform_expression(expr),
-                    type_annotation_span,
-                    type_annotation,
-                    decorators,
-                }
+                self.register_name_and_aliases(identifier, *identifier_span, decorators)?;
+                self.transform_expression(expr);
             }
             Statement::DefineVariable(define_variable) => {
-                Statement::DefineVariable(self.transform_define_variable(define_variable)?)
+                self.transform_define_variable(define_variable)?
             }
             Statement::DefineFunction {
                 function_name_span,
                 function_name,
-                type_parameters,
                 parameters,
                 body,
                 local_variables,
-                return_type_annotation,
-                decorators,
+                ..
             } => {
                 self.function_names.push(function_name.to_compact_string());
                 self.prefix_parser
-                    .add_other_identifier(function_name, function_name_span)?;
+                    .add_other_identifier(function_name, *function_name_span)?;
 
                 // We create a clone of the full transformer for the purpose
                 // of checking/transforming the function body. The reason for this
@@ -244,48 +190,31 @@ impl Transformer {
                 //   fn foo(t: Time) -> Time = t    # not okay: shadows 't' for ton
                 //
                 let mut fn_body_transformer = self.clone();
-                for (param_span, param, _) in &parameters {
+                for (param_span, param, _) in &*parameters {
                     fn_body_transformer
                         .prefix_parser
                         .add_other_identifier(param, *param_span)?;
                 }
 
-                Statement::DefineFunction {
-                    function_name_span,
-                    function_name,
-                    type_parameters,
-                    parameters,
-                    body: body.map(|expr| self.transform_expression(expr)),
-                    local_variables: local_variables
-                        .into_iter()
-                        .map(|def| self.transform_define_variable(def))
-                        .collect::<Result<_>>()?,
-                    return_type_annotation,
-                    decorators,
+                if let Some(expr) = body {
+                    self.transform_expression(expr);
+                }
+
+                for def in local_variables {
+                    self.transform_define_variable(def)?;
                 }
             }
-            Statement::DefineStruct {
-                struct_name_span,
-                struct_name,
-                fields,
-            } => Statement::DefineStruct {
-                struct_name_span,
-                struct_name,
-                fields,
-            },
-            Statement::DefineDimension(name_span, name, dexprs) => {
+            Statement::DefineDimension(_, name, _) => {
                 self.dimension_names.push(name.to_compact_string());
-                Statement::DefineDimension(name_span, name, dexprs)
             }
-            Statement::ProcedureCall(span, procedure, args) => Statement::ProcedureCall(
-                span,
-                procedure,
-                args.into_iter()
-                    .map(|arg| self.transform_expression(arg))
-                    .collect(),
-            ),
-            statement @ Statement::ModuleImport(_, _) => statement,
-        })
+            Statement::ProcedureCall(_, _, args) => {
+                for arg in args {
+                    self.transform_expression(arg);
+                }
+            }
+        }
+
+        Ok(())
     }
 
     pub fn transform<'a>(
@@ -294,7 +223,10 @@ impl Transformer {
     ) -> Result<Vec<Statement<'a>>> {
         statements
             .into_iter()
-            .map(|statement| self.transform_statement(statement))
+            .map(|mut statement| {
+                self.transform_statement(&mut statement)?;
+                Ok(statement)
+            })
             .collect()
     }
 }

numbat/src/typed_ast.rs

@@ -315,7 +315,7 @@ impl std::fmt::Display for Type {
                 )
             }
             Type::Struct(info) => {
-                let StructInfo { name, fields, .. } = &**info;
+                let StructInfo { name, fields, .. } = info.as_ref();
                 write!(
                     f,
                     "{name} {{{}}}",