From 121861cc8eeb1caff1863fe8c768e41c378cbdd8 Mon Sep 17 00:00:00 2001 From: Scott Richmond Date: Fri, 20 Jun 2025 15:35:09 -0400 Subject: [PATCH] fix function scoping bug --- assets/prelude.ld | 38 +++++++++++++++++++------------------- may_2025_thoughts.md | 19 +++++++++++++++++++ src/compiler.rs | 21 ++++++++++++++++++--- src/main.rs | 36 ++++++++++++++++++++++++++++-------- 4 files changed, 84 insertions(+), 30 deletions(-) diff --git a/assets/prelude.ld b/assets/prelude.ld index b6daf0c..27586af 100644 --- a/assets/prelude.ld +++ b/assets/prelude.ld @@ -1,10 +1,10 @@ & this file, uniquely, gets `base` loaded as context. See src/base.janet for exports -let base = base +& let base = base & some forward declarations & TODO: fix this so that we don't need (as many of) them -fn and +& fn and fn append fn apply_command fn assoc @@ -738,22 +738,22 @@ fn keyword? { & TODO: determine if Ludus should have a `keyword` function that takes a string and returns a keyword. Too many panics, it has weird memory consequences, etc. -& TODO: make `and` and `or` special forms which lazily evaluate arguments -fn and { - "Returns true if all values passed in are truthy. Note that this does not short-circuit: all arguments are evaulated before they are passed in." - () -> true - (x) -> bool (x) - (x, y) -> base :and (x, y) - (x, y, ...zs) -> fold (and, zs, base :and (x, y)) -} +& & TODO: make `and` and `or` special forms which lazily evaluate arguments +& fn and { +& "Returns true if all values passed in are truthy. Note that this does not short-circuit: all arguments are evaulated before they are passed in." +& () -> true +& (x) -> bool (x) +& (x, y) -> base :and (x, y) +& (x, y, ...zs) -> fold (and, zs, base :and (x, y)) +& } -fn or { - "Returns true if any value passed in is truthy. Note that this does not short-circuit: all arguments are evaluated before they are passed in." - () -> true - (x) -> bool (x) - (x, y) -> base :or (x, y) - (x, y, ...zs) -> fold (or, zs, base :or (x, y)) -} +& fn or { +& "Returns true if any value passed in is truthy. Note that this does not short-circuit: all arguments are evaluated before they are passed in." +& () -> true +& (x) -> bool (x) +& (x, y) -> base :or (x, y) +& (x, y, ...zs) -> fold (or, zs, base :or (x, y)) +& } fn assoc { "Takes a dict, key, and value, and returns a new dict with the key set to value." @@ -1308,7 +1308,7 @@ box state = nil #{ abs & math add & math - and & bool + & and & bool angle & math any? & dicts lists strings sets tuples append & lists sets @@ -1402,7 +1402,7 @@ box state = nil ok & results ok? & results & omit & set - or & bool + & or & bool ordered? & lists tuples strings pc! & turtles pd! & turtles diff --git a/may_2025_thoughts.md b/may_2025_thoughts.md index 08432e3..7ffd827 100644 --- a/may_2025_thoughts.md +++ b/may_2025_thoughts.md @@ -332,4 +332,23 @@ And then: quality of life improvements: * [ ] that suggests that we need a mapping from bytecodes to AST nodes * The way I had been planning on doing this is having a vec that moves in lockstep with bytecode that's just references to ast nodes, which are `'static`, so that shouldn't be too bad. But this is per-chunk, which means we need a reference to that vec in the VM. My sense is that what we want is actually a separate data structure that holds the AST nodes--we'll only need them in the sad path, which can be slow. +### Bugs discovered while trying to compile prelude +#### 2025-06-20 +Consider the following code: +``` +fn one { + (x as :number) -> { + fn two () -> :number + two + } + (x as :bool) -> { + fn two () -> :bool + two + } +} + +``` +The second clause causes a panic in the compiler. +I'm not entirely sure what's going on. +That said, I'm pretty sure the root of it is that the fact that `two` was already bound in the first clause means that it's in the constants vector in that chunk under the name "two." diff --git a/src/compiler.rs b/src/compiler.rs index e34b695..3acb33f 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -531,7 +531,10 @@ impl<'a> Compiler<'a> { name, stack_pos: binding.stack_pos, }, - None => self.enclosing.unwrap().get_upvalue(name), + None => { + println!("Getting upvalue {name}"); + self.enclosing.unwrap().get_upvalue(name) + } } } @@ -1244,7 +1247,13 @@ impl<'a> Compiler<'a> { if !is_anon { let declared = self.chunk.constants.iter().any(|val| match val { - Value::Fn(lfn) => lfn.name() == *name, + Value::Fn(lfn) => { + if matches!(lfn.as_ref(), LFn::Declared { .. }) { + lfn.name() == *name + } else { + false + } + } _ => false, }); if !declared { @@ -1394,7 +1403,13 @@ impl<'a> Compiler<'a> { .constants .iter() .position(|val| match val { - Value::Fn(declaration) => declaration.name() == *name, + Value::Fn(lfn) => { + if matches!(lfn.as_ref(), LFn::Declared { .. }) { + lfn.name() == *name + } else { + false + } + } _ => false, }) .unwrap(); diff --git a/src/main.rs b/src/main.rs index fd064fd..39fd265 100644 --- a/src/main.rs +++ b/src/main.rs @@ -27,16 +27,21 @@ use value::Value; mod vm; use vm::Vm; -const PRELUDE: &str = include_str!("../assets/test_prelude.ld"); +const PRELUDE: &str = include_str!("../assets/prelude.ld"); pub fn prelude() -> HashMap<&'static str, Value> { let tokens = lexer().parse(PRELUDE).into_output_errors().0.unwrap(); - let parsed = parser() + let (parsed, parse_errors) = parser() .parse(Stream::from_iter(tokens).map((0..PRELUDE.len()).into(), |(t, s)| (t, s))) - .into_output_errors() - .0 - .unwrap(); - let parsed: &'static Spanned = Box::leak(Box::new(parsed)); + .into_output_errors(); + + if !parse_errors.is_empty() { + println!("ERROR PARSING PRELUDE:"); + println!("{:?}", parse_errors); + panic!(); + } + + let parsed: &'static Spanned = Box::leak(Box::new(parsed.unwrap())); let mut compiler = Compiler::new(parsed, "prelude", PRELUDE, None, HashMap::new()); let base = base::make_base(); compiler.emit_constant(base); @@ -74,9 +79,13 @@ pub fn run(src: &'static str) { // in any event, the AST should live forever let parsed: &'static Spanned = Box::leak(Box::new(parse_result.unwrap())); - let prelude = prelude(); + // let prelude = prelude(); + let prelude = imbl::HashMap::new(); let mut compiler = Compiler::new(parsed, "test", src, None, prelude); + // let base = base::make_base(); + // compiler.emit_constant(base); + // compiler.bind("base"); compiler.compile(); if DEBUG_COMPILE { @@ -105,7 +114,18 @@ pub fn run(src: &'static str) { pub fn main() { env::set_var("RUST_BACKTRACE", "1"); let src = " -panics! () +fn one { + (x as :number) -> { + fn two () -> :number + two + } + (x as :bool) -> { + fn two () -> :bool + two + } +} + +one (true) () "; run(src); }