From 36ccc735f9a5c7c1d8b7dea971e4e26ccd03b64e Mon Sep 17 00:00:00 2001 From: imxyy_soope_ Date: Sat, 10 Jan 2026 11:50:28 +0800 Subject: [PATCH] refactor: avoid Pin hack --- Cargo.lock | 1 - nix-js/Cargo.toml | 3 +- nix-js/src/context.rs | 149 +++++++++++++++---------------- nix-js/src/context/downgrade.rs | 10 +-- nix-js/src/context/drop_guard.rs | 41 +++++++++ nix-js/src/ir/downgrade.rs | 1 + nix-js/src/ir/utils.rs | 9 +- nix-js/src/runtime.rs | 78 +++++++--------- 8 files changed, 158 insertions(+), 134 deletions(-) create mode 100644 nix-js/src/context/drop_guard.rs diff --git a/Cargo.lock b/Cargo.lock index ed4630a..73873a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1053,7 +1053,6 @@ dependencies = [ "itertools 0.14.0", "mimalloc", "nix-js-macros", - "pin-project", "regex", "rnix", "rustyline", diff --git a/nix-js/Cargo.toml b/nix-js/Cargo.toml index 6d560c0..081f882 100644 --- a/nix-js/Cargo.toml +++ b/nix-js/Cargo.toml @@ -12,12 +12,11 @@ anyhow = "1.0" rustyline = "14.0" derive_more = { version = "2", features = ["full"] } -pin-project = "1" +thiserror = "2" hashbrown = "0.16" string-interner = "0.19" -thiserror = "2" itertools = "0.14" regex = "1.11" diff --git a/nix-js/src/context.rs b/nix-js/src/context.rs index 972c6c5..04e5289 100644 --- a/nix-js/src/context.rs +++ b/nix-js/src/context.rs @@ -1,5 +1,4 @@ use std::path::PathBuf; -use std::pin::Pin; use std::ptr::NonNull; use hashbrown::HashMap; @@ -9,109 +8,87 @@ use string_interner::DefaultStringInterner; use crate::codegen::{CodegenContext, Compile}; use crate::error::{Error, Result}; use crate::ir::{Builtin, DowngradeContext, ExprId, Ir, SymId}; -use crate::runtime::Runtime; +use crate::runtime::{Runtime, RuntimeCtx}; use crate::value::Value; use downgrade::DowngradeCtx; +use drop_guard::{PathDropGuard, PathStackProvider}; mod downgrade; +mod drop_guard; + +mod private { + use super::*; + use std::ops::DerefMut; + use std::ptr::NonNull; + + pub struct CtxPtr(NonNull); + impl CtxPtr { + pub fn new(ctx: &mut Ctx) -> Self { + unsafe { CtxPtr(NonNull::new_unchecked(ctx)) } + } + fn as_ref(&self) -> &Ctx { + // SAFETY: This is safe since inner `NonNull` is obtained from `&mut Ctx` + unsafe { self.0.as_ref() } + } + fn as_mut(&mut self) -> &mut Ctx { + // SAFETY: This is safe since inner `NonNull` is obtained from `&mut Ctx` + unsafe { self.0.as_mut() } + } + } + impl PathStackProvider for CtxPtr { + fn path_stack(&mut self) -> &mut Vec { + &mut self.as_mut().path_stack + } + } + + impl RuntimeCtx for CtxPtr { + fn get_current_dir(&self) -> PathBuf { + self.as_ref().get_current_dir() + } + fn push_path_stack(&mut self, path: PathBuf) -> impl DerefMut { + PathDropGuard::new(path, self) + } + fn compile_code(&mut self, expr: &str) -> Result { + self.as_mut().compile_code(expr) + } + } +} +use private::CtxPtr; pub struct Context { - ctx: Pin>, - runtime: Runtime, -} - -pub(crate) struct CtxPtr(NonNull); -impl CtxPtr { - pub(crate) unsafe fn as_ref(&self) -> &Ctx { - unsafe { self.0.as_ref() } - } - pub(crate) unsafe fn as_mut(&mut self) -> Pin<&mut Ctx> { - unsafe { Pin::new_unchecked(self.0.as_mut()) } - } + ctx: Ctx, + runtime: Runtime, } impl Context { pub fn new() -> Result { - let mut ctx = Box::pin(Ctx::new()); - let ptr = unsafe { CtxPtr(NonNull::new_unchecked(Pin::get_unchecked_mut(ctx.as_mut()))) }; - let runtime = Runtime::new(ptr)?; + let ctx = Ctx::new(); + let runtime = Runtime::new()?; Ok(Self { ctx, runtime }) } pub fn eval_code(&mut self, expr: &str) -> Result { // Initialize `path_stack` with current directory for relative path resolution - let mut guard = PathDropGuard::new_cwd(self.ctx.as_mut())?; + let mut guard = PathDropGuard::new_cwd(&mut self.ctx)?; let ctx = guard.as_ctx(); - let root = rnix::Root::parse(expr); - if !root.errors().is_empty() { - return Err(Error::parse_error(root.errors().iter().join("; "))); - } - - #[allow(clippy::unwrap_used)] - // Always `Some` since there is no parse error - let root = ctx - .as_mut() - .downgrade_ctx() - .downgrade(root.tree().expr().unwrap())?; - let code = ctx.get_ir(root).compile(Pin::get_ref(ctx.as_ref())); - let code = format!("Nix.force({})", code); - println!("[DEBUG] generated code: {}", &code); - self.runtime.eval(code) + let code = ctx.compile_code(expr)?; + self.runtime.eval(code, CtxPtr::new(&mut self.ctx)) } #[allow(dead_code)] pub(crate) fn eval_js(&mut self, code: String) -> Result { - self.runtime.eval(code) + self.runtime.eval(code, CtxPtr::new(&mut self.ctx)) } } -#[pin_project::pin_project(PinnedDrop)] pub(crate) struct Ctx { irs: Vec, symbols: DefaultStringInterner, global: NonNull>, path_stack: Vec, - #[pin] - _marker: std::marker::PhantomPinned, -} - -#[pin_project::pinned_drop] -impl PinnedDrop for Ctx { - fn drop(self: Pin<&mut Self>) { - unsafe { - drop(Box::from_raw(self.global.as_ptr())); - } - } -} - -pub(crate) struct PathDropGuard<'ctx> { - ctx: Pin<&'ctx mut Ctx>, -} - -impl<'ctx> PathDropGuard<'ctx> { - pub(crate) fn new(path: PathBuf, mut ctx: Pin<&'ctx mut Ctx>) -> Self { - ctx.as_mut().project().path_stack.push(path); - Self { ctx } - } - pub(crate) fn new_cwd(mut ctx: Pin<&'ctx mut Ctx>) -> Result { - let cwd = std::env::current_dir() - .map_err(|err| Error::downgrade_error(format!("cannot get cwd: {err}")))?; - let virtual_file = cwd.join("__eval__.nix"); - ctx.as_mut().project().path_stack.push(virtual_file); - Ok(Self { ctx }) - } - pub(crate) fn as_ctx<'a>(&'a mut self) -> &'a mut Pin<&'ctx mut Ctx> { - &mut self.ctx - } -} - -impl Drop for PathDropGuard<'_> { - fn drop(&mut self) { - self.ctx.as_mut().project().path_stack.pop(); - } } impl Default for Ctx { @@ -165,7 +142,6 @@ impl Default for Ctx { irs, global: unsafe { NonNull::new_unchecked(Box::leak(Box::new(global))) }, path_stack: Vec::new(), - _marker: std::marker::PhantomPinned, } } } @@ -175,7 +151,7 @@ impl Ctx { Self::default() } - pub(crate) fn downgrade_ctx<'a>(self: Pin<&'a mut Self>) -> DowngradeCtx<'a> { + pub(crate) fn downgrade_ctx<'a>(&'a mut self) -> DowngradeCtx<'a> { let global_ref = unsafe { self.global.as_ref() }; DowngradeCtx::new(self, global_ref) } @@ -190,6 +166,23 @@ impl Ctx { .expect("path in path_stack should always have a parent dir. this is a bug") .to_path_buf() } + + fn compile_code(&mut self, expr: &str) -> Result { + let root = rnix::Root::parse(expr); + if !root.errors().is_empty() { + return Err(Error::parse_error(root.errors().iter().join("; "))); + } + + #[allow(clippy::unwrap_used)] + // Always `Some` since there is no parse error + let root = self + .downgrade_ctx() + .downgrade(root.tree().expr().unwrap())?; + let code = self.get_ir(root).compile(self); + let code = format!("Nix.force({})", code); + println!("[DEBUG] generated code: {}", &code); + Ok(code) + } } impl CodegenContext for Ctx { @@ -202,6 +195,12 @@ impl CodegenContext for Ctx { } } +impl PathStackProvider for Ctx { + fn path_stack(&mut self) -> &mut Vec { + &mut self.path_stack + } +} + #[cfg(test)] #[allow(clippy::unwrap_used)] mod test { diff --git a/nix-js/src/context/downgrade.rs b/nix-js/src/context/downgrade.rs index 524de25..9e4e099 100644 --- a/nix-js/src/context/downgrade.rs +++ b/nix-js/src/context/downgrade.rs @@ -1,5 +1,3 @@ -use std::pin::Pin; - use hashbrown::HashMap; use crate::codegen::CodegenContext; @@ -32,14 +30,14 @@ impl<'a, 'ctx> ScopeGuard<'a, 'ctx> { } pub struct DowngradeCtx<'ctx> { - ctx: Pin<&'ctx mut Ctx>, + ctx: &'ctx mut Ctx, irs: Vec>, scopes: Vec>, arg_id: usize, } impl<'ctx> DowngradeCtx<'ctx> { - pub fn new(ctx: Pin<&'ctx mut Ctx>, global: &'ctx HashMap) -> Self { + pub fn new(ctx: &'ctx mut Ctx, global: &'ctx HashMap) -> Self { Self { scopes: vec![Scope::Global(global)], irs: vec![], @@ -62,7 +60,7 @@ impl DowngradeContext for DowngradeCtx<'_> { } fn new_sym(&mut self, sym: String) -> SymId { - self.ctx.as_mut().project().symbols.get_or_intern(sym) + self.ctx.symbols.get_or_intern(sym) } fn get_sym(&self, id: SymId) -> &str { @@ -144,8 +142,6 @@ impl DowngradeContext for DowngradeCtx<'_> { fn downgrade(mut self, root: rnix::ast::Expr) -> Result { let root = root.downgrade(&mut self)?; self.ctx - .as_mut() - .project() .irs .extend(self.irs.into_iter().map(Option::unwrap)); Ok(root) diff --git a/nix-js/src/context/drop_guard.rs b/nix-js/src/context/drop_guard.rs new file mode 100644 index 0000000..58ce006 --- /dev/null +++ b/nix-js/src/context/drop_guard.rs @@ -0,0 +1,41 @@ +use std::ops::{Deref, DerefMut}; +use std::path::PathBuf; + +use crate::error::{Error, Result}; + +pub trait PathStackProvider { + fn path_stack(&mut self) -> &mut Vec; +} + +pub struct PathDropGuard<'ctx, Ctx: PathStackProvider> { + ctx: &'ctx mut Ctx, +} + +impl<'ctx, Ctx: PathStackProvider> PathDropGuard<'ctx, Ctx> { + pub fn new(path: PathBuf, ctx: &'ctx mut Ctx) -> Self { + ctx.path_stack().push(path); + Self { ctx } + } + pub fn new_cwd(ctx: &'ctx mut Ctx) -> Result { + let cwd = std::env::current_dir() + .map_err(|err| Error::downgrade_error(format!("cannot get cwd: {err}")))?; + let virtual_file = cwd.join("__eval__.nix"); + ctx.path_stack().push(virtual_file); + Ok(Self { ctx }) + } + pub fn as_ctx(&mut self) -> &mut Ctx { + self.ctx + } +} + +impl Deref for PathDropGuard<'_, Ctx> { + type Target = Ctx; + fn deref(&self) -> &Self::Target { + self.ctx + } +} +impl DerefMut for PathDropGuard<'_, Ctx> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.ctx + } +} diff --git a/nix-js/src/ir/downgrade.rs b/nix-js/src/ir/downgrade.rs index 59f2a15..65dbd5c 100644 --- a/nix-js/src/ir/downgrade.rs +++ b/nix-js/src/ir/downgrade.rs @@ -77,6 +77,7 @@ impl Downgrade for ast::Path { path_str } else { let current_dir = ctx.get_current_dir(); + dbg!(¤t_dir); current_dir .join(&path_str) diff --git a/nix-js/src/ir/utils.rs b/nix-js/src/ir/utils.rs index 1960339..8cee768 100644 --- a/nix-js/src/ir/utils.rs +++ b/nix-js/src/ir/utils.rs @@ -234,7 +234,7 @@ where Ctx: DowngradeContext, F: FnOnce(&mut Ctx, &[SymId]) -> Result, { - // 1. Collect all top-level binding keys + // Collect all top-level binding keys let mut binding_syms = HashSet::new(); for entry in &entries { @@ -271,14 +271,14 @@ where let binding_keys: Vec<_> = binding_syms.into_iter().collect(); - // 2. Reserve slots for bindings + // Reserve slots for bindings let slots_iter = ctx.reserve_slots(binding_keys.len()); let slots_clone = slots_iter.clone(); - // 3. Create let scope bindings + // Create let scope bindings let let_bindings: HashMap<_, _> = binding_keys.iter().copied().zip(slots_iter).collect(); - // 4. Process entries in let scope + // Process entries in let scope let body = ctx.with_let_scope(let_bindings, |ctx| { // Collect all bindings in a temporary AttrSet let mut temp_attrs = AttrSet { @@ -313,6 +313,5 @@ where body_fn(ctx, &binding_keys) })?; - // 5. Return the slots and body Ok((slots_clone.collect(), body)) } diff --git a/nix-js/src/runtime.rs b/nix-js/src/runtime.rs index 7cf1cad..94b268d 100644 --- a/nix-js/src/runtime.rs +++ b/nix-js/src/runtime.rs @@ -1,38 +1,40 @@ use std::borrow::Cow; -use std::pin::Pin; +use std::marker::PhantomData; +use std::ops::DerefMut; +use std::path::PathBuf; use std::sync::Once; -use deno_core::{Extension, ExtensionFileSource, JsRuntime, OpDecl, OpState, RuntimeOptions, v8}; +use deno_core::{Extension, ExtensionFileSource, JsRuntime, OpState, RuntimeOptions, v8}; use deno_error::JsErrorClass; -use crate::codegen::{CodegenContext, Compile}; -use crate::context::{CtxPtr, PathDropGuard}; use crate::error::{Error, Result}; -use crate::ir::DowngradeContext; use crate::value::{AttrSet, List, Symbol, Value}; type ScopeRef<'p, 's> = v8::PinnedRef<'p, v8::HandleScope<'s>>; type LocalValue<'a> = v8::Local<'a, v8::Value>; type LocalSymbol<'a> = v8::Local<'a, v8::Symbol>; -fn runtime_extension(ctx: CtxPtr) -> Extension { +pub(crate) trait RuntimeCtx: 'static { + fn get_current_dir(&self) -> PathBuf; + fn push_path_stack(&mut self, path: PathBuf) -> impl DerefMut; + fn compile_code(&mut self, code: &str) -> Result; +} + +fn runtime_extension() -> Extension { const ESM: &[ExtensionFileSource] = &deno_core::include_js_files!(nix_runtime dir "runtime-ts/dist", "runtime.js"); - const OPS: &[OpDecl] = &[ - op_import(), + let ops = vec![ + op_import::(), op_read_file(), op_path_exists(), - op_resolve_path(), + op_resolve_path::(), ]; Extension { name: "nix_runtime", esm_files: Cow::Borrowed(ESM), esm_entry_point: Some("ext:nix_runtime/runtime.js"), - ops: Cow::Borrowed(OPS), - op_state_fn: Some(Box::new(move |state| { - state.put(ctx); - })), + ops: Cow::Owned(ops), enabled: true, ..Default::default() } @@ -67,9 +69,11 @@ use private::NixError; #[deno_core::op2] #[string] -fn op_import(state: &mut OpState, #[string] path: String) -> std::result::Result { - let ptr = state.borrow_mut::(); - let ctx = unsafe { ptr.as_mut() }; +fn op_import( + state: &mut OpState, + #[string] path: String, +) -> std::result::Result { + let ctx = state.borrow_mut::(); let current_dir = ctx.get_current_dir(); let mut absolute_path = current_dir @@ -80,30 +84,13 @@ fn op_import(state: &mut OpState, #[string] path: String) -> std::result::Result absolute_path.push("default.nix") } - let mut guard = PathDropGuard::new(absolute_path.clone(), ctx); - let ctx = guard.as_ctx(); - let content = std::fs::read_to_string(&absolute_path) .map_err(|e| format!("Failed to read {}: {}", absolute_path.display(), e))?; - let root = rnix::Root::parse(&content); - if !root.errors().is_empty() { - return Err(format!( - "Parse error in {}: {:?}", - absolute_path.display(), - root.errors() - ) - .into()); - } + let mut guard = ctx.push_path_stack(absolute_path); + let ctx = guard.deref_mut(); - let expr = root.tree().expr().ok_or("No expression in file")?; - let expr_id = ctx - .as_mut() - .downgrade_ctx() - .downgrade(expr) - .map_err(|e| format!("Downgrade error: {}", e))?; - - Ok(ctx.get_ir(expr_id).compile(Pin::get_ref(ctx.as_ref()))) + Ok(ctx.compile_code(&content).map_err(|err| err.to_string())?) } #[deno_core::op2] @@ -119,12 +106,11 @@ fn op_path_exists(#[string] path: String) -> bool { #[deno_core::op2] #[string] -fn op_resolve_path( +fn op_resolve_path( state: &mut OpState, #[string] path: String, ) -> std::result::Result { - let ptr = state.borrow::(); - let ctx = unsafe { ptr.as_ref() }; + let ctx = state.borrow::(); // If already absolute, return as-is if path.starts_with('/') { @@ -141,14 +127,15 @@ fn op_resolve_path( .map_err(|e| format!("Failed to resolve path {}: {}", path, e))?) } -pub(crate) struct Runtime { +pub(crate) struct Runtime { js_runtime: JsRuntime, is_thunk_symbol: v8::Global, primop_metadata_symbol: v8::Global, + _marker: PhantomData, } -impl Runtime { - pub(crate) fn new(ctx: CtxPtr) -> Result { +impl Runtime { + pub(crate) fn new() -> Result { // Initialize V8 once static INIT: Once = Once::new(); INIT.call_once(|| { @@ -159,7 +146,7 @@ impl Runtime { }); let mut js_runtime = JsRuntime::new(RuntimeOptions { - extensions: vec![runtime_extension(ctx)], + extensions: vec![runtime_extension::()], ..Default::default() }); @@ -172,10 +159,13 @@ impl Runtime { js_runtime, is_thunk_symbol, primop_metadata_symbol, + _marker: PhantomData, }) } - pub(crate) fn eval(&mut self, script: String) -> Result { + pub(crate) fn eval(&mut self, script: String, ctx: Ctx) -> Result { + self.js_runtime.op_state().borrow_mut().put(ctx); + let global_value = self .js_runtime .execute_script("", script)