feat: Add span information to compiler errors

Signed-off-by: Anand Krishnamoorthi <anakrish@microsoft.com>
This commit is contained in:
Anand Krishnamoorthi
2025-12-02 12:29:15 -06:00
parent e060e43a6c
commit 5aefd51cb6
10 changed files with 159 additions and 57 deletions

View File

@@ -28,7 +28,8 @@ impl<'a> Compiler<'a> {
if !self.is_builtin(builtin_name) {
return Err(CompilerError::NotBuiltinFunction {
name: builtin_name.to_string(),
});
}
.into());
}
// Check if we already have an index for this builtin
@@ -44,7 +45,8 @@ impl<'a> Compiler<'a> {
} else {
return Err(CompilerError::UnknownBuiltinFunction {
name: builtin_name.to_string(),
});
}
.into());
};
// Create builtin info and add it to the program
@@ -199,10 +201,12 @@ impl<'a> Compiler<'a> {
expr: &ExprRef,
context: &str,
) -> Result<BindingPlan> {
self.get_binding_plan_for_expr(expr)
.ok_or_else(|| CompilerError::MissingBindingPlan {
self.get_binding_plan_for_expr(expr).ok_or_else(|| {
CompilerError::MissingBindingPlan {
context: context.to_string(),
})
}
.at(expr.span())
})
}
pub(super) fn resolve_variable(&mut self, var_name: &str, span: &Span) -> Result<Register> {

View File

@@ -4,6 +4,9 @@
use alloc::format;
use alloc::string::String;
use crate::lexer::Span;
use core::fmt;
#[derive(thiserror::Error, Debug)]
pub enum CompilerError {
#[error("Not a builtin function: {name}")]
@@ -68,4 +71,52 @@ impl From<anyhow::Error> for CompilerError {
}
}
pub type Result<T> = ::core::result::Result<T, CompilerError>;
#[derive(Debug)]
pub struct SpannedCompilerError {
pub error: CompilerError,
pub span: Option<Span>,
}
impl SpannedCompilerError {
pub fn new(error: CompilerError) -> Self {
Self { error, span: None }
}
pub fn with_span(mut self, span: &Span) -> Self {
if self.span.is_none() {
self.span = Some(span.clone());
}
self
}
}
impl fmt::Display for SpannedCompilerError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(span) = &self.span {
let msg = format!("{}", self.error);
write!(f, "{}", span.message("error", &msg))
} else {
write!(f, "{}", self.error)
}
}
}
impl From<CompilerError> for SpannedCompilerError {
fn from(error: CompilerError) -> Self {
Self::new(error)
}
}
impl core::error::Error for SpannedCompilerError {
fn source(&self) -> Option<&(dyn core::error::Error + 'static)> {
Some(&self.error)
}
}
impl CompilerError {
pub fn at(self, span: &Span) -> SpannedCompilerError {
SpannedCompilerError::from(self).with_span(span)
}
}
pub type Result<T> = ::core::result::Result<T, SpannedCompilerError>;

View File

@@ -66,11 +66,12 @@ impl<'a> Compiler<'a> {
let result: Result<Register> = match binding_plan {
BindingPlan::Assignment { plan } => self
.compile_assignment_plan_using_hoisted_destructuring(&plan, span)
.map_err(CompilerError::from),
.map_err(|e| CompilerError::from(e).at(span)),
other => Err(CompilerError::UnexpectedBindingPlan {
context: "assignment expression".to_string(),
found: format!("{other:?}"),
}),
}
.at(span)),
};
return result;

View File

@@ -220,7 +220,7 @@ impl<'a> Compiler<'a> {
);
Ok(dest)
}
_ => Err(CompilerError::InvalidUnaryMinus),
_ => Err(CompilerError::InvalidUnaryMinus.at(span)),
}
}

View File

@@ -16,15 +16,15 @@ impl<'a> Compiler<'a> {
params: &[ExprRef],
span: Span,
) -> Result<Register> {
let fcn_path =
get_path_string(fcn, None).map_err(|_| CompilerError::InvalidFunctionExpression)?;
let fcn_path = get_path_string(fcn, None)
.map_err(|_| CompilerError::InvalidFunctionExpression.at(&span))?;
let original_fcn_path = fcn_path.clone();
let full_fcn_path = if self.policy.inner.rules.contains_key(&fcn_path) {
fcn_path
} else {
get_path_string(fcn, Some(&self.current_package))
.map_err(|_| CompilerError::InvalidFunctionExpressionWithPackage)?
.map_err(|_| CompilerError::InvalidFunctionExpressionWithPackage.at(&span))?
};
let mut arg_regs = Vec::new();
@@ -68,7 +68,8 @@ impl<'a> Compiler<'a> {
} else {
return Err(CompilerError::UnknownFunction {
name: original_fcn_path,
});
}
.at(&span));
}
Ok(dest)

