diff --git a/nova_cli/src/main.rs b/nova_cli/src/main.rs index e05d28c51..718fd5248 100644 --- a/nova_cli/src/main.rs +++ b/nova_cli/src/main.rs @@ -1,6 +1,8 @@ // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. +#![allow(unknown_lints, can_use_no_gc_scope)] + mod helper; mod theme; diff --git a/nova_lint/Cargo.toml b/nova_lint/Cargo.toml index 17649db46..07be2709a 100644 --- a/nova_lint/Cargo.toml +++ b/nova_lint/Cargo.toml @@ -16,6 +16,10 @@ crate-type = ["cdylib"] name = "agent_comes_first" path = "ui/agent_comes_first.rs" +[[example]] +name = "can_use_no_gc_scope" +path = "ui/can_use_no_gc_scope.rs" + [[example]] name = "gc_scope_comes_last" path = "ui/gc_scope_comes_last.rs" diff --git a/nova_lint/src/agent_comes_first.rs b/nova_lint/src/agent_comes_first.rs index c930389ce..5f39d8ecc 100644 --- a/nova_lint/src/agent_comes_first.rs +++ b/nova_lint/src/agent_comes_first.rs @@ -1,5 +1,5 @@ use clippy_utils::{diagnostics::span_lint_and_help, is_self}; -use rustc_hir::{def_id::LocalDefId, intravisit::FnKind, Body, FnDecl}; +use rustc_hir::{Body, FnDecl, def_id::LocalDefId, intravisit::FnKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::Span; diff --git a/nova_lint/src/can_use_no_gc_scope.rs b/nova_lint/src/can_use_no_gc_scope.rs new file mode 100644 index 000000000..a1ab881c1 --- /dev/null +++ b/nova_lint/src/can_use_no_gc_scope.rs @@ -0,0 +1,151 @@ +use std::ops::ControlFlow; + +use clippy_utils::{ + diagnostics::span_lint_and_sugg, fn_def_id, is_trait_impl_item, source::HasSession, + visitors::for_each_expr, +}; +use rustc_errors::Applicability; +use rustc_hir::{Body, Expr, ExprKind, FnDecl, def::Res, def_id::LocalDefId, intravisit::FnKind}; +use rustc_hir_analysis::lower_ty; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::{BytePos, Span}; + +use crate::{could_be_builtin_method_def, is_gc_scope_ty, is_no_gc_method, is_trait_item}; + +dylint_linting::declare_late_lint! { + /// ### What it does + /// + /// Checks that a function which only needs `NoGcScope` uses it instead of + /// `GcScope`. + /// + /// ### Why is this bad? + /// + /// You usually should use `NoGcScope` instead of `GcScope` if you don't + /// need the latter. The reason this is bad is that it forces the caller + /// to scope any heap references held past the call site unnecessarily. + /// + /// ### Example + /// + /// ```rust + /// fn foo(gc: GcScope<'_, '_>) {} + /// ``` + /// + /// Use instead: + /// + /// ```rust + /// fn foo(gc: NoGcScope<'_, '_>) {} + /// ``` + pub CAN_USE_NO_GC_SCOPE, + Warn, + "you should use `NoGcScope` instead of `GcScope` if you don't need the latter" +} + +impl<'tcx> LateLintPass<'tcx> for CanUseNoGcScope { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + span: Span, + _: LocalDefId, + ) { + if span.from_expansion() { + return; + } + + // Skip closures + if matches!(kind, FnKind::Closure) { + return; + } + + // Skip trait definitions and methods + let res = { + let body_id = cx.tcx.hir_body_owner(body.id()); + is_trait_impl_item(cx, body_id) || is_trait_item(cx, body_id) + }; + if res { + return; + } + + // Skip builtin methods + if could_be_builtin_method_def(cx, kind) { + return; + } + + // Skip `GcScope` methods + if let FnKind::Method(_, sig) = kind + && let Some(maybe_self) = sig.decl.inputs.first() + && is_gc_scope_ty(cx, &lower_ty(cx.tcx, maybe_self)) + { + return; + } + + let typeck = cx.typeck_results(); + + let Some(gc_scope) = body.params.iter().find(|param| { + let ty = typeck.pat_ty(param.pat); + is_gc_scope_ty(cx, &ty) + }) else { + // Either the function already takes `NoGcScope` or it doesn't take any + // at all in which case we don't need to lint. + return; + }; + + if for_each_expr(cx, body.value, |expr| { + // Checks if the expression is function or method call + if let Some(did) = fn_def_id(cx, expr) + // If we encountered either a `nogc` och `into_nogc` method call + // we skip them because they don't count as needing a `GcScope`. + && !is_no_gc_method(cx, did) + { + // Check if the function actually uses `GcScope` in its signature + let sig = cx.tcx.fn_sig(did).instantiate_identity().skip_binder(); + if sig.inputs().iter().any(|input| is_gc_scope_ty(cx, input)) { + return ControlFlow::Break(()); + } + } + + // Calls to closures and other functions may also use `GcScope`, + // we need to check those as well. + if let ExprKind::Call( + Expr { + kind: ExprKind::Path(qpath), + hir_id: path_hir_id, + .. + }, + args, + ) = expr.kind + && let Res::Local(_) = typeck.qpath_res(qpath, *path_hir_id) + && args.iter().any(|arg| { + let ty = typeck.expr_ty(arg); + is_gc_scope_ty(cx, &ty) + }) + { + return ControlFlow::Break(()); + } + + ControlFlow::Continue(()) + }) + .is_none() + { + // We didn't find any calls in the body that would require a `GcScope` + // so we can suggest using `NoGcScope` instead. + let ty_span = cx + .sess() + .source_map() + .span_until_char(gc_scope.ty_span, '<'); + // Trim the start of the span to just before the `GcScope` type name + let ty_span = ty_span.with_lo(ty_span.hi() - BytePos(7)); + span_lint_and_sugg( + cx, + CAN_USE_NO_GC_SCOPE, + ty_span, + "you can use `NoGcScope` instead of `GcScope` here", + "replace with", + "NoGcScope".to_owned(), + Applicability::MaybeIncorrect, + ); + } + } +} diff --git a/nova_lint/src/gc_scope_comes_last.rs b/nova_lint/src/gc_scope_comes_last.rs index ba422a23c..85f799eed 100644 --- a/nova_lint/src/gc_scope_comes_last.rs +++ b/nova_lint/src/gc_scope_comes_last.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_help; -use rustc_hir::{def_id::LocalDefId, intravisit::FnKind, Body, FnDecl}; +use rustc_hir::{Body, FnDecl, def_id::LocalDefId, intravisit::FnKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::Span; diff --git a/nova_lint/src/gc_scope_is_only_passed_by_value.rs b/nova_lint/src/gc_scope_is_only_passed_by_value.rs index fb59027bd..7078c020e 100644 --- a/nova_lint/src/gc_scope_is_only_passed_by_value.rs +++ b/nova_lint/src/gc_scope_is_only_passed_by_value.rs @@ -1,5 +1,5 @@ use clippy_utils::{diagnostics::span_lint_and_help, is_self}; -use rustc_hir::{def_id::LocalDefId, intravisit::FnKind, Body, FnDecl}; +use rustc_hir::{Body, FnDecl, def_id::LocalDefId, intravisit::FnKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::TyKind; use rustc_span::Span; diff --git a/nova_lint/src/lib.rs b/nova_lint/src/lib.rs index 3976e9879..6d61853fd 100644 --- a/nova_lint/src/lib.rs +++ b/nova_lint/src/lib.rs @@ -9,7 +9,9 @@ extern crate rustc_ast_pretty; extern crate rustc_data_structures; extern crate rustc_errors; extern crate rustc_hir; +extern crate rustc_hir_analysis; extern crate rustc_hir_pretty; +extern crate rustc_hir_typeck; extern crate rustc_index; extern crate rustc_infer; extern crate rustc_lexer; @@ -23,6 +25,7 @@ extern crate rustc_target; extern crate rustc_trait_selection; mod agent_comes_first; +mod can_use_no_gc_scope; mod gc_scope_comes_last; mod gc_scope_is_only_passed_by_value; mod no_it_performs_the_following; @@ -35,6 +38,7 @@ pub(crate) use utils::*; #[unsafe(no_mangle)] pub fn register_lints(sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { agent_comes_first::register_lints(sess, lint_store); + can_use_no_gc_scope::register_lints(sess, lint_store); gc_scope_comes_last::register_lints(sess, lint_store); gc_scope_is_only_passed_by_value::register_lints(sess, lint_store); no_it_performs_the_following::register_lints(sess, lint_store); diff --git a/nova_lint/src/utils.rs b/nova_lint/src/utils.rs index 073f64a0b..642d1dfee 100644 --- a/nova_lint/src/utils.rs +++ b/nova_lint/src/utils.rs @@ -1,4 +1,6 @@ -use rustc_hir::def_id::DefId; +use clippy_utils::peel_hir_ty_options; +use rustc_hir::{FnSig, HirId, ItemKind, Node, def_id::DefId, intravisit::FnKind}; +use rustc_hir_analysis::lower_ty; use rustc_lint::LateContext; use rustc_middle::ty::{Ty, TyKind}; use rustc_span::symbol::Symbol; @@ -19,6 +21,23 @@ pub fn match_def_path(cx: &LateContext<'_>, did: DefId, syms: &[&str]) -> bool { .eq(path.iter().copied()) } +pub fn match_def_paths(cx: &LateContext<'_>, did: DefId, syms: &[&[&str]]) -> bool { + let path = cx.get_def_path(did); + syms.iter().any(|syms| { + syms.iter() + .map(|x| Symbol::intern(x)) + .eq(path.iter().copied()) + }) +} + +pub fn is_trait_item(cx: &LateContext<'_>, hir_id: HirId) -> bool { + if let Node::Item(item) = cx.tcx.parent_hir_node(hir_id) { + matches!(item.kind, ItemKind::Trait(..)) + } else { + false + } +} + pub fn is_param_ty(ty: &Ty) -> bool { matches!(ty.kind(), TyKind::Param(_)) } @@ -35,7 +54,7 @@ pub fn is_agent_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { } pub fn is_gc_scope_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { - match ty.kind() { + match ty.peel_refs().kind() { TyKind::Adt(def, _) => { match_def_path(cx, def.did(), &["nova_vm", "engine", "context", "GcScope"]) } @@ -43,8 +62,19 @@ pub fn is_gc_scope_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { } } +pub fn is_no_gc_method(cx: &LateContext<'_>, did: DefId) -> bool { + match_def_paths( + cx, + did, + &[ + &["nova_vm", "engine", "context", "GcScope", "nogc"], + &["nova_vm", "engine", "context", "GcScope", "into_nogc"], + ], + ) +} + pub fn is_no_gc_scope_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { - match ty.kind() { + match ty.peel_refs().kind() { TyKind::Adt(def, _) => match_def_path( cx, def.did(), @@ -53,3 +83,85 @@ pub fn is_no_gc_scope_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { _ => false, } } + +pub fn is_value_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { + match ty.peel_refs().kind() { + TyKind::Adt(def, _) => match_def_path( + cx, + def.did(), + &[ + "nova_vm", + "ecmascript", + "types", + "language", + "value", + "Value", + ], + ), + _ => false, + } +} + +pub fn is_object_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { + match ty.peel_refs().kind() { + TyKind::Adt(def, _) => match_def_path( + cx, + def.did(), + &[ + "nova_vm", + "ecmascript", + "types", + "language", + "object", + "Object", + ], + ), + _ => false, + } +} + +pub fn is_arguments_list_ty(cx: &LateContext<'_>, ty: &Ty) -> bool { + match ty.peel_refs().kind() { + TyKind::Adt(def, _) => match_def_path( + cx, + def.did(), + &[ + "nova_vm", + "ecmascript", + "builtins", + "builtin_function", + "ArgumentsList", + ], + ), + _ => false, + } +} + +pub fn could_be_builtin_method_sig<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'tcx>) -> bool { + sig.decl.inputs.len() == 4 + && is_agent_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[0])) + && is_value_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[1])) + && is_arguments_list_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[2])) + && is_gc_scope_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[3])) +} + +pub fn could_be_builtin_constructor_sig<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'tcx>) -> bool { + sig.decl.inputs.len() == 5 + && is_agent_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[0])) + && is_value_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[1])) + && is_arguments_list_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[2])) + && is_object_ty( + cx, + &lower_ty(cx.tcx, peel_hir_ty_options(cx, &sig.decl.inputs[3])), + ) + && is_gc_scope_ty(cx, &lower_ty(cx.tcx, &sig.decl.inputs[4])) +} + +pub fn could_be_builtin_method_def<'tcx>(cx: &LateContext<'tcx>, kind: FnKind<'tcx>) -> bool { + match kind { + FnKind::Method(_, sig) => { + could_be_builtin_method_sig(cx, sig) || could_be_builtin_constructor_sig(cx, sig) + } + _ => false, + } +} diff --git a/nova_lint/ui/can_use_no_gc_scope.rs b/nova_lint/ui/can_use_no_gc_scope.rs new file mode 100644 index 000000000..157545ba0 --- /dev/null +++ b/nova_lint/ui/can_use_no_gc_scope.rs @@ -0,0 +1,93 @@ +#![allow(dead_code, unused_variables)] + +use nova_vm::{ + ecmascript::{ + builtins::ArgumentsList, + execution::{Agent, JsResult}, + types::{Object, Value}, + }, + engine::context::{GcScope, NoGcScope}, +}; + +fn test_doesnt_need_gc_scope(gc: GcScope<'_, '_>) { + needs_nogc(gc.nogc()); +} + +fn test_doesnt_need_gc_scope_at_all(gc: GcScope<'_, '_>) { + unimplemented!() +} + +fn test_doesnt_need_gc_scope_with_qualified_path(gc: nova_vm::engine::context::GcScope<'_, '_>) { + unimplemented!() +} + +fn test_uses_gc_in_closure<'a, 'b>(work: impl FnOnce(GcScope<'a, 'b>) -> (), gc: GcScope<'a, 'b>) { + work(gc) +} + +fn test_needs_gc_scope(mut gc: GcScope<'_, '_>) { + needs_gc(gc.reborrow()); +} + +fn test_needs_both(mut gc: GcScope<'_, '_>) { + needs_gc(gc.reborrow()); + needs_nogc(gc.into_nogc()); +} + +fn test_uses_gc_method(mut gc: GcScope<'_, '_>) { + gc.reborrow(); +} + +struct BuiltinObject; + +impl BuiltinObject { + fn test_doesnt_need_gc_scope(&self, gc: GcScope<'_, '_>) { + needs_nogc(gc.nogc()); + } + + fn test_skips_builtin_method<'gc>( + _: &mut Agent, + _: Value, + _: ArgumentsList, + _: GcScope<'gc, '_>, + ) -> JsResult<'gc, Value<'gc>> { + unimplemented!() + } + + fn test_skips_builtin_constructor<'gc>( + _: &mut Agent, + _: Value, + _: ArgumentsList, + _: Option, + _: GcScope<'gc, '_>, + ) -> JsResult<'gc, Value<'gc>> { + unimplemented!() + } +} + +trait TakesGC { + fn test_doesnt_actually_need_gc(&mut self, gc: GcScope<'_, '_>); + + fn test_doesnt_actually_need_gc_with_default_impl(&mut self, gc: GcScope<'_, '_>) { + unimplemented!() + } +} + +impl TakesGC for BuiltinObject { + fn test_doesnt_actually_need_gc(&mut self, gc: GcScope<'_, '_>) { + unimplemented!() + } +} + +fn needs_nogc(gc: NoGcScope<'_, '_>) { + unimplemented!() +} + +fn needs_gc(mut gc: GcScope<'_, '_>) { + gc.reborrow(); + unimplemented!() +} + +fn main() { + unimplemented!() +} diff --git a/nova_lint/ui/can_use_no_gc_scope.stderr b/nova_lint/ui/can_use_no_gc_scope.stderr new file mode 100644 index 000000000..3660e1d8b --- /dev/null +++ b/nova_lint/ui/can_use_no_gc_scope.stderr @@ -0,0 +1,28 @@ +warning: you can use `NoGcScope` instead of `GcScope` here + --> $DIR/can_use_no_gc_scope.rs:12:34 + | +LL | fn test_doesnt_need_gc_scope(gc: GcScope<'_, '_>) { + | ^^^^^^^ help: replace with: `NoGcScope` + | + = note: `#[warn(can_use_no_gc_scope)]` on by default + +warning: you can use `NoGcScope` instead of `GcScope` here + --> $DIR/can_use_no_gc_scope.rs:16:41 + | +LL | fn test_doesnt_need_gc_scope_at_all(gc: GcScope<'_, '_>) { + | ^^^^^^^ help: replace with: `NoGcScope` + +warning: you can use `NoGcScope` instead of `GcScope` here + --> $DIR/can_use_no_gc_scope.rs:20:80 + | +LL | fn test_doesnt_need_gc_scope_with_qualified_path(gc: nova_vm::engine::context::GcScope<'_, '_>) { + | ^^^^^^^ help: replace with: `NoGcScope` + +warning: you can use `NoGcScope` instead of `GcScope` here + --> $DIR/can_use_no_gc_scope.rs:44:45 + | +LL | fn test_doesnt_need_gc_scope(&self, gc: GcScope<'_, '_>) { + | ^^^^^^^ help: replace with: `NoGcScope` + +warning: 4 warnings emitted + diff --git a/nova_lint/ui/gc_scope_comes_last.rs b/nova_lint/ui/gc_scope_comes_last.rs index 0fbcfd2e0..9fd26cfba 100644 --- a/nova_lint/ui/gc_scope_comes_last.rs +++ b/nova_lint/ui/gc_scope_comes_last.rs @@ -1,4 +1,10 @@ -#![allow(dead_code, unused_variables, clippy::disallowed_names)] +#![allow( + dead_code, + unused_variables, + clippy::disallowed_names, + unknown_lints, + can_use_no_gc_scope +)] type GcScope<'a, 'b> = nova_vm::engine::context::GcScope<'a, 'b>; type NoGcScope<'a, 'b> = nova_vm::engine::context::NoGcScope<'a, 'b>; diff --git a/nova_lint/ui/gc_scope_comes_last.stderr b/nova_lint/ui/gc_scope_comes_last.stderr index d3160d58b..29fcaed35 100644 --- a/nova_lint/ui/gc_scope_comes_last.stderr +++ b/nova_lint/ui/gc_scope_comes_last.stderr @@ -1,5 +1,5 @@ warning: the gc scope should be the last parameter of any function using it - --> $DIR/gc_scope_comes_last.rs:26:39 + --> $DIR/gc_scope_comes_last.rs:32:39 | LL | fn test_something_else_after_gc_scope(gc_scope: GcScope<'_, '_>, foo: ()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | fn test_something_else_after_gc_scope(gc_scope: GcScope<'_, '_>, foo: ()) { = note: `#[warn(gc_scope_comes_last)]` on by default warning: the gc scope should be the last parameter of any function using it - --> $DIR/gc_scope_comes_last.rs:31:5 + --> $DIR/gc_scope_comes_last.rs:37:5 | LL | gc_scope1: GcScope<'_, '_>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | gc_scope1: GcScope<'_, '_>, = help: consider moving the gc scope to the last parameter warning: the gc scope should be the last parameter of any function using it - --> $DIR/gc_scope_comes_last.rs:39:5 + --> $DIR/gc_scope_comes_last.rs:45:5 | LL | gc_scope1: NoGcScope<'_, '_>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ LL | gc_scope1: NoGcScope<'_, '_>, = help: consider moving the gc scope to the last parameter warning: the gc scope should be the last parameter of any function using it - --> $DIR/gc_scope_comes_last.rs:61:54 + --> $DIR/gc_scope_comes_last.rs:67:54 | LL | fn test_self_and_something_after_gc_scope(&self, gc_scope: GcScope<'_, '_>, foo: ()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL | fn test_self_and_something_after_gc_scope(&self, gc_scope: GcScope<'_, = help: consider moving the gc scope to the last parameter warning: the gc scope should be the last parameter of any function using it - --> $DIR/gc_scope_comes_last.rs:65:38 + --> $DIR/gc_scope_comes_last.rs:71:38 | LL | fn test_something_after_gc_scope(gc_scope: GcScope<'_, '_>, foo: ()) { | ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/nova_lint/ui/gc_scope_is_only_passed_by_value.rs b/nova_lint/ui/gc_scope_is_only_passed_by_value.rs index e0067affb..f9dd4c4e3 100644 --- a/nova_lint/ui/gc_scope_is_only_passed_by_value.rs +++ b/nova_lint/ui/gc_scope_is_only_passed_by_value.rs @@ -1,4 +1,10 @@ -#![allow(dead_code, unused_variables, clippy::disallowed_names)] +#![allow( + dead_code, + unused_variables, + clippy::disallowed_names, + unknown_lints, + can_use_no_gc_scope +)] type GcScope<'a, 'b> = nova_vm::engine::context::GcScope<'a, 'b>; type NoGcScope<'a, 'b> = nova_vm::engine::context::NoGcScope<'a, 'b>; diff --git a/nova_lint/ui/gc_scope_is_only_passed_by_value.stderr b/nova_lint/ui/gc_scope_is_only_passed_by_value.stderr index 2879b4c44..ef57b37b4 100644 --- a/nova_lint/ui/gc_scope_is_only_passed_by_value.stderr +++ b/nova_lint/ui/gc_scope_is_only_passed_by_value.stderr @@ -1,5 +1,5 @@ error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:22:52 + --> $DIR/gc_scope_is_only_passed_by_value.rs:28:52 | LL | fn test_borrowed_qualified_gc_scope_only(gc_scope: &nova_vm::engine::context::GcScope<'_, '_>) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | fn test_borrowed_qualified_gc_scope_only(gc_scope: &nova_vm::engine::contex = note: `#[deny(gc_scope_is_only_passed_by_value)]` on by default error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:26:42 + --> $DIR/gc_scope_is_only_passed_by_value.rs:32:42 | LL | fn test_borrowed_gc_scope_only(gc_scope: &GcScope<'_, '_>) { | ^^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | fn test_borrowed_gc_scope_only(gc_scope: &GcScope<'_, '_>) { = help: remove the reference error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:31:15 + --> $DIR/gc_scope_is_only_passed_by_value.rs:37:15 | LL | gc_scope: &nova_vm::engine::context::NoGcScope<'_, '_>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ LL | gc_scope: &nova_vm::engine::context::NoGcScope<'_, '_>, = help: remove the reference error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:36:45 + --> $DIR/gc_scope_is_only_passed_by_value.rs:42:45 | LL | fn test_borrowed_no_gc_scope_only(gc_scope: &NoGcScope<'_, '_>) { | ^^^^^^^^^^^^^^^^^^ @@ -32,7 +32,7 @@ LL | fn test_borrowed_no_gc_scope_only(gc_scope: &NoGcScope<'_, '_>) { = help: remove the reference error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:41:15 + --> $DIR/gc_scope_is_only_passed_by_value.rs:47:15 | LL | gc_scope: &mut nova_vm::engine::context::GcScope<'_, '_>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | gc_scope: &mut nova_vm::engine::context::GcScope<'_, '_>, = help: remove the reference error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:46:46 + --> $DIR/gc_scope_is_only_passed_by_value.rs:52:46 | LL | fn test_mut_borrowed_gc_scope_only(gc_scope: &mut GcScope<'_, '_>) { | ^^^^^^^^^^^^^^^^^^^^ @@ -48,7 +48,7 @@ LL | fn test_mut_borrowed_gc_scope_only(gc_scope: &mut GcScope<'_, '_>) { = help: remove the reference error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:51:15 + --> $DIR/gc_scope_is_only_passed_by_value.rs:57:15 | LL | gc_scope: &mut nova_vm::engine::context::NoGcScope<'_, '_>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -56,7 +56,7 @@ LL | gc_scope: &mut nova_vm::engine::context::NoGcScope<'_, '_>, = help: remove the reference error: gc scope should only be passed by value - --> $DIR/gc_scope_is_only_passed_by_value.rs:56:49 + --> $DIR/gc_scope_is_only_passed_by_value.rs:62:49 | LL | fn test_mut_borrowed_no_gc_scope_only(gc_scope: &mut NoGcScope<'_, '_>) { | ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/nova_vm/src/ecmascript/builtins/structured_data/atomics_object.rs b/nova_vm/src/ecmascript/builtins/structured_data/atomics_object.rs index 8d345322e..1da24c156 100644 --- a/nova_vm/src/ecmascript/builtins/structured_data/atomics_object.rs +++ b/nova_vm/src/ecmascript/builtins/structured_data/atomics_object.rs @@ -1674,6 +1674,11 @@ impl WaitAsyncJob { self.0._has_timeout } + // NOTE: The reason for using `GcScope` here even though we could've gotten + // away with `NoGcScope` is that this is essentially a trait impl method, + // but currently without the trait. The job trait will be added eventually + // and we can get rid of this lint exception. + #[allow(unknown_lints, can_use_no_gc_scope)] pub(crate) fn run<'gc>(self, agent: &mut Agent, gc: GcScope) -> JsResult<'gc, ()> { let gc = gc.into_nogc(); let promise = self.0.promise_to_resolve.take(agent).bind(gc); diff --git a/nova_vm/src/engine/bytecode/vm.rs b/nova_vm/src/engine/bytecode/vm.rs index 681793e3e..5580e4ede 100644 --- a/nova_vm/src/engine/bytecode/vm.rs +++ b/nova_vm/src/engine/bytecode/vm.rs @@ -656,10 +656,10 @@ impl Vm { | Instruction::ApplyBitwiseAndBinaryOperator => { execute_apply_binary_operator(agent, vm, instr.kind, gc)? } - Instruction::ArrayCreate => execute_array_create(agent, vm, instr, gc)?, + Instruction::ArrayCreate => execute_array_create(agent, vm, instr, gc.into_nogc())?, Instruction::ArrayPush => execute_array_push(agent, vm, gc)?, Instruction::ArrayElision => execute_array_elision(agent, vm, gc)?, - Instruction::BitwiseNot => execute_bitwise_not(agent, vm, gc)?, + Instruction::BitwiseNot => execute_bitwise_not(agent, vm, gc.into_nogc())?, Instruction::CreateUnmappedArgumentsObject => { execute_create_unmapped_arguments_object(agent, vm, gc.into_nogc())? } @@ -749,13 +749,15 @@ impl Vm { Instruction::ResolveBindingWithCache => { execute_resolve_binding_with_cache(agent, vm, executable, instr, gc)? } - Instruction::ResolveThisBinding => execute_resolve_this_binding(agent, vm, gc)?, + Instruction::ResolveThisBinding => { + execute_resolve_this_binding(agent, vm, gc.into_nogc())? + } Instruction::StoreCopy => vm.execute_store_copy(), Instruction::StringConcat => execute_string_concat(agent, vm, instr, gc)?, Instruction::Throw => vm.execute_throw(gc.into_nogc())?, Instruction::ThrowError => execute_throw_error(agent, vm, instr, gc.into_nogc())?, Instruction::ToNumber => execute_to_number(agent, vm, gc)?, - Instruction::ToObject => execute_to_object(agent, vm, gc)?, + Instruction::ToObject => execute_to_object(agent, vm, gc.into_nogc())?, Instruction::Typeof => execute_typeof(agent, vm, gc)?, Instruction::UnaryMinus => execute_unary_minus(agent, vm, gc.into_nogc()), Instruction::InitializeVariableEnvironment => { diff --git a/nova_vm/src/engine/bytecode/vm/execute_instructions.rs b/nova_vm/src/engine/bytecode/vm/execute_instructions.rs index 66d5683ca..07e4bd857 100644 --- a/nova_vm/src/engine/bytecode/vm/execute_instructions.rs +++ b/nova_vm/src/engine/bytecode/vm/execute_instructions.rs @@ -82,10 +82,9 @@ pub(super) fn execute_array_create<'gc>( agent: &mut Agent, vm: &mut Vm, instr: Instr, - gc: GcScope<'gc, '_>, + gc: NoGcScope<'gc, '_>, ) -> JsResult<'gc, ()> { - let result = - array_create(agent, 0, instr.get_first_index(), None, gc.into_nogc())?.into_value(); + let result = array_create(agent, 0, instr.get_first_index(), None, gc)?.into_value(); vm.result = Some(result.unbind()); Ok(()) } @@ -145,13 +144,13 @@ pub(super) fn execute_array_elision<'gc>( pub(super) fn execute_bitwise_not<'gc>( agent: &mut Agent, vm: &mut Vm, - gc: GcScope<'gc, '_>, + gc: NoGcScope<'gc, '_>, ) -> JsResult<'gc, ()> { // 2. Let oldValue be ? ToNumeric(? GetValue(expr)). // Note: This step is a separate instruction. let old_value = Numeric::try_from(vm.result.take().unwrap()) .unwrap() - .bind(gc.nogc()); + .bind(gc); // 3. If oldValue is a Number, then if let Ok(old_value) = Number::try_from(old_value) { @@ -250,9 +249,9 @@ fn execute_resolve_binding_with_cache_cold<'gc>( pub(super) fn execute_resolve_this_binding<'gc>( agent: &mut Agent, vm: &mut Vm, - gc: GcScope<'gc, '_>, + gc: NoGcScope<'gc, '_>, ) -> JsResult<'gc, ()> { - let this = resolve_this_binding(agent, gc.into_nogc())?.unbind(); + let this = resolve_this_binding(agent, gc)?.unbind(); vm.result = Some(this); Ok(()) } @@ -346,10 +345,10 @@ fn execute_to_numeric_cold<'gc>( pub(super) fn execute_to_object<'gc>( agent: &mut Agent, vm: &mut Vm, - gc: GcScope<'gc, '_>, + gc: NoGcScope<'gc, '_>, ) -> JsResult<'gc, ()> { vm.result = Some( - to_object(agent, vm.result.unwrap(), gc.into_nogc())? + to_object(agent, vm.result.unwrap(), gc)? .into_value() .unbind(), ); diff --git a/nova_vm/src/heap/heap_gc.rs b/nova_vm/src/heap/heap_gc.rs index 2039206b9..340659b66 100644 --- a/nova_vm/src/heap/heap_gc.rs +++ b/nova_vm/src/heap/heap_gc.rs @@ -1221,6 +1221,9 @@ pub fn heap_gc(agent: &mut Agent, root_realms: &mut [Option>], gc ndt::gc_done!(|| ()); } +// NOTE: This is the one true use of the `GcScope` which is why we allow a lint +// exception here. For future reference see [this comment](https://github.com/trynova/nova/pull/913#discussion_r2616482397). +#[allow(unknown_lints, can_use_no_gc_scope)] fn sweep( agent: &mut Agent, bits: &HeapBits,