diff --git a/Changelog.md b/Changelog.md index 19253da..76e2bf8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,10 @@ # Changelog +[0.2.3] + +- Fixed stack underflow with function invocations + - They are still "heavy", but they should work as expected now + [0.2.2] - Fixed some formatting issues when converting Markdown to Text Mesh Pro for diff --git a/ModData/About/About.xml b/ModData/About/About.xml index 9ed2509..ae84c45 100644 --- a/ModData/About/About.xml +++ b/ModData/About/About.xml @@ -2,7 +2,7 @@ Slang JoeDiertay - 0.2.2 + 0.2.3 [h1]Slang: High-Level Programming for Stationeers[/h1] diff --git a/csharp_mod/stationeersSlang.csproj b/csharp_mod/stationeersSlang.csproj index 554a462..445cd13 100644 --- a/csharp_mod/stationeersSlang.csproj +++ b/csharp_mod/stationeersSlang.csproj @@ -5,7 +5,7 @@ enable StationeersSlang Slang Compiler Bridge - 0.2.2 + 0.2.3 true latest diff --git a/rust_compiler/Cargo.lock b/rust_compiler/Cargo.lock index 3ffa5f4..e60ec2f 100644 --- a/rust_compiler/Cargo.lock +++ b/rust_compiler/Cargo.lock @@ -909,7 +909,7 @@ checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" [[package]] name = "slang" -version = "0.2.2" +version = "0.2.3" dependencies = [ "anyhow", "clap", diff --git a/rust_compiler/Cargo.toml b/rust_compiler/Cargo.toml index 7611197..89fff96 100644 --- a/rust_compiler/Cargo.toml +++ b/rust_compiler/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "slang" -version = "0.2.2" +version = "0.2.3" edition = "2021" [workspace] diff --git a/rust_compiler/libs/compiler/src/test/binary_expression.rs b/rust_compiler/libs/compiler/src/test/binary_expression.rs index f3a1258..558b04f 100644 --- a/rust_compiler/libs/compiler/src/test/binary_expression.rs +++ b/rust_compiler/libs/compiler/src/test/binary_expression.rs @@ -52,6 +52,8 @@ fn nested_binary_expressions() -> Result<()> { add r1 r10 r9 mul r2 r1 r8 move r15 r2 + j L1 + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 diff --git a/rust_compiler/libs/compiler/src/test/declaration_function_invocation.rs b/rust_compiler/libs/compiler/src/test/declaration_function_invocation.rs index 43e3131..5f3f499 100644 --- a/rust_compiler/libs/compiler/src/test/declaration_function_invocation.rs +++ b/rust_compiler/libs/compiler/src/test/declaration_function_invocation.rs @@ -17,6 +17,7 @@ fn no_arguments() -> anyhow::Result<()> { j main doSomething: push ra + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 @@ -34,14 +35,17 @@ fn no_arguments() -> anyhow::Result<()> { #[test] fn let_var_args() -> anyhow::Result<()> { - // !IMPORTANT this needs to be stabilized as it currently incorrectly calculates sp offset at - // both ends of the cleanup lifecycle let compiled = compile! { debug " - fn doSomething(arg1) {}; - let arg1 = 123; - let i = doSomething(arg1); + fn mul2(arg1) { + return arg1 * 2; + }; + loop { + let arg1 = 123; + let i = mul2(arg1); + i = i ** 2; + } " }; @@ -50,23 +54,31 @@ fn let_var_args() -> anyhow::Result<()> { indoc! { " j main - doSomething: + mul2: pop r8 #arg1 push ra + mul r1 r8 2 + move r15 r1 + j L1 + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 j ra main: + L2: move r8 123 #arg1 push r8 push r8 - jal doSomething + jal mul2 sub r0 sp 1 get r8 db r0 sub sp sp 1 move r9 r15 #i - sub sp sp 1 + pow r1 r9 2 + move r9 r1 #i + j L2 + L3: " } ); @@ -97,7 +109,9 @@ fn inline_literal_args() -> anyhow::Result<()> { let compiled = compile! { debug " - fn doSomething(arg1, arg2) {}; + fn doSomething(arg1, arg2) { + return 5; + }; let thisVariableShouldStayInPlace = 123; let returnedValue = doSomething(12, 34); " @@ -112,6 +126,9 @@ fn inline_literal_args() -> anyhow::Result<()> { pop r8 #arg2 pop r9 #arg1 push ra + move r15 5 #returnValue + j L1 + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 @@ -126,7 +143,6 @@ fn inline_literal_args() -> anyhow::Result<()> { get r8 db r0 sub sp sp 1 move r9 r15 #returnedValue - sub sp sp 1 " } ); @@ -154,6 +170,7 @@ fn mixed_args() -> anyhow::Result<()> { pop r8 #arg2 pop r9 #arg1 push ra + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 @@ -168,7 +185,6 @@ fn mixed_args() -> anyhow::Result<()> { get r8 db r0 sub sp sp 1 move r9 r15 #returnValue - sub sp sp 1 " } ); @@ -198,6 +214,8 @@ fn with_return_statement() -> anyhow::Result<()> { pop r8 #arg1 push ra move r15 456 #returnValue + j L1 + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 @@ -233,6 +251,7 @@ fn with_negative_return_literal() -> anyhow::Result<()> { doSomething: push ra move r15 -1 #returnValue + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 diff --git a/rust_compiler/libs/compiler/src/test/declaration_literal.rs b/rust_compiler/libs/compiler/src/test/declaration_literal.rs index 984bf63..171f83e 100644 --- a/rust_compiler/libs/compiler/src/test/declaration_literal.rs +++ b/rust_compiler/libs/compiler/src/test/declaration_literal.rs @@ -120,7 +120,7 @@ fn test_boolean_return() -> anyhow::Result<()> { fn getTrue() { return true; }; - + let val = getTrue(); " }; @@ -133,6 +133,8 @@ fn test_boolean_return() -> anyhow::Result<()> { getTrue: push ra move r15 1 #returnValue + j L1 + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 diff --git a/rust_compiler/libs/compiler/src/test/function_declaration.rs b/rust_compiler/libs/compiler/src/test/function_declaration.rs index 28200f7..374902e 100644 --- a/rust_compiler/libs/compiler/src/test/function_declaration.rs +++ b/rust_compiler/libs/compiler/src/test/function_declaration.rs @@ -21,6 +21,7 @@ fn test_function_declaration_with_spillover_params() -> anyhow::Result<()> { pop r13 #arg4 pop r14 #arg3 push ra + L1: sub r0 sp 1 get ra db r0 sub sp sp 3 @@ -31,6 +32,48 @@ fn test_function_declaration_with_spillover_params() -> anyhow::Result<()> { Ok(()) } +#[test] +fn test_early_return() -> anyhow::Result<()> { + let compiled = compile!(debug r#" + // This is a test function declaration with no body + fn doSomething() { + if (1 == 1) { + return; + } + let i = 1 + 2; + return; + }; + doSomething(); + "#); + + assert_eq!( + compiled, + indoc! { + " + j main + doSomething: + push ra + seq r1 1 1 + beq r1 0 L2 + j L1 + L2: + move r8 3 #i + j L1 + L1: + sub r0 sp 1 + get ra db r0 + sub sp sp 1 + j ra + main: + jal doSomething + move r1 r15 #__binary_temp_2 + " + } + ); + + Ok(()) +} + #[test] fn test_function_declaration_with_register_params() -> anyhow::Result<()> { let compiled = compile!(debug r#" @@ -47,6 +90,7 @@ fn test_function_declaration_with_register_params() -> anyhow::Result<()> { pop r8 #arg2 pop r9 #arg1 push ra + L1: sub r0 sp 1 get ra db r0 sub sp sp 1 diff --git a/rust_compiler/libs/compiler/src/v1.rs b/rust_compiler/libs/compiler/src/v1.rs index 7df2875..4d56a11 100644 --- a/rust_compiler/libs/compiler/src/v1.rs +++ b/rust_compiler/libs/compiler/src/v1.rs @@ -165,6 +165,7 @@ pub struct Compiler<'a, 'w, W: std::io::Write> { temp_counter: usize, label_counter: usize, loop_stack: Vec<(Cow<'a, str>, Cow<'a, str>)>, // Stores (start_label, end_label) + current_return_label: Option>, pub errors: Vec>, } @@ -186,6 +187,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { temp_counter: 0, label_counter: 0, loop_stack: Vec::new(), + current_return_label: None, errors: Vec::new(), } } @@ -900,7 +902,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { fn expression_function_invocation( &mut self, invoke_expr: Spanned>, - stack: &mut VariableScope<'a, '_>, + parent_scope: &mut VariableScope<'a, '_>, ) -> Result<(), Error<'a>> { let InvocationExpression { name, arguments } = invoke_expr.node; @@ -926,9 +928,10 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { // Best to skip generation of this call to prevent bad IC10 return Ok(()); } + let mut stack = VariableScope::scoped(parent_scope); // backup all used registers to the stack - let active_registers = stack.registers().cloned().collect::>(); + let active_registers = stack.registers(); for register in &active_registers { stack.add_variable( Cow::from(format!("temp_{register}")), @@ -991,7 +994,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { } Expression::Binary(bin_expr) => { // Compile the binary expression to a temp register - let result = self.expression_binary(bin_expr, stack)?; + let result = self.expression_binary(bin_expr, &mut stack)?; let reg_str = self.resolve_register(&result.location)?; self.write_output(format!("push {reg_str}"))?; if let Some(name) = result.temp_name { @@ -1000,7 +1003,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { } Expression::Logical(log_expr) => { // Compile the logical expression to a temp register - let result = self.expression_logical(log_expr, stack)?; + let result = self.expression_logical(log_expr, &mut stack)?; let reg_str = self.resolve_register(&result.location)?; self.write_output(format!("push {reg_str}"))?; if let Some(name) = result.temp_name { @@ -1019,7 +1022,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { end_line: 0, }, // Dummy span }, - stack, + &mut stack, )?; if let Some(result) = result_opt { @@ -1592,7 +1595,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { match expr.node { Expression::Return(ret_expr) => { - self.expression_return(*ret_expr, &mut scope)?; + self.expression_return(ret_expr, &mut scope)?; } _ => { // Swallow errors within expressions so block can continue @@ -1622,133 +1625,149 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { /// Takes the result of the expression and stores it in VariableScope::RETURN_REGISTER fn expression_return( &mut self, - expr: Spanned>, + expr: Option>>>, scope: &mut VariableScope<'a, '_>, ) -> Result, Error<'a>> { - if let Expression::Negation(neg_expr) = &expr.node - && let Expression::Literal(spanned_lit) = &neg_expr.node - && let Literal::Number(neg_num) = &spanned_lit.node - { - let loc = VariableLocation::Persistant(VariableScope::RETURN_REGISTER); - self.emit_variable_assignment( - Cow::from("returnValue"), - &loc, - Cow::from(format!("-{neg_num}")), - )?; - return Ok(loc); - }; - - match expr.node { - Expression::Variable(var_name) => { - match scope.get_location_of(&var_name.node, Some(var_name.span)) { - Ok(loc) => match loc { - VariableLocation::Temporary(reg) | VariableLocation::Persistant(reg) => { - self.write_output(format!( - "move r{} r{reg} {}", - VariableScope::RETURN_REGISTER, - debug!(self, "#returnValue") - ))?; - } - VariableLocation::Constant(lit) => { - let str = extract_literal(lit, false)?; - self.write_output(format!( - "move r{} {str} {}", - VariableScope::RETURN_REGISTER, - debug!(self, "#returnValue") - ))? - } - VariableLocation::Stack(offset) => { - self.write_output(format!( - "sub r{} sp {offset}", - VariableScope::TEMP_STACK_REGISTER - ))?; - self.write_output(format!( - "get r{} db r{}", - VariableScope::RETURN_REGISTER, - VariableScope::TEMP_STACK_REGISTER - ))?; - } - VariableLocation::Device(_) => { - return Err(Error::Unknown( - "You can not return a device from a function.".into(), - Some(var_name.span), - )); - } - }, - Err(_) => { - self.errors.push(Error::UnknownIdentifier( - var_name.node.clone(), - var_name.span, - )); - // Proceed with dummy - } - } - } - Expression::Literal(spanned_lit) => match spanned_lit.node { - Literal::Number(num) => { - self.emit_variable_assignment( - Cow::from("returnValue"), - &VariableLocation::Persistant(VariableScope::RETURN_REGISTER), - Cow::from(num.to_string()), - )?; - } - Literal::Boolean(b) => { - let val = if b { "1" } else { "0" }; - self.emit_variable_assignment( - Cow::from("returnValue"), - &VariableLocation::Persistant(VariableScope::RETURN_REGISTER), - Cow::from(val.to_string()), - )?; - } - _ => {} - }, - Expression::Binary(bin_expr) => { - let result = self.expression_binary(bin_expr, scope)?; - let result_reg = self.resolve_register(&result.location)?; - self.write_output(format!( - "move r{} {}", - VariableScope::RETURN_REGISTER, - result_reg - ))?; - if let Some(name) = result.temp_name { - scope.free_temp(name, None)?; - } - } - Expression::Logical(log_expr) => { - let result = self.expression_logical(log_expr, scope)?; - let result_reg = self.resolve_register(&result.location)?; - self.write_output(format!( - "move r{} {}", - VariableScope::RETURN_REGISTER, - result_reg - ))?; - if let Some(name) = result.temp_name { - scope.free_temp(name, None)?; - } - } - Expression::MemberAccess(access) => { - // Return result of member access - let res_opt = self.expression( - Spanned { - node: Expression::MemberAccess(access), - span: expr.span, - }, - scope, + if let Some(expr) = expr { + if let Expression::Negation(neg_expr) = &expr.node + && let Expression::Literal(spanned_lit) = &neg_expr.node + && let Literal::Number(neg_num) = &spanned_lit.node + { + let loc = VariableLocation::Persistant(VariableScope::RETURN_REGISTER); + self.emit_variable_assignment( + Cow::from("returnValue"), + &loc, + Cow::from(format!("-{neg_num}")), )?; - if let Some(res) = res_opt { - let reg = self.resolve_register(&res.location)?; - self.write_output(format!("move r{} {}", VariableScope::RETURN_REGISTER, reg))?; - if let Some(temp) = res.temp_name { - scope.free_temp(temp, None)?; + return Ok(loc); + }; + + match expr.node { + Expression::Variable(var_name) => { + match scope.get_location_of(&var_name.node, Some(var_name.span)) { + Ok(loc) => match loc { + VariableLocation::Temporary(reg) + | VariableLocation::Persistant(reg) => { + self.write_output(format!( + "move r{} r{reg} {}", + VariableScope::RETURN_REGISTER, + debug!(self, "#returnValue") + ))?; + } + VariableLocation::Constant(lit) => { + let str = extract_literal(lit, false)?; + self.write_output(format!( + "move r{} {str} {}", + VariableScope::RETURN_REGISTER, + debug!(self, "#returnValue") + ))? + } + VariableLocation::Stack(offset) => { + self.write_output(format!( + "sub r{} sp {offset}", + VariableScope::TEMP_STACK_REGISTER + ))?; + self.write_output(format!( + "get r{} db r{}", + VariableScope::RETURN_REGISTER, + VariableScope::TEMP_STACK_REGISTER + ))?; + } + VariableLocation::Device(_) => { + return Err(Error::Unknown( + "You can not return a device from a function.".into(), + Some(var_name.span), + )); + } + }, + Err(_) => { + self.errors.push(Error::UnknownIdentifier( + var_name.node.clone(), + var_name.span, + )); + // Proceed with dummy + } } } + Expression::Literal(spanned_lit) => match spanned_lit.node { + Literal::Number(num) => { + self.emit_variable_assignment( + Cow::from("returnValue"), + &VariableLocation::Persistant(VariableScope::RETURN_REGISTER), + Cow::from(num.to_string()), + )?; + } + Literal::Boolean(b) => { + let val = if b { "1" } else { "0" }; + self.emit_variable_assignment( + Cow::from("returnValue"), + &VariableLocation::Persistant(VariableScope::RETURN_REGISTER), + Cow::from(val.to_string()), + )?; + } + _ => {} + }, + Expression::Binary(bin_expr) => { + let result = self.expression_binary(bin_expr, scope)?; + let result_reg = self.resolve_register(&result.location)?; + self.write_output(format!( + "move r{} {}", + VariableScope::RETURN_REGISTER, + result_reg + ))?; + if let Some(name) = result.temp_name { + scope.free_temp(name, None)?; + } + } + Expression::Logical(log_expr) => { + let result = self.expression_logical(log_expr, scope)?; + let result_reg = self.resolve_register(&result.location)?; + self.write_output(format!( + "move r{} {}", + VariableScope::RETURN_REGISTER, + result_reg + ))?; + if let Some(name) = result.temp_name { + scope.free_temp(name, None)?; + } + } + Expression::MemberAccess(access) => { + // Return result of member access + let res_opt = self.expression( + Spanned { + node: Expression::MemberAccess(access), + span: expr.span, + }, + scope, + )?; + if let Some(res) = res_opt { + let reg = self.resolve_register(&res.location)?; + self.write_output(format!( + "move r{} {}", + VariableScope::RETURN_REGISTER, + reg + ))?; + if let Some(temp) = res.temp_name { + scope.free_temp(temp, None)?; + } + } + } + _ => { + return Err(Error::Unknown( + format!("Unsupported `return` statement: {:?}", expr), + None, + )); + } } - _ => { - return Err(Error::Unknown( - format!("Unsupported `return` statement: {:?}", expr), - None, - )); - } + } + + if let Some(label) = &self.current_return_label { + self.write_output(format!("j {}", label))?; + } else { + return Err(Error::Unknown( + "Return statement used outside of function context.".into(), + None, + )); } Ok(VariableLocation::Persistant(VariableScope::RETURN_REGISTER)) @@ -2344,8 +2363,13 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { } self.write_output("push ra")?; + + let return_label = self.next_label_name(); + + let prev_return_label = self.current_return_label.replace(return_label.clone()); + block_scope.add_variable( - Cow::from(format!("{}_ra", name.node)), + return_label.clone(), LocationRequest::Stack, Some(name.span), )?; @@ -2353,7 +2377,7 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { for expr in body.0 { match expr.node { Expression::Return(ret_expr) => { - self.expression_return(*ret_expr, &mut block_scope)?; + self.expression_return(ret_expr, &mut block_scope)?; } _ => { // Swallow internal errors @@ -2372,11 +2396,13 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { } // Get the saved return address and save it back into `ra` - let ra_res = - block_scope.get_location_of(&Cow::from(format!("{}_ra", name.node)), Some(name.span)); + let ra_res = block_scope.get_location_of(&return_label, Some(name.span)); let ra_stack_offset = match ra_res { - Ok(VariableLocation::Stack(offset)) => offset, + Ok(VariableLocation::Stack(offset)) => { + block_scope.free_temp(return_label.clone(), None)?; + offset + } _ => { // If we can't find RA, we can't return properly. // This usually implies a compiler bug or scope tracking error. @@ -2387,6 +2413,10 @@ impl<'a, 'w, W: std::io::Write> Compiler<'a, 'w, W> { } }; + self.current_return_label = prev_return_label; + + self.write_output(format!("{}:", return_label))?; + self.write_output(format!( "sub r{0} sp {ra_stack_offset}", VariableScope::TEMP_STACK_REGISTER diff --git a/rust_compiler/libs/compiler/src/variable_manager.rs b/rust_compiler/libs/compiler/src/variable_manager.rs index faf8a1a..96b2271 100644 --- a/rust_compiler/libs/compiler/src/variable_manager.rs +++ b/rust_compiler/libs/compiler/src/variable_manager.rs @@ -94,19 +94,21 @@ impl<'a, 'b> VariableScope<'a, 'b> { pub const RETURN_REGISTER: u8 = 15; pub const TEMP_STACK_REGISTER: u8 = 0; - pub fn registers(&self) -> impl Iterator { - self.var_lookup_table - .values() - .filter(|val| { - matches!( - val, - VariableLocation::Temporary(_) | VariableLocation::Persistant(_) - ) - }) - .map(|loc| match loc { - VariableLocation::Persistant(reg) | VariableLocation::Temporary(reg) => reg, - _ => unreachable!(), - }) + pub fn registers(&self) -> Vec { + let mut used = Vec::new(); + + for r in TEMP { + if !self.temporary_vars.contains(&r) { + used.push(r); + } + } + + for r in PERSIST { + if !self.persistant_vars.contains(&r) { + used.push(r); + } + } + used } pub fn scoped(parent: &'b VariableScope<'a, 'b>) -> Self { diff --git a/rust_compiler/libs/parser/src/lib.rs b/rust_compiler/libs/parser/src/lib.rs index b81e73f..661b943 100644 --- a/rust_compiler/libs/parser/src/lib.rs +++ b/rust_compiler/libs/parser/src/lib.rs @@ -1225,18 +1225,34 @@ impl<'a> Parser<'a> { // Need to capture return span let ret_start_span = Self::token_to_span(¤t_token); self.assign_next()?; - let expression = self.expression()?.ok_or(Error::UnexpectedEOF)?; + + let expr = if token_matches!( + self.current_token.as_ref().ok_or(Error::UnexpectedEOF)?, + TokenType::Symbol(Symbol::Semicolon) + ) { + // rewind 1 token so we can check for the semicolon at the bottom of this function. + self.tokenizer.seek(SeekFrom::Current(-1))?; + None + } else { + Some(self.expression()?.ok_or(Error::UnexpectedEOF)?) + }; let ret_span = Span { start_line: ret_start_span.start_line, start_col: ret_start_span.start_col, - end_line: expression.span.end_line, - end_col: expression.span.end_col, + end_line: expr + .as_ref() + .map(|e| e.span.end_line) + .unwrap_or(ret_start_span.end_line), + end_col: expr + .as_ref() + .map(|e| e.span.end_col) + .unwrap_or(ret_start_span.end_col), }; let return_expr = Spanned { span: ret_span, - node: Expression::Return(boxed!(expression)), + node: Expression::Return(expr.map(Box::new)), }; expressions.push(return_expr); diff --git a/rust_compiler/libs/parser/src/tree_node.rs b/rust_compiler/libs/parser/src/tree_node.rs index 60b4311..9f427f6 100644 --- a/rust_compiler/libs/parser/src/tree_node.rs +++ b/rust_compiler/libs/parser/src/tree_node.rs @@ -381,7 +381,7 @@ pub enum Expression<'a> { MethodCall(Spanned>), Negation(Box>>), Priority(Box>>), - Return(Box>>), + Return(Option>>>), Syscall(Spanned>), Ternary(Spanned>), Variable(Spanned>), @@ -409,7 +409,15 @@ impl<'a> std::fmt::Display for Expression<'a> { Expression::MethodCall(e) => write!(f, "{}", e), Expression::Negation(e) => write!(f, "(-{})", e), Expression::Priority(e) => write!(f, "({})", e), - Expression::Return(e) => write!(f, "(return {})", e), + Expression::Return(e) => write!( + f, + "(return {})", + if let Some(e) = e { + e.to_string() + } else { + "".to_string() + } + ), Expression::Syscall(e) => write!(f, "{}", e), Expression::Ternary(e) => write!(f, "{}", e), Expression::Variable(id) => write!(f, "{}", id),