Skip to content

Commit

Permalink
show callee source code in "is not a function" errors
Browse files Browse the repository at this point in the history
Fixes #75
  • Loading branch information
y21 committed Dec 24, 2023
1 parent e4a83bf commit 4cd9cfd
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 11 deletions.
2 changes: 1 addition & 1 deletion cli/src/cmd/dump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub fn dump(arg: &ArgMatches) -> anyhow::Result<()> {
}
}

let bytecode = dash_compiler::FunctionCompiler::new(opt, tcx, interner)
let bytecode = dash_compiler::FunctionCompiler::new(&source, opt, tcx, interner)
.compile_ast(ast, true)
.map_err(|err| anyhow!("{}", [err].formattable(&source, true)))?;

Expand Down
2 changes: 1 addition & 1 deletion crates/dash_compiler/src/from_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl<'interner> FunctionCompiler<'interner> {

let tcx = TypeInferCtx::new(counter);

Self::new(opt, tcx, interner)
Self::new(input, opt, tcx, interner)
.compile_ast(ast, true)
.map_err(|err| vec![err])
}
Expand Down
8 changes: 6 additions & 2 deletions crates/dash_compiler/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ macro_rules! simple_instruction {
}
}

impl<'cx, 'inp> InstructionBuilder<'cx, 'inp> {
impl<'cx, 'interner> InstructionBuilder<'cx, 'interner> {
pub fn build_jmp_header(&mut self, label: Label, is_local_label: bool) {
self.write_all(&[0, 0]);
match is_local_label {
Expand Down Expand Up @@ -120,7 +120,11 @@ impl<'cx, 'inp> InstructionBuilder<'cx, 'inp> {
self.write(kind as u8);
}

pub fn build_call(&mut self, meta: FunctionCallMetadata, spread_arg_indices: Vec<u8>) {
pub fn build_call(&mut self, meta: FunctionCallMetadata, spread_arg_indices: Vec<u8>, target_span: Span) {
let ip = self.current_function().buf.len();
self.current_function_mut()
.debug_symbols
.add(ip.try_into().unwrap(), target_span);
self.write_instr(Instruction::Call);
self.write(meta.into());
self.write(spread_arg_indices.len().try_into().unwrap());
Expand Down
15 changes: 12 additions & 3 deletions crates/dash_compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use dash_middle::compiler::constant::{Buffer, Constant, ConstantPool, Function};
use dash_middle::compiler::external::External;
use dash_middle::compiler::instruction::{AssignKind, IntrinsicOperation};
use dash_middle::compiler::scope::{CompileValueType, Scope, ScopeLocal};
use dash_middle::compiler::{CompileResult, FunctionCallMetadata, StaticImportKind};
use dash_middle::compiler::{CompileResult, DebugSymbols, FunctionCallMetadata, StaticImportKind};
use dash_middle::interner::{sym, StringInterner, Symbol};
use dash_middle::lexer::token::TokenType;
use dash_middle::parser::error::Error;
Expand Down Expand Up @@ -82,6 +82,7 @@ struct FunctionLocalState {
/// Keeps track of the total number of loops to be able to have unique IDs
switch_counter: usize,
id: FuncId,
debug_symbols: DebugSymbols,
}

impl FunctionLocalState {
Expand All @@ -97,6 +98,7 @@ impl FunctionLocalState {
loop_counter: 0,
switch_counter: 0,
id,
debug_symbols: DebugSymbols::default(),
}
}

Expand Down Expand Up @@ -153,15 +155,17 @@ pub struct FunctionCompiler<'interner> {
/// Optimization level
#[allow(unused)]
opt_level: OptLevel,
source: Rc<str>,
}

impl<'interner> FunctionCompiler<'interner> {
pub fn new(opt_level: OptLevel, tcx: TypeInferCtx, interner: &'interner mut StringInterner) -> Self {
pub fn new(source: &str, opt_level: OptLevel, tcx: TypeInferCtx, interner: &'interner mut StringInterner) -> Self {
Self {
opt_level,
tcx,
interner,
function_stack: Vec::new(),
source: Rc::from(source),
}
}

Expand Down Expand Up @@ -218,6 +222,8 @@ impl<'interner> FunctionCompiler<'interner> {
cp: root.cp,
locals,
externals,
source: self.source.into(),
debug_symbols: root.debug_symbols,
})
}

Expand Down Expand Up @@ -1195,6 +1201,7 @@ impl<'interner> Visitor<Result<(), Error>> for FunctionCompiler<'interner> {
arguments,
}: FunctionCall,
) -> Result<(), Error> {
let target_span = target.span;
let mut ib = InstructionBuilder::new(self);
// TODO: this also needs to be specialized for assignment expressions with property access as target

Expand Down Expand Up @@ -1308,7 +1315,7 @@ impl<'interner> Visitor<Result<(), Error>> for FunctionCompiler<'interner> {
let meta = FunctionCallMetadata::new_checked(argc, constructor_call, has_this)
.ok_or(Error::ParameterLimitExceeded(span))?;

ib.build_call(meta, spread_arg_indices);
ib.build_call(meta, spread_arg_indices, target_span);

Ok(())
}
Expand Down Expand Up @@ -1614,6 +1621,8 @@ impl<'interner> Visitor<Result<(), Error>> for FunctionCompiler<'interner> {
r#async,
rest_local,
poison_ips: RefCell::new(HashSet::new()),
debug_symbols: cmp.debug_symbols,
source: Rc::clone(&ib.source),
};
ib.build_constant(Constant::Function(Rc::new(function)))
.map_err(|_| Error::ConstantPoolLimitExceeded(span))?;
Expand Down
3 changes: 3 additions & 0 deletions crates/dash_middle/src/compiler/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::parser::expr::LiteralExpr;
use crate::parser::statement::FunctionKind;

use super::external::External;
use super::DebugSymbols;

/// The instruction buffer.
/// Uses interior mutability since we store it in a `Rc<Function>`
Expand Down Expand Up @@ -81,6 +82,8 @@ pub struct Function {
// JIT-poisoned code regions (instruction pointers)
// TODO: refactor this a bit so this isn't "visible" to e.g. the bytecode compiler with builder pattern
pub poison_ips: RefCell<HashSet<usize>>,
pub source: Rc<str>,
pub debug_symbols: DebugSymbols,
}

impl Function {
Expand Down
37 changes: 37 additions & 0 deletions crates/dash_middle/src/compiler/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::rc::Rc;

use strum_macros::FromRepr;

use crate::parser;
use crate::sourcemap::Span;

use self::constant::ConstantPool;
use self::external::External;
Expand All @@ -22,6 +25,40 @@ pub struct CompileResult {
pub cp: ConstantPool,
pub locals: usize,
pub externals: Vec<External>,
pub debug_symbols: DebugSymbols,
pub source: Rc<str>,
}

/// For error purposes, this contains source code snippets used to improve errors, e.g. `x is not a function`
// IMPL DETAILS: We intentionally use a rather "dense" representation to save memory, even if it slows down the error path.
#[cfg_attr(feature = "format", derive(Serialize, Deserialize))]
#[derive(Debug, Clone, Default)]
pub struct DebugSymbols(Vec<(u16, Span)>);

impl DebugSymbols {
pub fn add(&mut self, ip: u16, symbol: Span) {
#[cfg(debug_assertions)]
{
if let Some(&(last_ip, _)) = self.0.last() {
// ensure requirement for binary search
assert!(last_ip <= ip);
}
}

self.0.push((ip, symbol));
}

pub fn get(&self, ip: u16) -> Span {
self.0
.binary_search_by_key(&ip, |(ip, _)| *ip)
.ok()
.map(|i| self.0[i].1)
.unwrap()
}

pub fn iter(&self) -> impl Iterator<Item = &(u16, Span)> {
self.0.iter()
}
}

/// Function call metadata
Expand Down
6 changes: 5 additions & 1 deletion crates/dash_middle/src/sourcemap.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
#[cfg(feature = "format")]
use serde::{Deserialize, Serialize};

#[cfg_attr(feature = "format", derive(Serialize, Deserialize))]
#[derive(Debug, Clone, Copy)]
pub struct Span {
pub lo: u32,
Expand Down Expand Up @@ -26,7 +30,7 @@ impl Span {
&src[self.lo as usize..self.hi as usize]
}
pub fn to(self, other: Span) -> Span {
debug_assert!(other.hi >= self.lo);
debug_assert!(other.hi >= self.lo && self.is_user_span() && other.is_user_span());
Span {
lo: self.lo,
hi: other.hi,
Expand Down
7 changes: 5 additions & 2 deletions crates/dash_vm/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ mod handlers {
this: Value,
argc: usize,
is_constructor: bool,
call_ip: u16,
) -> Result<Option<HandleResult>, Unrooted> {
let (args, refs) = {
let mut args = Vec::with_capacity(argc);
Expand Down Expand Up @@ -642,7 +643,7 @@ mod handlers {
let ret = if is_constructor {
callee.construct(&mut cx, this, args)?
} else {
callee.apply(&mut cx, this, args)?
callee.apply_with_debug(&mut cx, this, args, call_ip)?
};

// SAFETY: no need to root, we're directly pushing into the value stack which itself is a root
Expand All @@ -651,6 +652,8 @@ mod handlers {
}

pub fn call<'sc, 'vm>(mut cx: DispatchContext<'sc, 'vm>) -> Result<Option<HandleResult>, Unrooted> {
let call_ip = cx.active_frame().ip as u16 - 1;

let meta = FunctionCallMetadata::from(cx.fetch_and_inc_ip());
let argc = usize::from(meta.value());
let is_constructor = meta.is_constructor_call();
Expand All @@ -674,7 +677,7 @@ mod handlers {
then {
call_flat(cx, &callee, this, function, user_function, argc, is_constructor)
} else {
call_generic(cx, &callee, this, argc, is_constructor)
call_generic(cx, &callee, this, argc, is_constructor, call_ip)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/dash_vm/src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl Vm {
.map_err(EvalError::Middle)?;

let tcx = TypeInferCtx::new(counter);
let cr = FunctionCompiler::new(opt, tcx, interner)
let cr = FunctionCompiler::new(input, opt, tcx, interner)
.compile_ast(ast, true)
.map_err(|err| EvalError::Middle(vec![err]))?;
let mut frame = Frame::from_compile_result(cr);
Expand Down
2 changes: 2 additions & 0 deletions crates/dash_vm/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ impl Frame {
r#async: false,
rest_local: None,
poison_ips: RefCell::new(HashSet::new()),
source: cr.source,
debug_symbols: cr.debug_symbols,
};

Self {
Expand Down
28 changes: 28 additions & 0 deletions crates/dash_vm/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use dash_proc_macro::Trace;

use crate::gc::handle::Handle;
use crate::gc::trace::Trace;
use crate::util::cold_path;
use crate::value::function::FunctionKind;
use crate::value::primitive::{Null, Undefined};
use crate::{delegate, throw};
Expand Down Expand Up @@ -384,6 +385,33 @@ impl Value {
}
}

/// Calls a function with debug information. This will print the function being attempted to call as written in the source code.
pub(crate) fn apply_with_debug(
&self,
sc: &mut LocalScope,
this: Value,
args: Vec<Value>,
ip: u16,
) -> Result<Unrooted, Unrooted> {
match self {
Self::Object(o) => o.apply(sc, this, args),
Self::External(o) => o.apply(sc, this, args),
_ => {
cold_path();

let frame = sc.frames.last().unwrap();
let snippet = frame
.function
.debug_symbols
.get(ip)
.res(&frame.function.source)
.to_owned();

throw!(sc, TypeError, "{} is not a function", snippet)
}
}
}

pub fn construct(&self, sc: &mut LocalScope, this: Value, args: Vec<Value>) -> Result<Unrooted, Unrooted> {
match self {
Self::Object(o) => o.construct(sc, this, args),
Expand Down

0 comments on commit 4cd9cfd

Please sign in to comment.