Skip to content

Commit

Permalink
More Strings replaced with &strs (#579)
Browse files Browse the repository at this point in the history
Removed large number of allocations by borrowing strings from the input where possible
Note that this generally means the input must actually be longer lived than the evaluated stuff (not a problem except in testing, evidently)
  • Loading branch information
rben01 authored Sep 26, 2024
1 parent 3e06299 commit 1d4324f
Show file tree
Hide file tree
Showing 16 changed files with 390 additions and 351 deletions.
119 changes: 61 additions & 58 deletions numbat/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,47 +56,52 @@ impl PrettyPrint for BinaryOperator {
}

#[derive(Debug, Clone, PartialEq)]
pub enum StringPart {
pub enum StringPart<'a> {
Fixed(String),
Interpolation {
span: Span,
expr: Box<Expression>,
expr: Box<Expression<'a>>,
format_specifiers: Option<String>,
},
}

#[derive(Debug, Clone, PartialEq)]
pub enum Expression {
pub enum Expression<'a> {
Scalar(Span, Number),
Identifier(Span, String),
Identifier(Span, &'a str),
UnitIdentifier(Span, Prefix, String, String),
TypedHole(Span),
UnaryOperator {
op: UnaryOperator,
expr: Box<Expression>,
expr: Box<Expression<'a>>,
span_op: Span,
},
BinaryOperator {
op: BinaryOperator,
lhs: Box<Expression>,
rhs: Box<Expression>,
lhs: Box<Expression<'a>>,
rhs: Box<Expression<'a>>,
span_op: Option<Span>, // not available for implicit multiplication and unicode exponents
},
FunctionCall(Span, Span, Box<Expression>, Vec<Expression>),
FunctionCall(Span, Span, Box<Expression<'a>>, Vec<Expression<'a>>),
Boolean(Span, bool),
String(Span, Vec<StringPart>),
Condition(Span, Box<Expression>, Box<Expression>, Box<Expression>),
String(Span, Vec<StringPart<'a>>),
Condition(
Span,
Box<Expression<'a>>,
Box<Expression<'a>>,
Box<Expression<'a>>,
),
InstantiateStruct {
full_span: Span,
ident_span: Span,
name: String,
fields: Vec<(Span, String, Expression)>,
name: &'a str,
fields: Vec<(Span, &'a str, Expression<'a>)>,
},
AccessField(Span, Span, Box<Expression>, String),
List(Span, Vec<Expression>),
AccessField(Span, Span, Box<Expression<'a>>, &'a str),
List(Span, Vec<Expression<'a>>),
}

impl Expression {
impl Expression<'_> {
pub fn full_span(&self) -> Span {
match self {
Expression::Scalar(span, _) => *span,
Expand Down Expand Up @@ -217,9 +222,9 @@ macro_rules! struct_ {
crate::ast::Expression::InstantiateStruct {
full_span: Span::dummy(),
ident_span: Span::dummy(),
name: stringify!($name).to_owned(),
name: stringify!($name),
fields: vec![
$((Span::dummy(), stringify!($field).to_owned(), $val)),*
$((Span::dummy(), stringify!($field), $val)),*
]
}
}};
Expand Down Expand Up @@ -394,48 +399,48 @@ pub enum TypeParameterBound {
}

#[derive(Debug, Clone, PartialEq)]
pub struct DefineVariable {
pub struct DefineVariable<'a> {
pub identifier_span: Span,
pub identifier: String,
pub expr: Expression,
pub identifier: &'a str,
pub expr: Expression<'a>,
pub type_annotation: Option<TypeAnnotation>,
pub decorators: Vec<Decorator>,
pub decorators: Vec<Decorator<'a>>,
}

#[derive(Debug, Clone, PartialEq)]
pub enum Statement {
Expression(Expression),
DefineVariable(DefineVariable),
pub enum Statement<'a> {
Expression(Expression<'a>),
DefineVariable(DefineVariable<'a>),
DefineFunction {
function_name_span: Span,
function_name: String,
type_parameters: Vec<(Span, String, Option<TypeParameterBound>)>,
function_name: &'a str,
type_parameters: Vec<(Span, &'a str, Option<TypeParameterBound>)>,
/// Parameters, optionally with type annotations.
parameters: Vec<(Span, String, Option<TypeAnnotation>)>,
parameters: Vec<(Span, &'a str, Option<TypeAnnotation>)>,
/// Function body. If it is absent, the function is implemented via FFI
body: Option<Expression>,
body: Option<Expression<'a>>,
/// Local variables
local_variables: Vec<DefineVariable>,
local_variables: Vec<DefineVariable<'a>>,
/// Optional annotated return type
return_type_annotation: Option<TypeAnnotation>,
decorators: Vec<Decorator>,
decorators: Vec<Decorator<'a>>,
},
DefineDimension(Span, String, Vec<TypeExpression>),
DefineBaseUnit(Span, String, Option<TypeExpression>, Vec<Decorator>),
DefineDimension(Span, &'a str, Vec<TypeExpression>),
DefineBaseUnit(Span, &'a str, Option<TypeExpression>, Vec<Decorator<'a>>),
DefineDerivedUnit {
identifier_span: Span,
identifier: String,
expr: Expression,
identifier: &'a str,
expr: Expression<'a>,
type_annotation_span: Option<Span>,
type_annotation: Option<TypeAnnotation>,
decorators: Vec<Decorator>,
decorators: Vec<Decorator<'a>>,
},
ProcedureCall(Span, ProcedureKind, Vec<Expression>),
ProcedureCall(Span, ProcedureKind, Vec<Expression<'a>>),
ModuleImport(Span, ModulePath),
DefineStruct {
struct_name_span: Span,
struct_name: String,
fields: Vec<(Span, String, TypeAnnotation)>,
struct_name: &'a str,
fields: Vec<(Span, &'a str, TypeAnnotation)>,
},
}

Expand Down Expand Up @@ -491,7 +496,7 @@ impl ReplaceSpans for TypeExpression {
}

#[cfg(test)]
impl ReplaceSpans for StringPart {
impl ReplaceSpans for StringPart<'_> {
fn replace_spans(&self) -> Self {
match self {
f @ StringPart::Fixed(_) => f.clone(),
Expand All @@ -509,11 +514,11 @@ impl ReplaceSpans for StringPart {
}

#[cfg(test)]
impl ReplaceSpans for Expression {
impl ReplaceSpans for Expression<'_> {
fn replace_spans(&self) -> Self {
match self {
Expression::Scalar(_, name) => Expression::Scalar(Span::dummy(), *name),
Expression::Identifier(_, name) => Expression::Identifier(Span::dummy(), name.clone()),
Expression::Identifier(_, name) => Expression::Identifier(Span::dummy(), name),
Expression::UnitIdentifier(_, prefix, name, full_name) => {
Expression::UnitIdentifier(Span::dummy(), *prefix, name.clone(), full_name.clone())
}
Expand Down Expand Up @@ -557,17 +562,17 @@ impl ReplaceSpans for Expression {
Expression::InstantiateStruct { name, fields, .. } => Expression::InstantiateStruct {
full_span: Span::dummy(),
ident_span: Span::dummy(),
name: name.clone(),
name,
fields: fields
.iter()
.map(|(_, n, v)| (Span::dummy(), n.clone(), v.replace_spans()))
.map(|(_, n, v)| (Span::dummy(), *n, v.replace_spans()))
.collect(),
},
Expression::AccessField(_, _, expr, attr) => Expression::AccessField(
Span::dummy(),
Span::dummy(),
Box::new(expr.replace_spans()),
attr.clone(),
attr,
),
Expression::List(_, elements) => Expression::List(
Span::dummy(),
Expand All @@ -579,11 +584,11 @@ impl ReplaceSpans for Expression {
}

#[cfg(test)]
impl ReplaceSpans for DefineVariable {
impl ReplaceSpans for DefineVariable<'_> {
fn replace_spans(&self) -> Self {
Self {
identifier_span: Span::dummy(),
identifier: self.identifier.clone(),
identifier: self.identifier,
expr: self.expr.replace_spans(),
type_annotation: self.type_annotation.as_ref().map(|t| t.replace_spans()),
decorators: self.decorators.clone(),
Expand All @@ -592,7 +597,7 @@ impl ReplaceSpans for DefineVariable {
}

#[cfg(test)]
impl ReplaceSpans for Statement {
impl ReplaceSpans for Statement<'_> {
fn replace_spans(&self) -> Self {
match self {
Statement::Expression(expr) => Statement::Expression(expr.replace_spans()),
Expand All @@ -610,17 +615,17 @@ impl ReplaceSpans for Statement {
decorators,
} => Statement::DefineFunction {
function_name_span: Span::dummy(),
function_name: function_name.clone(),
function_name,
type_parameters: type_parameters
.iter()
.map(|(_, name, bound)| (Span::dummy(), name.clone(), bound.clone()))
.map(|(_, name, bound)| (Span::dummy(), *name, bound.clone()))
.collect(),
parameters: parameters
.iter()
.map(|(_, name, type_)| {
(
Span::dummy(),
name.clone(),
*name,
type_.as_ref().map(|t| t.replace_spans()),
)
})
Expand All @@ -635,12 +640,12 @@ impl ReplaceSpans for Statement {
},
Statement::DefineDimension(_, name, dexprs) => Statement::DefineDimension(
Span::dummy(),
name.clone(),
name,
dexprs.iter().map(|t| t.replace_spans()).collect(),
),
Statement::DefineBaseUnit(_, name, type_, decorators) => Statement::DefineBaseUnit(
Span::dummy(),
name.clone(),
name,
type_.as_ref().map(|t| t.replace_spans()),
decorators.clone(),
),
Expand All @@ -653,7 +658,7 @@ impl ReplaceSpans for Statement {
decorators,
} => Statement::DefineDerivedUnit {
identifier_span: Span::dummy(),
identifier: identifier.clone(),
identifier,
expr: expr.replace_spans(),
type_annotation_span: type_annotation_span.map(|_| Span::dummy()),
type_annotation: type_annotation.as_ref().map(|t| t.replace_spans()),
Expand All @@ -673,20 +678,18 @@ impl ReplaceSpans for Statement {
..
} => Statement::DefineStruct {
struct_name_span: Span::dummy(),
struct_name: struct_name.clone(),
struct_name,
fields: fields
.iter()
.map(|(_span, name, type_)| {
(Span::dummy(), name.clone(), type_.replace_spans())
})
.map(|(_span, name, type_)| (Span::dummy(), *name, type_.replace_spans()))
.collect(),
},
}
}
}

#[cfg(test)]
impl ReplaceSpans for Vec<Statement> {
impl ReplaceSpans for Vec<Statement<'_>> {
fn replace_spans(&self) -> Self {
self.iter().map(|s| s.replace_spans()).collect()
}
Expand Down
19 changes: 9 additions & 10 deletions numbat/src/bytecode_interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,11 @@ impl BytecodeInterpreter {

// For variables, we ignore the prefix info and only use the names
let aliases = crate::decorator::name_and_aliases(identifier, decorators)
.map(|(name, _)| name)
.cloned()
.map(|(name, _)| name.to_owned())
.collect::<Vec<_>>();
let metadata = LocalMetadata {
name: crate::decorator::name(decorators),
url: crate::decorator::url(decorators),
name: crate::decorator::name(decorators).map(ToOwned::to_owned),
url: crate::decorator::url(decorators).map(ToOwned::to_owned),
description: crate::decorator::description(decorators),
aliases: aliases.clone(),
};
Expand Down Expand Up @@ -380,7 +379,7 @@ impl BytecodeInterpreter {
}
Statement::DefineBaseUnit(unit_name, decorators, annotation, type_) => {
let aliases = decorator::name_and_aliases(unit_name, decorators)
.map(|(name, ap)| (name.clone(), ap))
.map(|(name, ap)| (name.to_owned(), ap))
.collect();

self.vm
Expand All @@ -394,11 +393,11 @@ impl BytecodeInterpreter {
.map(|a| a.pretty_print())
.unwrap_or(type_.to_readable_type(dimension_registry, false)),
aliases,
name: decorator::name(decorators),
name: decorator::name(decorators).map(ToOwned::to_owned),
canonical_name: decorator::get_canonical_unit_name(
unit_name, decorators,
),
url: decorator::url(decorators),
url: decorator::url(decorators).map(ToOwned::to_owned),
description: decorator::description(decorators),
binary_prefixes: decorators.contains(&Decorator::BinaryPrefixes),
metric_prefixes: decorators.contains(&Decorator::MetricPrefixes),
Expand All @@ -424,7 +423,7 @@ impl BytecodeInterpreter {
_readable_type,
) => {
let aliases = decorator::name_and_aliases(unit_name, decorators)
.map(|(name, ap)| (name.clone(), ap))
.map(|(name, ap)| (name.to_owned(), ap))
.collect();

let constant_idx = self.vm.add_constant(Constant::Unit(Unit::new_base(
Expand All @@ -450,9 +449,9 @@ impl BytecodeInterpreter {
.map(|a| a.pretty_print())
.unwrap_or(type_.to_readable_type(dimension_registry, false)),
aliases,
name: decorator::name(decorators),
name: decorator::name(decorators).map(ToOwned::to_owned),
canonical_name: decorator::get_canonical_unit_name(unit_name, decorators),
url: decorator::url(decorators),
url: decorator::url(decorators).map(ToOwned::to_owned),
description: decorator::description(decorators),
binary_prefixes: decorators.contains(&Decorator::BinaryPrefixes),
metric_prefixes: decorators.contains(&Decorator::MetricPrefixes),
Expand Down
Loading

0 comments on commit 1d4324f

Please sign in to comment.