From 97d0fc68b2908f2064643b9b5326d6a1b2548597 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Wed, 8 Jan 2025 16:54:41 -0500 Subject: [PATCH] Add macro and helper to clean up expression translations --- core/translate/expr.rs | 324 ++++++++++++++--------------------------- 1 file changed, 107 insertions(+), 217 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index ec2205e7..aef2fe30 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -21,6 +21,46 @@ pub struct ConditionMetadata { pub parent_op: Option, } +fn emit_cond_jump(program: &mut ProgramBuilder, cond_meta: ConditionMetadata, reg: usize) { + if cond_meta.jump_if_condition_is_true { + program.emit_insn(Insn::If { + reg, + target_pc: cond_meta.jump_target_when_true, + null_reg: reg, + }); + } else { + program.emit_insn(Insn::IfNot { + reg, + target_pc: cond_meta.jump_target_when_false, + null_reg: reg, + }); + } +} +macro_rules! emit_cmp_insn { + ( + $program:expr, + $cond:expr, + $op_true:ident, + $op_false:ident, + $lhs:expr, + $rhs:expr + ) => {{ + if $cond.jump_if_condition_is_true { + $program.emit_insn(Insn::$op_true { + lhs: $lhs, + rhs: $rhs, + target_pc: $cond.jump_target_when_true, + }); + } else { + $program.emit_insn(Insn::$op_false { + lhs: $lhs, + rhs: $rhs, + target_pc: $cond.jump_target_when_false, + }); + } + }}; +} + pub fn translate_condition_expr( program: &mut ProgramBuilder, referenced_tables: &[TableReference], @@ -63,7 +103,7 @@ pub fn translate_condition_expr( let local_true_label = program.allocate_label(); let local_false_label = program.allocate_label(); - // evaluate LHS in normal OR fashion, short-circuit if true + // evaluate LHS in normal OR fashion, short-circuit local if true let lhs_metadata = ConditionMetadata { jump_if_condition_is_true: true, jump_target_when_true: local_true_label, @@ -89,8 +129,8 @@ pub fn translate_condition_expr( program.emit_insn(Insn::Goto { target_pc: condition_metadata.jump_target_when_false, }); - // local_true means (C OR D) is true => we do *not* jump to parent's "true" label, - // because the parent is an AND, so we want to keep evaluating the rest: + // local_true: we do not jump to parent's "true" label because the parent is AND, + // so we want to keep evaluating the rest program.resolve_label(local_true_label, program.offset()); } else { let jump_target_when_false = program.allocate_label(); @@ -114,106 +154,26 @@ pub fn translate_condition_expr( } } ast::Expr::Binary(lhs, op, rhs) => { - let lhs_reg = program.alloc_register(); - let _ = translate_expr(program, Some(referenced_tables), lhs, lhs_reg, resolver); - if let ast::Expr::Literal(_) = lhs.as_ref() { - program.mark_last_insn_constant() - } - let rhs_reg = program.alloc_register(); - let _ = translate_expr(program, Some(referenced_tables), rhs, rhs_reg, resolver); - if let ast::Expr::Literal(_) = rhs.as_ref() { - program.mark_last_insn_constant() - } + let lhs_reg = translate_and_mark(program, Some(referenced_tables), lhs, resolver)?; + let rhs_reg = translate_and_mark(program, Some(referenced_tables), rhs, resolver)?; match op { ast::Operator::Greater => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Gt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Le { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Gt, Le, lhs_reg, rhs_reg) } ast::Operator::GreaterEquals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Ge { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Lt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Ge, Lt, lhs_reg, rhs_reg) } ast::Operator::Less => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Lt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Ge { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Lt, Ge, lhs_reg, rhs_reg) } ast::Operator::LessEquals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Le { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Gt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Le, Gt, lhs_reg, rhs_reg) } ast::Operator::Equals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Eq, Ne, lhs_reg, rhs_reg) } ast::Operator::NotEquals => { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }) - } else { - program.emit_insn(Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }) - } + emit_cmp_insn!(program, condition_metadata, Ne, Eq, lhs_reg, rhs_reg) } ast::Operator::Is => todo!(), ast::Operator::IsNot => todo!(), @@ -231,19 +191,7 @@ pub fn translate_condition_expr( value: int_value, dest: reg, }); - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::If { - reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: reg, - }) - } else { - program.emit_insn(Insn::IfNot { - reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: reg, - }) - } + emit_cond_jump(program, condition_metadata, reg); } else { crate::bail_parse_error!("unsupported literal type in condition"); } @@ -254,19 +202,7 @@ pub fn translate_condition_expr( value: string.clone(), dest: reg, }); - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::If { - reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: reg, - }) - } else { - program.emit_insn(Insn::IfNot { - reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: reg, - }) - } + emit_cond_jump(program, condition_metadata, reg); } unimpl => todo!("literal {:?} not implemented", unimpl), }, @@ -392,18 +328,8 @@ pub fn translate_condition_expr( match op { ast::LikeOperator::Like | ast::LikeOperator::Glob => { let pattern_reg = program.alloc_register(); - let column_reg = program.alloc_register(); let mut constant_mask = 0; - let _ = translate_expr( - program, - Some(referenced_tables), - lhs, - column_reg, - resolver, - )?; - if let ast::Expr::Literal(_) = lhs.as_ref() { - program.mark_last_insn_constant(); - } + let _ = translate_and_mark(program, Some(referenced_tables), lhs, resolver); let _ = translate_expr( program, Some(referenced_tables), @@ -411,7 +337,7 @@ pub fn translate_condition_expr( pattern_reg, resolver, )?; - if let ast::Expr::Literal(_) = rhs.as_ref() { + if matches!(rhs.as_ref(), ast::Expr::Literal(_)) { program.mark_last_insn_constant(); constant_mask = 1; } @@ -434,19 +360,7 @@ pub fn translate_condition_expr( ast::LikeOperator::Regexp => todo!(), } if !*not { - if condition_metadata.jump_if_condition_is_true { - program.emit_insn(Insn::If { - reg: cur_reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: cur_reg, - }); - } else { - program.emit_insn(Insn::IfNot { - reg: cur_reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: cur_reg, - }); - } + emit_cond_jump(program, condition_metadata, cur_reg); } else if condition_metadata.jump_if_condition_is_true { program.emit_insn(Insn::IfNot { reg: cur_reg, @@ -462,16 +376,18 @@ pub fn translate_condition_expr( } } ast::Expr::Parenthesized(exprs) => { - // TODO: this is probably not correct; multiple expressions in a parenthesized expression - // are reserved for special cases like `(a, b) IN ((1, 2), (3, 4))`. - for expr in exprs { + if exprs.len() == 1 { let _ = translate_condition_expr( program, referenced_tables, - expr, + &exprs[0], condition_metadata, resolver, ); + } else { + crate::bail_parse_error!( + "parenthesized condtional should have exactly one expression" + ); } } _ => todo!("op {:?} not implemented", expr), @@ -875,7 +791,7 @@ pub fn translate_expr( unreachable!("this is always ast::Expr::Cast") } ScalarFunc::Changes => { - if let Some(_) = args { + if args.is_some() { crate::bail_parse_error!( "{} fucntion with more than 0 arguments", srf @@ -1110,12 +1026,8 @@ pub fn translate_expr( ); }; for arg in args { - let reg = program.alloc_register(); let _ = - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + translate_and_mark(program, referenced_tables, arg, resolver); } program.emit_insn(Insn::Function { // Only constant patterns for LIKE are supported currently, so this @@ -1153,12 +1065,11 @@ pub fn translate_expr( srf.to_string() ); }; - - let regs = program.alloc_register(); - translate_expr(program, referenced_tables, &args[0], regs, resolver)?; + let reg = + translate_and_mark(program, referenced_tables, &args[0], resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, - start_reg: regs, + start_reg: reg, dest: target_register, func: func_ctx, }); @@ -1184,12 +1095,10 @@ pub fn translate_expr( if let Some(args) = args { for arg in args.iter() { // register containing result of each argument expression - let target_reg = program.alloc_register(); - _ = translate_expr( + let _ = translate_and_mark( program, referenced_tables, arg, - target_reg, resolver, )?; } @@ -1221,15 +1130,14 @@ pub fn translate_expr( let str_reg = program.alloc_register(); let start_reg = program.alloc_register(); let length_reg = program.alloc_register(); - - translate_expr( + let str_reg = translate_expr( program, referenced_tables, &args[0], str_reg, resolver, )?; - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[1], @@ -1245,7 +1153,6 @@ pub fn translate_expr( resolver, )?; } - program.emit_insn(Insn::Function { constant_mask: 0, start_reg: str_reg, @@ -1265,8 +1172,8 @@ pub fn translate_expr( } else { crate::bail_parse_error!("hex function with no arguments",); }; - let regs = program.alloc_register(); - translate_expr(program, referenced_tables, &args[0], regs, resolver)?; + let regs = + translate_and_mark(program, referenced_tables, &args[0], resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, start_reg: regs, @@ -1306,12 +1213,10 @@ pub fn translate_expr( if let Some(args) = args { for arg in args.iter() { // register containing result of each argument expression - let target_reg = program.alloc_register(); - _ = translate_expr( + let _ = translate_and_mark( program, referenced_tables, arg, - target_reg, resolver, )?; } @@ -1325,7 +1230,7 @@ pub fn translate_expr( Ok(target_register) } ScalarFunc::TotalChanges => { - if let Some(_) = args { + if args.is_some() { crate::bail_parse_error!( "{} fucntion with more than 0 arguments", srf.to_string() @@ -1361,11 +1266,7 @@ pub fn translate_expr( }; for arg in args.iter() { - let reg = program.alloc_register(); - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant(); - } + translate_and_mark(program, referenced_tables, arg, resolver)?; } program.emit_insn(Insn::Function { constant_mask: 0, @@ -1387,12 +1288,7 @@ pub fn translate_expr( crate::bail_parse_error!("min function with no arguments"); }; for arg in args { - let reg = program.alloc_register(); - let _ = - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + translate_and_mark(program, referenced_tables, arg, resolver)?; } program.emit_insn(Insn::Function { @@ -1415,12 +1311,7 @@ pub fn translate_expr( crate::bail_parse_error!("max function with no arguments"); }; for arg in args { - let reg = program.alloc_register(); - let _ = - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + translate_and_mark(program, referenced_tables, arg, resolver)?; } program.emit_insn(Insn::Function { @@ -1456,7 +1347,7 @@ pub fn translate_expr( resolver, )?; let second_reg = program.alloc_register(); - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[1], @@ -1507,34 +1398,30 @@ pub fn translate_expr( srf.to_string() ); }; - let str_reg = program.alloc_register(); let pattern_reg = program.alloc_register(); let replacement_reg = program.alloc_register(); - - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[0], str_reg, resolver, )?; - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[1], pattern_reg, resolver, )?; - - translate_expr( + let _ = translate_expr( program, referenced_tables, &args[2], replacement_reg, resolver, )?; - program.emit_insn(Insn::Function { constant_mask: 0, start_reg: str_reg, @@ -1601,12 +1488,12 @@ pub fn translate_expr( }; let mut start_reg = None; if let Some(arg) = args.first() { - let reg = program.alloc_register(); - start_reg = Some(reg); - translate_expr(program, referenced_tables, arg, reg, resolver)?; - if let ast::Expr::Literal(_) = arg { - program.mark_last_insn_constant() - } + start_reg = Some(translate_and_mark( + program, + referenced_tables, + arg, + resolver, + )?); } program.emit_insn(Insn::Function { constant_mask: 0, @@ -1648,10 +1535,8 @@ pub fn translate_expr( crate::bail_parse_error!("{} function with no arguments", math_func); }; - let reg = program.alloc_register(); - - translate_expr(program, referenced_tables, &args[0], reg, resolver)?; - + let reg = + translate_and_mark(program, referenced_tables, &args[0], resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, start_reg: reg, @@ -1673,20 +1558,12 @@ pub fn translate_expr( } else { crate::bail_parse_error!("{} function with no arguments", math_func); }; - let reg1 = program.alloc_register(); + let _ = + translate_expr(program, referenced_tables, &args[0], reg1, resolver)?; let reg2 = program.alloc_register(); - - translate_expr(program, referenced_tables, &args[0], reg1, resolver)?; - if let ast::Expr::Literal(_) = &args[0] { - program.mark_last_insn_constant(); - } - - translate_expr(program, referenced_tables, &args[1], reg2, resolver)?; - if let ast::Expr::Literal(_) = &args[1] { - program.mark_last_insn_constant(); - } - + let _ = + translate_expr(program, referenced_tables, &args[1], reg2, resolver)?; program.emit_insn(Insn::Function { constant_mask: 0, start_reg: target_register + 1, @@ -1710,7 +1587,6 @@ pub fn translate_expr( }; let regs = program.alloc_registers(args.len()); - for (i, arg) in args.iter().enumerate() { translate_expr(program, referenced_tables, arg, regs + i, resolver)?; } @@ -1989,6 +1865,20 @@ pub fn maybe_apply_affinity(col_type: Type, target_register: usize, program: &mu } } +pub fn translate_and_mark( + program: &mut ProgramBuilder, + referenced_tables: Option<&[TableReference]>, + expr: &ast::Expr, + resolver: &Resolver, +) -> Result { + let target_register = program.alloc_register(); + translate_expr(program, referenced_tables, expr, target_register, resolver)?; + if matches!(expr, ast::Expr::Literal(_)) { + program.mark_last_insn_constant(); + } + Ok(target_register) +} + /// Get an appropriate name for an expression. /// If the query provides an alias (e.g. `SELECT a AS b FROM t`), use that (e.g. `b`). /// If the expression is a column from a table, use the column name (e.g. `a`).