Skip to content

Commit ad7a328

Browse files
penbergpenberg
committed
core/translate: Improve error handling in expr.rs
1 parent 3269465 commit ad7a328

File tree

1 file changed

+61
-27
lines changed

1 file changed

+61
-27
lines changed

‎core/translate/expr.rs‎

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -450,13 +450,19 @@ pub fn translate_condition_expr(
450450

451451
if *not {
452452
// When IN is TRUE (match found), NOT IN should be FALSE
453-
program.resolve_label(not_true_label.unwrap(), program.offset());
453+
let label = not_true_label.ok_or_else(|| {
454+
crate::LimboError::InternalError("not_true_label not set".to_string())
455+
})?;
456+
program.resolve_label(label, program.offset());
454457
program.emit_insn(Insn::Goto {
455458
target_pc: jump_target_when_false,
456459
});
457460

458461
// When IN is FALSE (no match), NOT IN should be TRUE
459-
program.resolve_label(not_false_label.unwrap(), program.offset());
462+
let label = not_false_label.ok_or_else(|| {
463+
crate::LimboError::InternalError("not_false_label not set".to_string())
464+
})?;
465+
program.resolve_label(label, program.offset());
460466
program.emit_insn(Insn::Goto {
461467
target_pc: jump_target_when_true,
462468
});
@@ -631,7 +637,10 @@ pub fn translate_expr(
631637
value: 0,
632638
dest: target_register,
633639
});
634-
let lhs_columns = match unwrap_parens(lhs.as_ref().unwrap())? {
640+
let lhs = lhs.as_ref().ok_or_else(|| {
641+
crate::LimboError::InternalError("lhs is None".to_string())
642+
})?;
643+
let lhs_columns = match unwrap_parens(lhs)? {
635644
ast::Expr::Parenthesized(exprs) => {
636645
exprs.iter().map(|e| e.as_ref()).collect()
637646
}
@@ -647,7 +656,12 @@ pub fn translate_expr(
647656
lhs_column_regs_start + i,
648657
resolver,
649658
)?;
650-
if !lhs_column.is_nonnull(referenced_tables.as_ref().unwrap()) {
659+
let ref_tables = referenced_tables.as_ref().ok_or_else(|| {
660+
crate::LimboError::InternalError(
661+
"referenced_tables is None".to_string(),
662+
)
663+
})?;
664+
if !lhs_column.is_nonnull(ref_tables) {
651665
program.emit_insn(Insn::IsNull {
652666
reg: lhs_column_regs_start + i,
653667
target_pc: if *not_in {
@@ -783,13 +797,10 @@ pub fn translate_expr(
783797
let base_reg = base.as_ref().map(|_| program.alloc_register());
784798
let expr_reg = program.alloc_register();
785799
if let Some(base_expr) = base {
786-
translate_expr(
787-
program,
788-
referenced_tables,
789-
base_expr,
790-
base_reg.unwrap(),
791-
resolver,
792-
)?;
800+
let reg = base_reg.ok_or_else(|| {
801+
crate::LimboError::InternalError("base_reg is None".to_string())
802+
})?;
803+
translate_expr(program, referenced_tables, base_expr, reg, resolver)?;
793804
};
794805
for (when_expr, then_expr) in when_then_pairs {
795806
translate_expr_no_constant_opt(
@@ -857,7 +868,9 @@ pub fn translate_expr(
857868
Ok(target_register)
858869
}
859870
ast::Expr::Cast { expr, type_name } => {
860-
let type_name = type_name.as_ref().unwrap(); // TODO: why is this optional?
871+
let type_name = type_name.as_ref().ok_or_else(|| {
872+
crate::LimboError::ParseError("CAST requires a type name".to_string())
873+
})?;
861874
translate_expr(program, referenced_tables, expr, target_register, resolver)?;
862875
let type_affinity = Affinity::affinity(&type_name.name);
863876
program.emit_insn(Insn::Cast {
@@ -894,8 +907,11 @@ pub fn translate_expr(
894907
crate::bail_parse_error!("unknown function {}", name.as_str());
895908
}
896909

910+
let func = func_type.ok_or_else(|| {
911+
crate::LimboError::ParseError(format!("unknown function {}", name.as_str()))
912+
})?;
897913
let func_ctx = FuncCtx {
898-
func: func_type.unwrap(),
914+
func,
899915
arg_count: args_count,
900916
};
901917

@@ -1222,9 +1238,12 @@ pub fn translate_expr(
12221238
start_reg = Some(start_reg.unwrap_or(reg));
12231239
translate_expr(program, referenced_tables, arg, reg, resolver)?;
12241240
}
1241+
let reg = start_reg.ok_or_else(|| {
1242+
crate::LimboError::InternalError("start_reg not set".to_string())
1243+
})?;
12251244
program.emit_insn(Insn::Function {
12261245
constant_mask: 0,
1227-
start_reg: start_reg.unwrap(),
1246+
start_reg: reg,
12281247
dest: target_register,
12291248
func: func_ctx,
12301249
});
@@ -1339,10 +1358,12 @@ pub fn translate_expr(
13391358
}
13401359

13411360
if args.len() % 2 != 0 {
1361+
let last_arg =
1362+
args.last().expect("args.len() % 2 != 0 implies non-empty");
13421363
translate_expr_no_constant_opt(
13431364
program,
13441365
referenced_tables,
1345-
args.last().unwrap(),
1366+
last_arg,
13461367
target_register,
13471368
resolver,
13481369
NoConstantOptReason::RegisterReuse,
@@ -2035,8 +2056,11 @@ pub fn translate_expr(
20352056
crate::bail_parse_error!("unknown function {}", name.as_str());
20362057
}
20372058

2059+
let func = func_type.ok_or_else(|| {
2060+
crate::LimboError::ParseError(format!("unknown function {}", name.as_str()))
2061+
})?;
20382062
let func_ctx = FuncCtx {
2039-
func: func_type.unwrap(),
2063+
func,
20402064
arg_count: args_count,
20412065
};
20422066

@@ -3341,7 +3365,10 @@ pub fn unwrap_parens(expr: &ast::Expr) -> Result<&ast::Expr> {
33413365
match expr {
33423366
ast::Expr::Column { .. } => Ok(expr),
33433367
ast::Expr::Parenthesized(exprs) => match exprs.len() {
3344-
1 => unwrap_parens(exprs.first().unwrap()),
3368+
1 => {
3369+
let first = exprs.first().expect("checked length == 1");
3370+
unwrap_parens(first)
3371+
}
33453372
_ => Ok(expr), // If the expression is e.g. (x, y), as used in e.g. (x, y) IN (SELECT ...), return as is.
33463373
},
33473374
_ => Ok(expr),
@@ -3356,7 +3383,8 @@ pub fn unwrap_parens_owned(expr: ast::Expr) -> Result<(ast::Expr, usize)> {
33563383
ast::Expr::Parenthesized(mut exprs) => match exprs.len() {
33573384
1 => {
33583385
paren_count += 1;
3359-
let (expr, count) = unwrap_parens_owned(*exprs.pop().unwrap().clone())?;
3386+
let last = exprs.pop().expect("checked length == 1");
3387+
let (expr, count) = unwrap_parens_owned(*last.clone())?;
33603388
paren_count += count;
33613389
Ok((expr, paren_count))
33623390
}
@@ -3777,7 +3805,7 @@ pub fn bind_and_rewrite_expr<'a>(
37773805
let Some(col_idx) = col_idx else {
37783806
crate::bail_parse_error!("no such column: {}", normalized_id);
37793807
};
3780-
let col = tbl.columns().get(col_idx).unwrap();
3808+
let col = tbl.columns().get(col_idx).expect("column index is valid");
37813809
*expr = Expr::Column {
37823810
database: None, // TODO: support different databases
37833811
table: tbl_id,
@@ -3839,7 +3867,7 @@ pub fn bind_and_rewrite_expr<'a>(
38393867
))
38403868
})?;
38413869

3842-
let col = table.columns().get(col_idx).unwrap();
3870+
let col = table.columns().get(col_idx).expect("column index is valid");
38433871

38443872
// Check if this is a rowid alias
38453873
let is_rowid_alias = col.is_rowid_alias();
@@ -4080,9 +4108,12 @@ pub fn get_expr_affinity(
40804108
Affinity::Blob
40814109
}
40824110
}
4083-
ast::Expr::Parenthesized(exprs) if exprs.len() == 1 => {
4084-
get_expr_affinity(exprs.first().unwrap(), referenced_tables)
4085-
}
4111+
ast::Expr::Parenthesized(exprs) if exprs.len() == 1 => get_expr_affinity(
4112+
exprs
4113+
.first()
4114+
.expect("exprs.len() == 1 guarantees first() returns Some"),
4115+
referenced_tables,
4116+
),
40864117
ast::Expr::Collate(expr, _) => get_expr_affinity(expr, referenced_tables),
40874118
// Literals have NO affinity in SQLite!
40884119
ast::Expr::Literal(_) => Affinity::Blob, // No affinity!
@@ -4171,9 +4202,9 @@ pub fn emit_literal(
41714202
.chunks_exact(2)
41724203
.map(|pair| {
41734204
// We assume that sqlite3-parser has already validated that
4174-
// the input is valid hex string, thus unwrap is safe.
4175-
let hex_byte = std::str::from_utf8(pair).unwrap();
4176-
u8::from_str_radix(hex_byte, 16).unwrap()
4205+
// the input is valid hex string, thus expect is safe.
4206+
let hex_byte = std::str::from_utf8(pair).expect("parser validated hex string");
4207+
u8::from_str_radix(hex_byte, 16).expect("parser validated hex digit")
41774208
})
41784209
.collect();
41794210
program.emit_insn(Insn::Blob {
@@ -4330,7 +4361,10 @@ pub(crate) fn emit_returning_results<'a>(
43304361
}
43314362

43324363
turso_assert!(table_references.joined_tables().len() == 1, "RETURNING is only used with INSERT, UPDATE, or DELETE statements, which target a single table");
4333-
let table = table_references.joined_tables().first().unwrap();
4364+
let table = table_references
4365+
.joined_tables()
4366+
.first()
4367+
.ok_or_else(|| crate::LimboError::InternalError("no joined tables".to_string()))?;
43344368

43354369
resolver.enable_expr_to_reg_cache();
43364370
let expr = Expr::RowId {

0 commit comments

Comments
 (0)