Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement basic span error reporting #118

Merged
merged 3 commits into from Sep 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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