View File

@@ -20,11 +20,14 @@ impl<'a> Compiler<'a> {
.loop_hoisting_table
.get_statement_loops(self.current_module_index, stmt.sidx)
.cloned()
.ok_or_else(|| CompilerError::General {
message: format!(
"missing loop hoisting data for statement at {}:{}",
stmt.span.line, stmt.span.col
),
.ok_or_else(|| {
CompilerError::General {
message: format!(
"missing loop hoisting data for statement at {}:{}",
stmt.span.line, stmt.span.col
),
}
.at(&stmt.span)
})
}
@@ -70,7 +73,8 @@ impl<'a> Compiler<'a> {
}
LoopType::Walk => Err(CompilerError::General {
message: "walk loops are not yet supported in the RVM compiler".to_string(),
}),
}
.into()),
}
}
@@ -195,7 +199,8 @@ impl<'a> Compiler<'a> {
return Err(CompilerError::UnexpectedBindingPlan {
context: format!("loop index pattern {}", key_var.span().text()),
found: format!("{binding_plan:?}"),
});
}
.at(key_var.span()));
}
} else {
match key_var.as_ref() {
@@ -217,7 +222,8 @@ impl<'a> Compiler<'a> {
_ => {
return Err(CompilerError::MissingBindingPlan {
context: format!("loop index pattern {}", key_var.span().text()),
});
}
.at(key_var.span()));
}
}
}
@@ -244,7 +250,7 @@ impl<'a> Compiler<'a> {
if let Some((binding_plan, plan_span)) = key_binding_plan.as_ref() {
self.apply_binding_plan(binding_plan, key_reg, plan_span)
.map_err(CompilerError::from)?;
.map_err(|e| CompilerError::from(e).at(plan_span))?;
}
let body_stmts = &remaining_stmts[0..];
@@ -335,12 +341,13 @@ impl<'a> Compiler<'a> {
value_reg,
collection.span(),
)
.map_err(CompilerError::from)?;
.map_err(|e| CompilerError::from(e).at(collection.span()))?;
} else {
return Err(CompilerError::UnexpectedBindingPlan {
context: format!("some-in binding {}", collection.span().text()),
found: format!("{binding_plan:?}"),
});
}
.at(collection.span()));
}
} else {
if let Some(key_expr) = key {
@@ -348,13 +355,24 @@ impl<'a> Compiler<'a> {
ast::Expr::Var {
value: var_name, ..
} => {
let var_name = var_name.as_string()?.to_string();
let var_name = var_name
.as_string()
.map_err(|err| {
CompilerError::General {
message: format!(
"Failed to read some-in key variable name: {err}"
),
}
.at(key_expr.span())
})?
.to_string();
self.store_variable(var_name, key_reg);
}
_ => {
return Err(CompilerError::MissingBindingPlan {
context: format!("some-in key pattern {}", key_expr.span().text()),
});
}
.at(key_expr.span()));
}
}
}
@@ -363,13 +381,24 @@ impl<'a> Compiler<'a> {
ast::Expr::Var {
value: var_name, ..
} => {
let var_name = var_name.as_string()?.to_string();
let var_name = var_name
.as_string()
.map_err(|err| {
CompilerError::General {
message: format!(
"Failed to read some-in value variable name: {err}"
),
}
.at(value.span())
})?
.to_string();
self.store_variable(var_name, value_reg);
}
_ => {
return Err(CompilerError::MissingBindingPlan {
context: format!("some-in value pattern {}", value.span().text()),
});
}
.at(value.span()));
}
}
}

View File

