From 3fbe176253faaec6605063f525c8bf9d795d41bb Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Fri, 30 Apr 2021 02:16:37 +0200 Subject: [PATCH 1/6] refactor(core): convert core.print() to a builtin op --- core/bindings.rs | 40 ---------------------------------------- core/core.js | 5 +++++ core/lib.rs | 1 + core/ops_builtin.rs | 21 +++++++++++++++++++++ runtime/web_worker.rs | 1 + runtime/worker.rs | 1 + 6 files changed, 29 insertions(+), 40 deletions(-) diff --git a/core/bindings.rs b/core/bindings.rs index 4574e2d4e37ae4..4823ee9ea5bf8c 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -14,7 +14,6 @@ use serde::Serialize; use serde_v8::to_v8; use std::convert::TryFrom; use std::convert::TryInto; -use std::io::{stdout, Write}; use std::option::Option; use url::Url; use v8::MapFnTo; @@ -22,9 +21,6 @@ use v8::MapFnTo; lazy_static::lazy_static! { pub static ref EXTERNAL_REFERENCES: v8::ExternalReferences = v8::ExternalReferences::new(&[ - v8::ExternalReference { - function: print.map_fn_to() - }, v8::ExternalReference { function: opcall.map_fn_to() }, @@ -118,7 +114,6 @@ pub fn initialize_context<'s>( deno_val.set(scope, core_key.into(), core_val.into()); // Bind functions to Deno.core.* - set_func(scope, core_val, "print", print); set_func(scope, core_val, "opcall", opcall); set_func( scope, @@ -282,41 +277,6 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { }; } -fn print( - scope: &mut v8::HandleScope, - args: v8::FunctionCallbackArguments, - _rv: v8::ReturnValue, -) { - let arg_len = args.length(); - if !(0..=2).contains(&arg_len) { - return throw_type_error(scope, "Expected a maximum of 2 arguments."); - } - - let obj = args.get(0); - let is_err_arg = args.get(1); - - let mut is_err = false; - if arg_len == 2 { - let int_val = match is_err_arg.integer_value(scope) { - Some(v) => v, - None => return throw_type_error(scope, "Invalid arugment. Argument 2 should indicate wheter or not to print to stderr."), - }; - is_err = int_val != 0; - }; - let tc_scope = &mut v8::TryCatch::new(scope); - let str_ = match obj.to_string(tc_scope) { - Some(s) => s, - None => v8::String::new(tc_scope, "").unwrap(), - }; - if is_err { - eprint!("{}", str_.to_rust_string_lossy(tc_scope)); - stdout().flush().unwrap(); - } else { - print!("{}", str_.to_rust_string_lossy(tc_scope)); - stdout().flush().unwrap(); - } -} - fn opcall<'s>( scope: &mut v8::HandleScope<'s>, args: v8::FunctionCallbackArguments, diff --git a/core/core.js b/core/core.js index fd9b2c3ea32f5f..f9097d572c6fde 100644 --- a/core/core.js +++ b/core/core.js @@ -124,11 +124,16 @@ opSync("op_close", rid); } + function print(str, ioChannel) { + opSync("op_print", [str, ioChannel]); + } + Object.assign(window.Deno.core, { opAsync, opSync, ops, close, + print, resources, registerErrorClass, handleAsyncMsgFromRust, diff --git a/core/lib.rs b/core/lib.rs index 37055bcc8d7095..b625a2d5629e50 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -65,6 +65,7 @@ pub use crate::ops::OpState; pub use crate::ops::OpTable; pub use crate::ops::PromiseId; pub use crate::ops_builtin::op_close; +pub use crate::ops_builtin::op_print; pub use crate::ops_builtin::op_resources; pub use crate::ops_json::op_async; pub use crate::ops_json::op_sync; diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs index 5c74c733003721..b57b58b4009af5 100644 --- a/core/ops_builtin.rs +++ b/core/ops_builtin.rs @@ -4,6 +4,7 @@ use crate::error::AnyError; use crate::resources::ResourceId; use crate::OpState; use crate::ZeroCopyBuf; +use std::io::{stdout, Write}; // TODO(@AaronO): provide these ops grouped as a runtime extension // e.g: @@ -43,3 +44,23 @@ pub fn op_close( Ok(()) } + +/// Builtin utility to print to stdout/stderr +/// +/// This op must be wrapped in `op_sync`. +pub fn op_print( + _state: &mut OpState, + args: (String, Option), + _zero_copy: Option, +) -> Result<(), AnyError> { + let (msg, channel) = args; + let is_err = channel.unwrap_or_default() != 0; + if is_err { + eprint!("{}", msg); + stdout().flush().unwrap(); + } else { + print!("{}", msg); + stdout().flush().unwrap(); + } + Ok(()) +} diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index 5feb0212cecede..7f369fb1b83a85 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -263,6 +263,7 @@ impl WebWorker { options.create_web_worker_cb.clone(), ); ops::reg_sync(js_runtime, "op_close", deno_core::op_close); + ops::reg_sync(js_runtime, "op_print", deno_core::op_print); ops::reg_sync(js_runtime, "op_resources", deno_core::op_resources); ops::io::init(js_runtime); diff --git a/runtime/worker.rs b/runtime/worker.rs index fddaf1f01d8a44..a0f5330c4a8ebf 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -149,6 +149,7 @@ impl MainWorker { options.create_web_worker_cb.clone(), ); ops::reg_sync(js_runtime, "op_close", deno_core::op_close); + ops::reg_sync(js_runtime, "op_print", deno_core::op_print); ops::reg_sync(js_runtime, "op_resources", deno_core::op_resources); ops::fs_events::init(js_runtime); ops::fs::init(js_runtime); From 6d9b2b4a4f39eb6b3318ca4602dfb819eae86e8e Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Fri, 30 Apr 2021 02:21:46 +0200 Subject: [PATCH 2/6] tweak --- core/ops_builtin.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs index b57b58b4009af5..f09c488ab462d5 100644 --- a/core/ops_builtin.rs +++ b/core/ops_builtin.rs @@ -50,11 +50,10 @@ pub fn op_close( /// This op must be wrapped in `op_sync`. pub fn op_print( _state: &mut OpState, - args: (String, Option), + args: (String, bool), _zero_copy: Option, ) -> Result<(), AnyError> { - let (msg, channel) = args; - let is_err = channel.unwrap_or_default() != 0; + let (msg, is_err) = args; if is_err { eprint!("{}", msg); stdout().flush().unwrap(); From c93960be41f847038daefcc88fb16f2bdf05d09d Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Fri, 30 Apr 2021 02:22:28 +0200 Subject: [PATCH 3/6] tweak --- core/core.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/core.js b/core/core.js index f9097d572c6fde..a46518c562b52a 100644 --- a/core/core.js +++ b/core/core.js @@ -124,8 +124,8 @@ opSync("op_close", rid); } - function print(str, ioChannel) { - opSync("op_print", [str, ioChannel]); + function print(str, isErr) { + opSync("op_print", [str, isErr]); } Object.assign(window.Deno.core, { From e9f4ed13b8e4bb05a9709cbf31635256955d8436 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Fri, 30 Apr 2021 02:24:34 +0200 Subject: [PATCH 4/6] tweak --- core/core.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/core.js b/core/core.js index a46518c562b52a..dad462a281d83f 100644 --- a/core/core.js +++ b/core/core.js @@ -124,7 +124,7 @@ opSync("op_close", rid); } - function print(str, isErr) { + function print(str, isErr = false) { opSync("op_print", [str, isErr]); } From c852cd926593e54b0dec16dd0bf1d278df730630 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Sat, 1 May 2021 15:13:47 +0200 Subject: [PATCH 5/6] declare op_print in builtin extension --- core/ops_builtin.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs index a36c41e9a975fa..546d9cf32d1ac3 100644 --- a/core/ops_builtin.rs +++ b/core/ops_builtin.rs @@ -18,6 +18,7 @@ pub(crate) fn init_builtins() -> Extension { )) .ops(vec![ ("op_close", op_sync(op_close)), + ("op_print", op_sync(op_print)), ("op_resources", op_sync(op_resources)), ]) .build() @@ -55,8 +56,6 @@ pub fn op_close( } /// Builtin utility to print to stdout/stderr -/// -/// This op must be wrapped in `op_sync`. pub fn op_print( _state: &mut OpState, args: (String, bool), From b834a7ea0dbf9aa03dd922214dcf195aa5fac074 Mon Sep 17 00:00:00 2001 From: Aaron O'Mullan Date: Sat, 1 May 2021 15:49:39 +0200 Subject: [PATCH 6/6] cli/tsc: fix core.print() usage --- cli/tsc/99_main_compiler.js | 11 ++++++----- cli/tsc/compiler.d.ts | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index f944b21b8d25fd..b4626374d81648 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -34,15 +34,16 @@ delete Object.prototype.__proto__; } } + function printStderr(msg) { + core.print(msg, true); + } + function debug(...args) { if (logDebug) { const stringifiedArgs = args.map((arg) => typeof arg === "string" ? arg : JSON.stringify(arg) ).join(" "); - // adding a non-zero integer value to the end of the debug string causes - // the message to be printed to stderr instead of stdout, which is better - // aligned to the behaviour of debug messages - core.print(`DEBUG ${logSource} - ${stringifiedArgs}\n`, 1); + printStderr(`DEBUG ${logSource} - ${stringifiedArgs}\n`); } } @@ -52,7 +53,7 @@ delete Object.prototype.__proto__; ? String(arg) : JSON.stringify(arg) ).join(" "); - core.print(`ERROR ${logSource} = ${stringifiedArgs}\n`, 1); + printStderr(`ERROR ${logSource} = ${stringifiedArgs}\n`); } class AssertionError extends Error { diff --git a/cli/tsc/compiler.d.ts b/cli/tsc/compiler.d.ts index 13e6564b44dbb4..e5ce12cd323f5d 100644 --- a/cli/tsc/compiler.d.ts +++ b/cli/tsc/compiler.d.ts @@ -36,7 +36,7 @@ declare global { // deno-lint-ignore no-explicit-any opSync(name: string, params: T): any; ops(): void; - print(msg: string, code?: number): void; + print(msg: string, stderr: bool): void; registerErrorClass( name: string, Ctor: typeof Error,