Skip to content

Commit

Permalink
Implement basic span error reporting (#118)
Browse files Browse the repository at this point in the history
  • Loading branch information
mitsuhiko committed Sep 17, 2022
1 parent aa17bfd commit f9bfb4f
Show file tree
Hide file tree
Showing 45 changed files with 646 additions and 75 deletions.
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -25,7 +25,7 @@ run-tests:
@echo "CARGO TEST SPEEDUPS"
@cd minijinja; cargo test --no-default-features --features=speedups,$(FEATURES)
@echo "CARGO CHECK NO_DEFAULT_FEATURES"
@cd minijinja; cargo check --no-default-features
@cd minijinja; cargo check --no-default-features --features=debug

check:
@echo "check no default features:"
Expand Down
2 changes: 1 addition & 1 deletion examples/error/src/main.rs
Expand Up @@ -26,7 +26,7 @@ fn main() {
let template = env.get_template("hello.txt").unwrap();
let ctx = context! {
seq => vec![2, 4, 8],
other_seq => (0..20).collect::<Vec<_>>(),
other_seq => (0..5).collect::<Vec<_>>(),
bar => "test"
};
match template.render(&ctx) {
Expand Down
40 changes: 32 additions & 8 deletions minijinja/src/compiler/codegen.rs
Expand Up @@ -27,6 +27,7 @@ pub struct CodeGenerator<'source> {
blocks: BTreeMap<&'source str, Instructions<'source>>,
pending_block: Vec<PendingBlock>,
current_line: usize,
span_stack: Vec<Span>,
}

impl<'source> CodeGenerator<'source> {
Expand All @@ -37,6 +38,7 @@ impl<'source> CodeGenerator<'source> {
blocks: BTreeMap::new(),
pending_block: Vec::new(),
current_line: 0,
span_stack: Vec::new(),
}
}

Expand All @@ -50,8 +52,24 @@ impl<'source> CodeGenerator<'source> {
self.set_line(span.start_line);
}

/// Pushes a span to the stack
pub fn push_span(&mut self, span: Span) {
self.span_stack.push(span);
self.set_line_from_span(span);
}

/// Pops a span from the stack.
pub fn pop_span(&mut self) {
self.span_stack.pop();
}

/// Add a simple instruction.
pub fn add(&mut self, instr: Instruction<'source>) -> usize {
if let Some(span) = self.span_stack.last() {
if span.start_line == self.current_line {
return self.instructions.add_with_span(instr, *span);
}
}
self.instructions.add_with_line(instr, self.current_line)
}

Expand Down Expand Up @@ -353,15 +371,15 @@ impl<'source> CodeGenerator<'source> {
pub fn compile_assignment(&mut self, expr: &ast::Expr<'source>) -> Result<(), Error> {
match expr {
ast::Expr::Var(var) => {
self.set_line_from_span(var.span());
self.add(Instruction::StoreLocal(var.id));
}
ast::Expr::List(list) => {
self.set_line_from_span(list.span());
self.push_span(list.span());
self.add(Instruction::UnpackList(list.items.len()));
for expr in &list.items {
self.compile_assignment(expr)?;
}
self.pop_span();
}
_ => panic!("bad assignment target"),
}
Expand Down Expand Up @@ -404,7 +422,7 @@ impl<'source> CodeGenerator<'source> {
self.end_if();
}
ast::Expr::Filter(f) => {
self.set_line_from_span(f.span());
self.push_span(f.span());
if let Some(ref expr) = f.expr {
self.compile_expr(expr)?;
}
Expand All @@ -413,26 +431,30 @@ impl<'source> CodeGenerator<'source> {
}
self.add(Instruction::BuildList(f.args.len() + 1));
self.add(Instruction::ApplyFilter(f.name));
self.pop_span();
}
ast::Expr::Test(f) => {
self.set_line_from_span(f.span());
self.push_span(f.span());
self.compile_expr(&f.expr)?;
for arg in &f.args {
self.compile_expr(arg)?;
}
self.add(Instruction::BuildList(f.args.len() + 1));
self.add(Instruction::PerformTest(f.name));
self.pop_span();
}
ast::Expr::GetAttr(g) => {
self.set_line_from_span(g.span());
self.push_span(g.span());
self.compile_expr(&g.expr)?;
self.add(Instruction::GetAttr(g.name));
self.pop_span();
}
ast::Expr::GetItem(g) => {
self.set_line_from_span(g.span());
self.push_span(g.span());
self.compile_expr(&g.expr)?;
self.compile_expr(&g.subscript_expr)?;
self.add(Instruction::GetItem);
self.pop_span();
}
ast::Expr::Call(c) => {
self.compile_call(c)?;
Expand All @@ -458,7 +480,7 @@ impl<'source> CodeGenerator<'source> {
}

fn compile_call(&mut self, c: &ast::Spanned<ast::Call<'source>>) -> Result<(), Error> {
self.set_line_from_span(c.span());
self.push_span(c.span());
match c.identify_call() {
ast::CallType::Function(name) => {
for arg in &c.args {
Expand All @@ -485,11 +507,12 @@ impl<'source> CodeGenerator<'source> {
self.add(Instruction::CallObject);
}
};
self.pop_span();
Ok(())
}

fn compile_bin_op(&mut self, c: &ast::Spanned<ast::BinOp<'source>>) -> Result<(), Error> {
self.set_line_from_span(c.span());
self.push_span(c.span());
let instr = match c.op {
ast::BinOpKind::Eq => Instruction::Eq,
ast::BinOpKind::Ne => Instruction::Ne,
Expand Down Expand Up @@ -518,6 +541,7 @@ impl<'source> CodeGenerator<'source> {
self.compile_expr(&c.left)?;
self.compile_expr(&c.right)?;
self.add(instr);
self.pop_span();
Ok(())
}

Expand Down
79 changes: 67 additions & 12 deletions minijinja/src/compiler/instructions.rs
Expand Up @@ -4,6 +4,7 @@ use std::fmt;
#[cfg(test)]
use similar_asserts::assert_eq;

use crate::compiler::tokens::Span;
use crate::value::Value;

/// This loop has the loop var.
Expand Down Expand Up @@ -182,15 +183,23 @@ pub enum Instruction<'source> {
}

#[derive(Copy, Clone)]
struct Loc {
struct LineInfo {
first_instruction: u32,
line: u32,
}

#[derive(Copy, Clone)]
struct SpanInfo {
first_instruction: u32,
span: Option<Span>,
}

/// Wrapper around instructions to help with location management.
pub struct Instructions<'source> {
pub(crate) instructions: Vec<Instruction<'source>>,
locations: Vec<Loc>,
line_infos: Vec<LineInfo>,
#[cfg(feature = "debug")]
span_infos: Vec<SpanInfo>,
name: &'source str,
source: &'source str,
}
Expand All @@ -200,7 +209,9 @@ impl<'source> Instructions<'source> {
pub fn new(name: &'source str, source: &'source str) -> Instructions<'source> {
Instructions {
instructions: Vec::new(),
locations: Vec::new(),
line_infos: Vec::new(),
#[cfg(feature = "debug")]
span_infos: Vec::new(),
name,
source,
}
Expand Down Expand Up @@ -234,35 +245,79 @@ impl<'source> Instructions<'source> {
rv
}

/// Adds a new instruction with location info.
pub fn add_with_line(&mut self, instr: Instruction<'source>, line: usize) -> usize {
let rv = self.add(instr);
fn add_line_record(&mut self, instr: usize, line: usize) {
let same_loc = self
.locations
.line_infos
.last()
.map_or(false, |last_loc| last_loc.line as usize == line);
if !same_loc {
self.locations.push(Loc {
first_instruction: rv as u32,
self.line_infos.push(LineInfo {
first_instruction: instr as u32,
line: line as u32,
});
}
}

/// Adds a new instruction with line number.
pub fn add_with_line(&mut self, instr: Instruction<'source>, line: usize) -> usize {
let rv = self.add(instr);
self.add_line_record(rv, line);
rv
}

/// Adds a new instruction with span.
pub fn add_with_span(&mut self, instr: Instruction<'source>, span: Span) -> usize {
let rv = self.add(instr);
#[cfg(feature = "debug")]
{
let same_loc = self
.span_infos
.last()
.map_or(false, |last_loc| last_loc.span == Some(span));
if !same_loc {
self.span_infos.push(SpanInfo {
first_instruction: rv as u32,
span: Some(span),
});
}
}
self.add_line_record(rv, span.start_line);
rv
}

/// Looks up the line for an instruction
pub fn get_line(&self, idx: usize) -> Option<usize> {
let loc = match self
.locations
.line_infos
.binary_search_by_key(&idx, |x| x.first_instruction as usize)
{
Ok(idx) => &self.locations[idx as usize],
Ok(idx) => &self.line_infos[idx as usize],
Err(0) => return None,
Err(idx) => &self.locations[idx as usize - 1],
Err(idx) => &self.line_infos[idx as usize - 1],
};
Some(loc.line as usize)
}

/// Looks up a span for an instruction.
pub fn get_span(&self, idx: usize) -> Option<Span> {
#[cfg(feature = "debug")]
{
let loc = match self
.span_infos
.binary_search_by_key(&idx, |x| x.first_instruction as usize)
{
Ok(idx) => &self.span_infos[idx as usize],
Err(0) => return None,
Err(idx) => &self.span_infos[idx as usize - 1],
};
loc.span
}
#[cfg(not(feature = "debug"))]
{
None
}
}

/// Returns a list of all names referenced in the current block backwards
/// from the given pc.
#[cfg(feature = "debug")]
Expand Down

0 comments on commit f9bfb4f

Please sign in to comment.