From d57bfc3004ae209e954f71abcf3af6a6368d3904 Mon Sep 17 00:00:00 2001 From: Jeffrey Charles Date: Tue, 10 Dec 2024 09:30:24 -0500 Subject: [PATCH] Remove use of Intrinsic (#860) --- crates/javy/src/apis/console/mod.rs | 29 ++---------- crates/javy/src/apis/json.rs | 29 +++--------- crates/javy/src/apis/mod.rs | 11 +---- crates/javy/src/apis/random/mod.rs | 14 ++---- crates/javy/src/apis/stream_io/mod.rs | 14 ++---- crates/javy/src/apis/text_encoding/mod.rs | 14 ++---- crates/javy/src/runtime.rs | 55 +++++++++++++---------- 7 files changed, 53 insertions(+), 113 deletions(-) diff --git a/crates/javy/src/apis/console/mod.rs b/crates/javy/src/apis/console/mod.rs index 2bbcb629..449b0dc7 100644 --- a/crates/javy/src/apis/console/mod.rs +++ b/crates/javy/src/apis/console/mod.rs @@ -1,35 +1,14 @@ -use std::io::{stderr, stdout, Write}; -use std::ptr::NonNull; +use std::io::Write; use crate::{ hold, hold_and_release, - quickjs::{context::Intrinsic, prelude::MutFn, qjs, Ctx, Function, Object, Value}, + quickjs::{prelude::MutFn, Ctx, Function, Object, Value}, to_js_error, val_to_string, Args, }; use anyhow::Result; -/// An implementation of JavaScript `console` APIs. -/// This implementation is *not* standard in the sense that it redirects the output of -/// `console.log` to stderr. -pub(crate) struct NonStandardConsole; - -/// An implemetation of JavaScript `console` APIs. This implementation is -/// standard as it redirects `console.log` to stdout and `console.error` to -/// stderr. -pub(crate) struct Console; - -impl Intrinsic for NonStandardConsole { - unsafe fn add_intrinsic(ctx: NonNull) { - register(Ctx::from_raw(ctx), stderr(), stderr()).expect("registering console to succeed"); - } -} - -impl Intrinsic for Console { - unsafe fn add_intrinsic(ctx: NonNull) { - register(Ctx::from_raw(ctx), stdout(), stderr()).expect("registering console to succeed"); - } -} - +/// Register a `console` object on the global object with `.log` and `.error` +/// streams. pub(crate) fn register(this: Ctx<'_>, mut log_stream: T, mut error_stream: U) -> Result<()> where T: Write + 'static, diff --git a/crates/javy/src/apis/json.rs b/crates/javy/src/apis/json.rs index 5a1d2c2c..c9f2ed04 100644 --- a/crates/javy/src/apis/json.rs +++ b/crates/javy/src/apis/json.rs @@ -20,10 +20,9 @@ use crate::{ hold, hold_and_release, json, quickjs::{ - context::Intrinsic, function::This, prelude::{MutFn, Rest}, - qjs, Ctx, Exception, Function, Object, String as JSString, Value, + Ctx, Exception, Function, Object, String as JSString, Value, }, to_js_error, val_to_string, Args, }; @@ -35,32 +34,14 @@ use simd_json::Error as SError; use anyhow::{anyhow, bail, Result}; use std::{ io::{Read, Write}, - ptr::NonNull, sync::OnceLock, time::SystemTime, }; static DEFAULT_PARSE_KEY: OnceLock = OnceLock::new(); -/// Intrinsic to attach faster JSON.{parse/stringify} functions. -pub struct Json; - -/// Intrinsic to attach functions under the `Javy.JSON` namespace. -pub struct JavyJson; - -impl Intrinsic for JavyJson { - unsafe fn add_intrinsic(ctx: NonNull) { - register_javy_json(Ctx::from_raw(ctx)).expect("registering Javy.JSON builtins to succeed") - } -} - -impl Intrinsic for Json { - unsafe fn add_intrinsic(ctx: NonNull) { - register(Ctx::from_raw(ctx)).expect("registering JSON builtins to succeed") - } -} - -fn register<'js>(this: Ctx<'js>) -> Result<()> { +/// Use SIMD implementations for `JSON.parse` and `JSON.stringify`. +pub(crate) fn register<'js>(this: Ctx<'js>) -> Result<()> { let global = this.globals(); let json: Object = global.get("JSON")?; @@ -193,7 +174,9 @@ fn call_json_stringify(args: Args<'_>) -> Result> { } } -fn register_javy_json(this: Ctx<'_>) -> Result<()> { +/// Register `Javy.JSON.fromStdin` and `Javy.JSON.toStdout` functions on the +/// global object. +pub(crate) fn register_javy_json(this: Ctx<'_>) -> Result<()> { let globals = this.globals(); let javy = if globals.get::<_, Object>("Javy").is_err() { Object::new(this.clone())? diff --git a/crates/javy/src/apis/mod.rs b/crates/javy/src/apis/mod.rs index 67979d7e..c9d2d2a7 100644 --- a/crates/javy/src/apis/mod.rs +++ b/crates/javy/src/apis/mod.rs @@ -1,8 +1,6 @@ //! A collection of APIs for Javy. //! -//! APIs are enabled through the the [`Config`](crate::Config) and are defined -//! in term of the [`Intrinsic`](rquickjs::context::Intrinsic) provided by -//! rquickjs. +//! APIs are enabled through the the [`Config`](crate::Config). //! //! Example usage: //! ```rust @@ -62,10 +60,3 @@ pub(crate) mod json; pub(crate) mod random; pub(crate) mod stream_io; pub(crate) mod text_encoding; - -pub(crate) use console::*; -#[cfg(feature = "json")] -pub(crate) use json::*; -pub(crate) use random::*; -pub(crate) use stream_io::*; -pub(crate) use text_encoding::*; diff --git a/crates/javy/src/apis/random/mod.rs b/crates/javy/src/apis/random/mod.rs index 5bf4d67e..15f9f163 100644 --- a/crates/javy/src/apis/random/mod.rs +++ b/crates/javy/src/apis/random/mod.rs @@ -1,15 +1,9 @@ -use crate::quickjs::{context::Intrinsic, prelude::Func, qjs, Ctx, Object}; +use crate::quickjs::{prelude::Func, Ctx, Object}; use anyhow::{Error, Result}; -pub struct Random; - -impl Intrinsic for Random { - unsafe fn add_intrinsic(ctx: std::ptr::NonNull) { - register(Ctx::from_raw(ctx)).expect("`Random` APIs to succeed") - } -} - -fn register(cx: Ctx) -> Result<()> { +/// Register a `random` object on the global object that seeds itself at first +/// execution. +pub(crate) fn register(cx: Ctx) -> Result<()> { let globals = cx.globals(); let math: Object<'_> = globals.get("Math").expect("Math global to be defined"); math.set("random", Func::from(fastrand::f64))?; diff --git a/crates/javy/src/apis/stream_io/mod.rs b/crates/javy/src/apis/stream_io/mod.rs index c0076ffb..083b3aac 100644 --- a/crates/javy/src/apis/stream_io/mod.rs +++ b/crates/javy/src/apis/stream_io/mod.rs @@ -3,19 +3,13 @@ use std::io::{Read, Stdin, Write}; use crate::{ hold, hold_and_release, - quickjs::{context::Intrinsic, qjs, qjs::JS_GetArrayBuffer, Ctx, Function, Object, Value}, + quickjs::{qjs::JS_GetArrayBuffer, Ctx, Function, Object, Value}, to_js_error, Args, }; -pub struct StreamIO; - -impl Intrinsic for StreamIO { - unsafe fn add_intrinsic(ctx: std::ptr::NonNull) { - register(Ctx::from_raw(ctx)).expect("Registering StreamIO functions to succeed"); - } -} - -fn register(this: Ctx<'_>) -> Result<()> { +/// Register `Javy.IO.readSync` and `Javy.IO.writeSync` functions on the +/// global object. +pub(crate) fn register(this: Ctx<'_>) -> Result<()> { let globals = this.globals(); if globals.get::<_, Object>("Javy").is_err() { globals.set("Javy", Object::new(this.clone())?)? diff --git a/crates/javy/src/apis/text_encoding/mod.rs b/crates/javy/src/apis/text_encoding/mod.rs index 7bfdd6af..9c57ef0b 100644 --- a/crates/javy/src/apis/text_encoding/mod.rs +++ b/crates/javy/src/apis/text_encoding/mod.rs @@ -3,22 +3,14 @@ use std::str; use crate::{ hold, hold_and_release, quickjs::{ - context::{EvalOptions, Intrinsic}, - qjs, Ctx, Exception, Function, String as JSString, TypedArray, Value, + context::EvalOptions, Ctx, Exception, Function, String as JSString, TypedArray, Value, }, to_js_error, to_string_lossy, Args, }; use anyhow::{anyhow, bail, Error, Result}; -pub struct TextEncoding; - -impl Intrinsic for TextEncoding { - unsafe fn add_intrinsic(ctx: std::ptr::NonNull) { - register(Ctx::from_raw(ctx)).expect("Register TextEncoding APIs to succeed"); - } -} - -fn register(this: Ctx<'_>) -> Result<()> { +/// Register `TextDecoder` and `TextEncoder` classes. +pub(crate) fn register(this: Ctx<'_>) -> Result<()> { let globals = this.globals(); globals.set( "__javy_decodeUtf8BufferToString", diff --git a/crates/javy/src/runtime.rs b/crates/javy/src/runtime.rs index 81011bb4..08a68214 100644 --- a/crates/javy/src/runtime.rs +++ b/crates/javy/src/runtime.rs @@ -1,20 +1,22 @@ // use crate::quickjs::JSContextRef; use super::from_js_error; +#[cfg(feature = "json")] +use crate::apis::json; use crate::{ - apis::{Console, NonStandardConsole, Random, StreamIO, TextEncoding}, + apis::{console, random, stream_io, text_encoding}, config::{JSIntrinsics, JavyIntrinsics}, Config, }; -#[cfg(feature = "json")] -use crate::apis::{JavyJson, Json}; - use anyhow::{bail, Result}; use rquickjs::{ context::{intrinsic, Intrinsic}, Context, Module, Runtime as QRuntime, }; -use std::mem::ManuallyDrop; +use std::{ + io::{stderr, stdout}, + mem::ManuallyDrop, +}; /// A JavaScript Runtime. /// @@ -57,17 +59,21 @@ impl Runtime { rt.set_memory_limit(cfg.memory_limit); rt.set_max_stack_size(cfg.max_stack_size); - // We always set Random given that the principles around snapshotting and - // random are applicable when using Javy from the CLI (the usage of - // Wizer from the CLI is not optional). - // NB: Users of Javy as a crate are welcome to switch this config, - // however note that the usage of a custom `Random` implementation - // should not affect the output of `Math.random()`. - let context = Context::custom::(rt)?; + // Using `Context::base` seems to have a bug where it tries to register + // the same intrinsic twice. + let context = Context::custom::<()>(rt)?; // We use `Context::with` to ensure that there's a proper lock on the // context, making it totally safe to add the intrinsics below. context.with(|ctx| { + // We always set Random given that the principles around snapshotting and + // random are applicable when using Javy from the CLI (the usage of + // Wizer from the CLI is not optional). + // NB: Users of Javy as a crate are welcome to switch this config, + // however note that the usage of a custom `Random` implementation + // should not affect the output of `Math.random()`. + random::register(ctx.clone()).expect("registering `random` APIs to succeed"); + if intrinsics.contains(JSIntrinsics::DATE) { unsafe { intrinsic::Date::add_intrinsic(ctx.as_raw()) } } @@ -88,11 +94,9 @@ impl Runtime { unsafe { intrinsic::Json::add_intrinsic(ctx.as_raw()) } } + #[cfg(feature = "json")] if cfg.simd_json_builtins { - #[cfg(feature = "json")] - unsafe { - Json::add_intrinsic(ctx.as_raw()) - } + json::register(ctx.clone()).expect("registering JSON builtins to succeed"); } if intrinsics.contains(JSIntrinsics::PROXY) { @@ -128,24 +132,27 @@ impl Runtime { } if intrinsics.contains(JSIntrinsics::TEXT_ENCODING) { - unsafe { TextEncoding::add_intrinsic(ctx.as_raw()) } + text_encoding::register(ctx.clone()) + .expect("registering TextEncoding APIs to succeed"); } if cfg.redirect_stdout_to_stderr { - unsafe { NonStandardConsole::add_intrinsic(ctx.as_raw()) } + console::register(ctx.clone(), stderr(), stderr()) + .expect("registering console to succeed"); } else { - unsafe { Console::add_intrinsic(ctx.as_raw()) } + console::register(ctx.clone(), stdout(), stderr()) + .expect("registering console to succeed"); } if javy_intrinsics.contains(JavyIntrinsics::STREAM_IO) { - unsafe { StreamIO::add_intrinsic(ctx.as_raw()) } + stream_io::register(ctx.clone()) + .expect("registering StreamIO functions to succeed"); } + #[cfg(feature = "json")] if javy_intrinsics.contains(JavyIntrinsics::JSON) { - #[cfg(feature = "json")] - unsafe { - JavyJson::add_intrinsic(ctx.as_raw()) - } + json::register_javy_json(ctx.clone()) + .expect("registering Javy.JSON builtins to succeed"); } });