From b6dcdbde8dc328ffde4a234dea2d9ca81e8cf9b4 Mon Sep 17 00:00:00 2001 From: Brummel Date: Fri, 27 Mar 2026 22:04:20 +0100 Subject: [PATCH] Refactor instantiate to return Closure The `instantiate` method in `Environment` has been refactored to return `Result, String>` instead of `Rc`. This change separates the creation of a `Closure` from its execution, allowing the caller to manage the `VM` lifecycle and choose the appropriate execution strategy (e.g., single run vs. repeated benchmark iterations). The `NativeFunction` type is now exclusively used for true Rust intrinsics. The benchmark runner has also been updated to reuse a single `VM` across iterations, improving performance by avoiding repeated VM allocations. --- docs/Analysis_Environment.md | 2 +- src/ast/environment.rs | 72 ++++++++++++++++-------------------- src/utils/tester.rs | 13 ++++--- tests/closures.rs | 4 +- 4 files changed, 42 insertions(+), 49 deletions(-) diff --git a/docs/Analysis_Environment.md b/docs/Analysis_Environment.md index 0f37840..df424df 100644 --- a/docs/Analysis_Environment.md +++ b/docs/Analysis_Environment.md @@ -24,4 +24,4 @@ Der Code enthält einige starke Indikatoren für unscharfe Systemgrenzen ("Leaky * ~~**Verwaschene Kompilierungsschritte:**~~ **✓ Behoben (Design korrekt, ein Detail bereinigt).** Die drei Methoden bilden eine saubere Adapter-Hierarchie: `compile_pipeline` (Kern, Diagnostics-Sink) ← `compile` (public API, merged Parse+Compile-Fehler in `CompilationResult`) / `compile_syntax` (privater Adapter für Modul-Loader, konvertiert zu `Result<_, String>`). Das unterschiedliche Error-Reporting ist situations-appropriat, nicht inkonsistent. **Behobener Design-Geruch:** `dump_ast` umging zuvor `compile()` und verschluckte Parse-Fehler still. Behoben: `dump_ast` delegiert jetzt an `compile(source).into_result()?`. * ~~**Die `specialize_node`-Closure:**~~ **✓ Kein Problem (Design korrekt verstanden).** Die `compiler`-Closure ist kein Zeichen unsauberer Kapselung — im Gegenteil. `Specializer` ist bewusst von TypeChecker, Optimizer und VM entkoppelt; der `CompileFunc`-Typ (`specializer.rs:15`) ist der explizite **Dependency-Injection-Punkt** (Strategy Pattern). Die innere Pipeline (TypeCheck → Analyze → sub-Specialize → Optimize → Lower → VM.run) **dupliziert nicht** die äußere — ihr Zweck ist fundamental verschieden: sie erzeugt einen gecachten `Value` (compile-and-evaluate), während die äußere Pipeline einen `ExecNode` für die Skriptausführung erzeugt. Die Closure captured 6 `Rc>`-Handles statt `&self`, weil Rust keine `self`-Referenz in eine `'static`-Closure erlaubt — das ist der korrekte Rust-Weg. **Design-Limitation (dokumentiert, kein Bug):** Der `sub_specializer` innerhalb der Closure hat `compiler: None`, was Endlos-Rekursion verhindert, aber bedeutet, dass nur eine Spezialisierungsebene pro Aufruf expandiert wird. * ~~**Type-Checker Hack:**~~ **✓ Kein Problem (Kommentar bereinigt).** Der ursprüngliche Kommentar war veraltet und irreführend. Das `BoundLike`-Trait vereinigt alle Phasen mit identischer Binding-Struktur (`BoundPhase`, `TypedPhase`, `AnalyzedPhase`). `TypeChecker::check_node_as_bound()` ist korrekte Generic-Programmierung — kein Hack. Die Phasen passen sauber ineinander. Der Kommentar wurde durch eine korrekte Erklärung des Monomorphisierungs-Designs ersetzt. -* **Konzeptioneller Bruch bei `instantiate`:** Diese Methode nimmt einen fertig kompilierten Ast (`ExecNode`) und wickelt ihn in eine `NativeFunction` ein. Dadurch wird ein in Myc geschriebenes Skript in der Registry strukturell als "native Rust-Funktion" getarnt. Das ist pragmatisch, verwischt aber die semantische Grenze zwischen User-Code (Closures) und echten Runtime-Intrinsics. +* ~~**Konzeptioneller Bruch bei `instantiate`:**~~ **✓ Behoben.** `instantiate` gibt jetzt `Result, String>` zurück statt `Rc`. Die VM-Erzeugung und -Ausführung liegt beim Aufrufer (`run_script_compiled`, Benchmark-Runner), der die passende Strategie kennt (einmaliger Run vs. N Iterationen mit VM-Reuse). `NativeFunction` bleibt ausschließlich für echte Rust-Intrinsics (RTL). Bonus: der Benchmark-Runner erstellt jetzt eine VM für N Iterationen statt N VMs — `run_with_args` resettet `stack` und `frames` am Anfang jedes Aufrufs, VM-Reuse ist sicher. diff --git a/src/ast/environment.rs b/src/ast/environment.rs index a9d36cc..078ed34 100644 --- a/src/ast/environment.rs +++ b/src/ast/environment.rs @@ -633,55 +633,46 @@ impl Environment { Lowering::lower(optimized) } - pub fn instantiate(&self, node: ExecNode) -> Rc { - let root_values = self.root_values.clone(); - if let NodeKind::Lambda { - params, - body, - info, - } = &node.kind + /// Converts a linked `ExecNode` into a callable `Closure`. + /// + /// Fast path: if the node is already a top-level lambda without upvalues, + /// the `Closure` is constructed directly without running the VM. + /// + /// Slow path: the node is executed once to obtain its resulting `Closure` + /// value (e.g. a `do`-block that returns a lambda). + /// + /// The caller is responsible for creating a `VM` and invoking the closure + /// via `vm.run_with_args`. This keeps VM lifecycle and error handling at + /// the call site, where the required strategy (single run vs. repeated + /// benchmark iterations) is known. + pub fn instantiate(&self, node: ExecNode) -> Result, String> { + // Fast path: top-level lambda without upvalues — build Closure directly. + if let NodeKind::Lambda { params, body, info } = &node.kind && info.upvalues.is_empty() { - let closure = Rc::new(Closure::new( + return Ok(Rc::new(Closure::new( params.clone(), body.ty.original.clone(), body.clone(), Vec::new(), info.positional_count, node.ty.stack_size, - )); - return Rc::new(NativeFunction { - purity: Purity::Impure, - func: Rc::new(move |args| { - let mut vm = VM::new(root_values.clone()); - match vm.run_with_args(closure.clone(), args) { - Ok(v) => v, - Err(e) => panic!("Myc Runtime Error: {}", e), - } - }), - }); + ))); } - let exec_node = Rc::new(node); - Rc::new(NativeFunction { - purity: Purity::Impure, - func: Rc::new(move |args| { - let mut vm = VM::new(root_values.clone()); - let res = match vm.run(&exec_node) { - Ok(v) => v, - Err(e) => panic!("Myc Runtime Error: {}", e), - }; + // Slow path: run the node once to extract the resulting Closure. + let mut vm = VM::new(self.root_values.clone()); + let res = vm.run(&node)?; + if let Value::Closure(obj) = res { + Ok(obj) + } else { + Err("Script did not produce a callable closure".to_string()) + } + } - if let Value::Closure(obj) = &res { - match vm.run_with_args(obj.clone(), args) { - Ok(v) => v, - Err(e) => panic!("Myc Runtime Error (Closure): {}", e), - } - } else { - res - } - }), - }) + /// Creates a new `VM` connected to this environment's global scope. + pub fn create_vm(&self) -> VM { + VM::new(self.root_values.clone()) } fn specialize_node(&self, node: AnalyzedNode) -> AnalyzedNode { @@ -780,8 +771,9 @@ impl Environment { pub fn run_script_compiled(&self, compiled: TypedNode) -> Result { let linked = self.link(compiled); - let func = self.instantiate(linked); - let res = (func.func)(&[]); + let closure = self.instantiate(linked)?; + let mut vm = self.create_vm(); + let res = vm.run_with_args(closure, &[])?; self.run_pipeline(); Ok(res) } diff --git a/src/utils/tester.rs b/src/utils/tester.rs index b16faf6..f0238ed 100644 --- a/src/utils/tester.rs +++ b/src/utils/tester.rs @@ -55,7 +55,7 @@ pub fn run_functional_tests_with_optimization(enabled: bool) -> Vec results } -pub fn run_benchmarks(update: bool, filter: Option<&str>) -> Vec { +pub fn ca(update: bool, filter: Option<&str>) -> Vec { let mut results = Vec::new(); let entries = fs::read_dir("examples").unwrap(); let is_release = !cfg!(debug_assertions); @@ -104,18 +104,19 @@ pub fn run_benchmarks(update: bool, filter: Option<&str>) -> Vec Result { let env = Environment::new(); - - // Link once per sample let linked = env.link(node.clone()); - let func = env.instantiate(linked); + let closure = env.instantiate(linked)?; + let mut vm = env.create_vm(); let mut total = Duration::ZERO; for _ in 0..n { let start = Instant::now(); - let _ = (func.func)(&[]); + let _ = vm.run_with_args(closure.clone(), &[])?; total += start.elapsed(); } Ok(total) diff --git a/tests/closures.rs b/tests/closures.rs index 6a9ec10..a2af81e 100644 --- a/tests/closures.rs +++ b/tests/closures.rs @@ -18,8 +18,8 @@ fn test_closure_modification_from_source() { .into_result() .expect("Failed to compile"); let linked = env.link(compiled); - let func = env.instantiate(linked); - let result: Result = Ok((func.func)(&[])); + let closure = env.instantiate(linked).expect("Failed to instantiate"); + let result = env.create_vm().run_with_args(closure, &[]); match result { Ok(Value::Int(20)) => (),