@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
use super::{Compiler, Result};
use super::{Compiler, CompilerError, Result};
use crate::interpreter::Interpreter;
use crate::rvm::program::{Program, RuleType, SpanInfo};
use crate::rvm::Instruction;
@@ -129,7 +129,9 @@ impl<'a> Compiler<'a> {
self.program.entry_points = self.entry_points;
if !self.program.builtin_info_table.is_empty() {
self.program.initialize_resolved_builtins()?;
self.program
.initialize_resolved_builtins()
.map_err(|err| CompilerError::from(err))?;
}
Ok(self.program)

View File

@@ -192,7 +192,7 @@ impl<'a> Compiler<'a> {
}
Ok(())
} else {
Err(CompilerError::MissingYieldContext)
Err(CompilerError::MissingYieldContext.into())
}
}
@@ -204,7 +204,7 @@ impl<'a> Compiler<'a> {
self.compile_rego_expr_with_span(expr, &stmt.span, assert_condition)?;
}
ast::Literal::SomeIn { .. } => {
return Err(CompilerError::SomeInNotHoisted);
return Err(CompilerError::SomeInNotHoisted.at(&stmt.span));
}
ast::Literal::Every {
key,

View File

@@ -81,7 +81,7 @@ pub(super) fn parse_reference_chain(expr: &ExprRef) -> Result<ReferenceChain> {
current_expr = refr;
}
_ => {
return Err(CompilerError::NotSimpleReferenceChain);
return Err(CompilerError::NotSimpleReferenceChain.at(current_expr.span()));
}
}
}
@@ -117,7 +117,7 @@ impl<'a> Compiler<'a> {
fn compile_data_chain(&mut self, chain: &ReferenceChain, span: &Span) -> Result<Register> {
if chain.components.is_empty() {
// Just "data" - direct access to data root is not allowed
return Err(CompilerError::DirectDataAccess);
return Err(CompilerError::DirectDataAccess.at(span));
}
// Build the static prefix path components for rule matching
@@ -301,7 +301,8 @@ impl<'a> Compiler<'a> {
// No rule found - undefined variable
Err(CompilerError::UndefinedVariable {
name: chain.root.clone(),
})
}
.at(span))
}
/// Compile chain access using appropriate instructions based on chain length and complexity

View File

@@ -21,7 +21,8 @@ impl<'a> Compiler<'a> {
let Some(definitions) = self.policy.inner.rules.get(rule_path) else {
return Err(CompilerError::General {
message: format!("no definitions found for rule path '{}'", rule_path),
});
}
.into());
};
let rule_types: BTreeSet<RuleType> = definitions
@@ -51,15 +52,16 @@ impl<'a> Compiler<'a> {
"internal: rule '{}' has multiple types: {:?}",
rule_path, rule_types
),
});
}
.into());
}
rule_types
.into_iter()
.next()
.ok_or_else(|| CompilerError::RuleTypeNotFound {
rule_types.into_iter().next().ok_or_else(|| {
CompilerError::RuleTypeNotFound {
rule_path: rule_path.to_string(),
})
}
.into()
})
}
pub(super) fn get_or_assign_rule_index(&mut self, rule_path: &str) -> Result<u16> {
@@ -130,13 +132,19 @@ impl<'a> Compiler<'a> {
for policy_rule in &module.policy {
let policy_rule_ptr = policy_rule.as_ref() as *const Rule;
if policy_rule_ptr == rule_ptr {
let package_path = get_path_string(&module.package.refr, Some("data"))
.map_err(|e| CompilerError::General {
message: format!(
"Failed to get package path for module: {}",
e
),
})?;
let package_path =
match get_path_string(&module.package.refr, Some("data")) {
Ok(path) => path,
Err(e) => {
return Err(CompilerError::General {
message: format!(
"Failed to get package path for module: {}",
e
),
}
.into());
}
};
return Ok((package_path, module_index as u32));
}
}
@@ -212,7 +220,8 @@ impl<'a> Compiler<'a> {
"Compile-time recursion detected in rule call chain: {}",
chain.join(" -> ")
),
});
}
.into());
}
}
@@ -225,7 +234,8 @@ impl<'a> Compiler<'a> {
} else {
return Err(CompilerError::General {
message: format!("Rule index not found for '{}'", entry.rule_path),
});
}
.into());
};
call_stack.push(entry.rule_path.clone());
@@ -264,14 +274,15 @@ impl<'a> Compiler<'a> {
let saved_register_counter = self.register_counter;
if let Some(rule_definitions) = rules.get(rule_path) {
let rule_index = self.rule_index_map.get(rule_path).copied().ok_or_else(|| {
CompilerError::General {
let Some(rule_index) = self.rule_index_map.get(rule_path).copied() else {
return Err(CompilerError::General {
message: format!(
"Rule '{}' not found in rule index map during compilation",
rule_path
),
}
})?;
.into());
};
let rule_type = self.rule_types[rule_index as usize].clone();
let result_register = 0;
@@ -358,12 +369,13 @@ impl<'a> Compiler<'a> {
if let BindingPlan::Parameter { .. } = &binding_plan {
self.apply_binding_plan(&binding_plan, param_reg, arg.span())
.map_err(CompilerError::from)?;
.map_err(|e| CompilerError::from(e).at(arg.span()))?;
} else {
return Err(CompilerError::UnexpectedBindingPlan {
context: context_desc,
found: format!("{binding_plan:?}"),
});
}
.at(arg.span()));
}
last_param_span = Some(arg.span().clone());
@@ -396,7 +408,8 @@ impl<'a> Compiler<'a> {
"Function rule '{}' definition {} has {} parameters but expected {} parameters",
rule_path, def_idx, param_names.len(), expected_count
),
});
}
.at(span));
}
}
}