Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builtin lines function part I - Refactoring #565

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Builtin lines function part I - Refactoring #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/modules/expression/binop/add.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,13 @@ impl TranslateModule for Add {
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let left = self.left.translate_eval(meta, false);
let right = self.right.translate_eval(meta, false);
let quote = meta.gen_quote();
match self.kind {
Type::Array(_) => {
let quote = meta.gen_quote();
let id = meta.gen_array_id();
meta.stmt_queue.push_back(format!("__AMBER_ARRAY_ADD_{id}=({left} {right})"));
format!("{quote}${{__AMBER_ARRAY_ADD_{id}[@]}}{quote}")
let name = format!("__AMBER_ARRAY_ADD_{id}");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to pass name into a new TranslateModule function in the next PR, so it makes sense to store it in a local variable here.

meta.stmt_queue.push_back(format!("{name}=({left} {right})"));
format!("{quote}${{{name}[@]}}{quote}")
},
Type::Text => format!("{}{}", left, right),
_ => translate_computation(meta, ArithOp::Add, Some(left), Some(right))
Expand Down
56 changes: 36 additions & 20 deletions src/modules/shorthand/add.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,31 +48,47 @@ impl SyntaxModule<ParserMetadata> for ShorthandAdd {
}

impl TranslateModule for ShorthandAdd {
//noinspection DuplicatedCode
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let expr = self.is_ref
.then(|| self.expr.translate_eval(meta, true))
.unwrap_or_else(|| self.expr.translate(meta));
let name: String = match self.global_id {
Some(id) => format!("__{id}_{}", self.var),
None => if self.is_ref { format!("${{{}}}", self.var) } else { self.var.clone() }
let name = if let Some(id) = self.global_id {
format!("__{id}_{}", self.var)
} else if self.is_ref {
format!("${{{}}}", self.var)
} else {
self.var.clone()
};
let stmt = match self.kind {
Type::Text => format!("{}+={}", name, expr),
Type::Array(_) => format!("{}+=({})", name, expr),
match self.kind {
Type::Text => {
if self.is_ref {
let expr = self.expr.translate_eval(meta, true);
format!("eval \"{name}+={expr}\"")
} else {
let expr = self.expr.translate(meta);
format!("{name}+={expr}")
}
}
Type::Array(_) => {
if self.is_ref {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I unrolled some of the code which was previously shared across expression types in this match statement; this is necessary so we can append additional code to the Array case in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also make it slightly easier to implement @Ph0enixKM's proposed new translation layer.

let expr = self.expr.translate_eval(meta, true);
format!("eval \"{name}+=({expr})\"")
} else {
let expr = self.expr.translate(meta);
format!("{name}+=({expr})")
}
}
_ => {
let var = if self.is_ref { format!("\\${{{name}}}") } else { format!("${{{name}}}") };
let translated_computation = if self.is_ref {
translate_computation_eval(meta, ArithOp::Add, Some(var), Some(expr))
if self.is_ref {
let var = format!("\\${{{name}}}");
let expr = self.expr.translate_eval(meta, true);
let expr = translate_computation_eval(meta, ArithOp::Add, Some(var), Some(expr));
format!("eval \"{name}={expr}\"")
} else {
translate_computation(meta, ArithOp::Add, Some(var), Some(expr))
};
format!("{}={}", name, translated_computation)
let var = format!("${{{name}}}");
let expr = self.expr.translate(meta);
let expr = translate_computation(meta, ArithOp::Add, Some(var), Some(expr));
format!("{name}={expr}")
}
}
};
if self.is_ref {
format!("eval \"{}\"", stmt)
} else {
stmt
}
}
}
Expand Down
26 changes: 15 additions & 11 deletions src/modules/shorthand/div.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,25 @@ impl SyntaxModule<ParserMetadata> for ShorthandDiv {
}

impl TranslateModule for ShorthandDiv {
//noinspection DuplicatedCode
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let expr = self.is_ref
.then(|| self.expr.translate_eval(meta, true))
.unwrap_or_else(|| self.expr.translate(meta));
let name = match self.global_id {
Some(id) => format!("__{id}_{}", self.var),
None => if self.is_ref { format!("${{{}}}", self.var) } else { self.var.clone() }
let name = if let Some(id) = self.global_id {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to match the shorthand add operator.

format!("__{id}_{}", self.var)
} else if self.is_ref {
format!("${{{}}}", self.var)
} else {
self.var.clone()
};
let var = if self.is_ref { format!("\\${{{name}}}") } else { format!("${{{name}}}") };
if self.is_ref {
let eval = translate_computation_eval(meta, ArithOp::Div, Some(var), Some(expr));
format!("eval \"{}={}\"", name, eval)
let var = format!("\\${{{name}}}");
let expr = self.expr.translate_eval(meta, true);
let expr = translate_computation_eval(meta, ArithOp::Div, Some(var), Some(expr));
format!("eval \"{name}={expr}\"")
} else {
let eval = translate_computation(meta, ArithOp::Div, Some(var), Some(expr));
format!("{}={}", name, eval)
let var = format!("${{{name}}}");
let expr = self.expr.translate(meta);
let expr = translate_computation(meta, ArithOp::Div, Some(var), Some(expr));
format!("{name}={expr}")
}
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/modules/shorthand/modulo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,25 @@ impl SyntaxModule<ParserMetadata> for ShorthandModulo {
}

impl TranslateModule for ShorthandModulo {
//noinspection DuplicatedCode
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let expr = self.is_ref
.then(|| self.expr.translate_eval(meta, true))
.unwrap_or_else(|| self.expr.translate(meta));
let name = match self.global_id {
Some(id) => format!("__{id}_{}", self.var),
None => if self.is_ref { format!("${{{}}}", self.var) } else { self.var.clone() }
let name = if let Some(id) = self.global_id {
format!("__{id}_{}", self.var)
} else if self.is_ref {
format!("${{{}}}", self.var)
} else {
self.var.clone()
};
let var = if self.is_ref { format!("\\${{{name}}}") } else { format!("${{{name}}}") };
if self.is_ref {
let var = format!("\\${{{name}}}");
let expr = self.expr.translate_eval(meta, true);
let expr = translate_computation_eval(meta, ArithOp::Modulo, Some(var), Some(expr));
format!("eval \"{}={}\"", name, expr)
format!("eval \"{name}={expr}\"")
} else {
let var = format!("${{{name}}}");
let expr = self.expr.translate(meta);
let expr = translate_computation(meta, ArithOp::Modulo, Some(var), Some(expr));
format!("{}={}", name, expr)
format!("{name}={expr}")
}
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/modules/shorthand/mul.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,25 @@ impl SyntaxModule<ParserMetadata> for ShorthandMul {
}

impl TranslateModule for ShorthandMul {
//noinspection DuplicatedCode
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let expr = self.is_ref
.then(|| self.expr.translate_eval(meta, true))
.unwrap_or_else(|| self.expr.translate(meta));
let name = match self.global_id {
Some(id) => format!("__{id}_{}", self.var),
None => if self.is_ref { format!("${{{}}}", self.var) } else { self.var.clone() }
let name = if let Some(id) = self.global_id {
format!("__{id}_{}", self.var)
} else if self.is_ref {
format!("${{{}}}", self.var)
} else {
self.var.clone()
};
let var = if self.is_ref { format!("\\${{{name}}}") } else { format!("${{{name}}}") };
if self.is_ref {
let var = format!("\\${{{name}}}");
let expr = self.expr.translate_eval(meta, true);
let expr = translate_computation_eval(meta, ArithOp::Mul, Some(var), Some(expr));
format!("eval \"{}={}\"", name, expr)
format!("eval \"{name}={expr}\"")
} else {
let var = format!("${{{name}}}");
let expr = self.expr.translate(meta);
let expr = translate_computation(meta, ArithOp::Mul, Some(var), Some(expr));
format!("{}={}", name, expr)
format!("{name}={expr}")
}
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/modules/shorthand/sub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,25 @@ impl SyntaxModule<ParserMetadata> for ShorthandSub {
}

impl TranslateModule for ShorthandSub {
//noinspection DuplicatedCode
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let expr = self.is_ref
.then(|| self.expr.translate_eval(meta, true))
.unwrap_or_else(|| self.expr.translate(meta));
let name = match self.global_id {
Some(id) => format!("__{id}_{}", self.var),
None => if self.is_ref { format!("${{{}}}", self.var) } else { self.var.clone() }
let name = if let Some(id) = self.global_id {
format!("__{id}_{}", self.var)
} else if self.is_ref {
format!("${{{}}}", self.var)
} else {
self.var.clone()
};
let var = if self.is_ref { format!("\\${{{name}}}") } else { format!("${{{name}}}") };
if self.is_ref {
let var = format!("\\${{{name}}}");
let expr = self.expr.translate_eval(meta, true);
let expr = translate_computation_eval(meta, ArithOp::Sub, Some(var), Some(expr));
format!("eval \"{}={}\"", name, expr)
format!("eval \"{name}={expr}\"")
} else {
let var = format!("${{{name}}}");
let expr = self.expr.translate(meta);
let expr = translate_computation(meta, ArithOp::Sub, Some(var), Some(expr));
format!("{}={}", name, expr)
format!("{name}={expr}")
}
}
}
Expand Down
12 changes: 7 additions & 5 deletions src/modules/variable/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,16 @@ impl SyntaxModule<ParserMetadata> for VariableInit {
impl TranslateModule for VariableInit {
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let name = self.name.clone();
let mut expr = self.expr.translate(meta);
let mut expr = self.expr.translate(meta);
if let Type::Array(_) = self.expr.get_type() {
expr = format!("({expr})");
}
let local = if self.is_fun_ctx { "local " } else { "" };
match self.global_id {
Some(id) => format!("__{id}_{name}={expr}"),
None => format!("{local}{name}={expr}")
if let Some(id) = self.global_id {
Copy link
Contributor Author

@hdwalters hdwalters Nov 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We previously tested self.global_id after self.is_fun_ctx, but I switched the order to match the shorthand operators. I think this is safe, because global_id and is_fun_ctx cannot both be enabled at the same time.

format!("__{id}_{name}={expr}")
} else if self.is_fun_ctx {
format!("local {name}={expr}")
} else {
format!("{name}={expr}")
}
}
}
Expand Down
46 changes: 26 additions & 20 deletions src/modules/variable/set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,29 @@ use crate::modules::types::{Typed, Type};
#[derive(Debug, Clone)]
pub struct VariableSet {
name: String,
value: Box<Expr>,
expr: Box<Expr>,
global_id: Option<usize>,
index: Option<Expr>,
is_ref: bool
}

impl VariableSet {
fn translate_eval_if_ref(&self, expr: &Expr, meta: &mut TranslateMetadata) -> String {
if self.is_ref {
expr.translate_eval(meta, true)
} else {
expr.translate(meta)
}
}
}

impl SyntaxModule<ParserMetadata> for VariableSet {
syntax_name!("Variable Set");

fn new() -> Self {
VariableSet {
name: String::new(),
value: Box::new(Expr::new()),
expr: Box::new(Expr::new()),
global_id: None,
index: None,
is_ref: false
Expand All @@ -32,13 +42,13 @@ impl SyntaxModule<ParserMetadata> for VariableSet {
self.name = variable(meta, variable_name_extensions())?;
self.index = handle_index_accessor(meta)?;
token(meta, "=")?;
syntax(meta, &mut *self.value)?;
syntax(meta, &mut *self.expr)?;
let variable = handle_variable_reference(meta, tok.clone(), &self.name)?;
self.global_id = variable.global_id;
self.is_ref = variable.is_ref;
// Typecheck the variable
let left_type = variable.kind.clone();
let right_type = self.value.get_type();
let right_type = self.expr.get_type();
// Check if the variable can be indexed
if self.index.is_some() && !matches!(variable.kind, Type::Array(_)) {
return error!(meta, tok, format!("Cannot assign a value to an index of a non-array variable of type '{left_type}'"));
Expand All @@ -47,14 +57,14 @@ impl SyntaxModule<ParserMetadata> for VariableSet {
if self.index.is_some() {
// Check if the assigned value is compatible with the array
if let Type::Array(kind) = variable.kind.clone() {
if *kind != self.value.get_type() {
let right_type = self.value.get_type();
if *kind != self.expr.get_type() {
let right_type = self.expr.get_type();
return error!(meta, tok, format!("Cannot assign value of type '{right_type}' to an array of '{kind}'"));
}
}
}
// Check if the variable is compatible with the assigned value
else if variable.kind != self.value.get_type() {
else if variable.kind != self.expr.get_type() {
return error!(meta, tok, format!("Cannot assign value of type '{right_type}' to a variable of type '{left_type}'"));
}
Ok(())
Expand All @@ -65,23 +75,19 @@ impl TranslateModule for VariableSet {
fn translate(&self, meta: &mut TranslateMetadata) -> String {
let name = self.name.clone();
let index = self.index.as_ref()
.map(|index| format!("[{}]", self.is_ref
.then(|| index.translate_eval(meta, true))
.unwrap_or_else(|| index.translate(meta))))
.map(|index| self.translate_eval_if_ref(index, meta))
Copy link
Contributor Author

@hdwalters hdwalters Nov 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added helper function translate_eval_if_ref() to remove duplicated code here. I would have moved this new function to the definition of TranslateModule, but it only seems to be required here.

.map(|index| format!("[{index}]"))
.unwrap_or_default();
let mut expr = self.is_ref
.then(|| self.value.translate_eval(meta, true))
.unwrap_or_else(|| self.value.translate(meta));
if let Type::Array(_) = self.value.get_type() {
expr = format!("({})", expr);
let mut expr = self.translate_eval_if_ref(self.expr.as_ref(), meta);
if let Type::Array(_) = self.expr.get_type() {
expr = format!("({expr})");
}
if self.is_ref {
if let Some(id) = self.global_id {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We previously tested self.global_id after self.is_ref, but I switched the order to match the shorthand operators. I think this is safe, because global_id and is_ref cannot both be enabled at the same time.

format!("__{id}_{name}{index}={expr}")
} else if self.is_ref {
format!("eval \"${{{name}}}{index}={expr}\"")
} else {
match self.global_id {
Some(id) => format!("__{id}_{name}{index}={expr}"),
None => format!("{name}{index}={expr}")
}
format!("{name}{index}={expr}")
}
}
}
Expand Down