From d3de7013528d36ef6ec5106d107ea8bca3e71fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Sj=C3=B6green?= Date: Mon, 1 Dec 2025 21:08:31 +0100 Subject: [PATCH 1/6] feat(linter): Warn when you could've used NoGcScope --- nova_lint/Cargo.toml | 46 +++++---- nova_lint/src/can_use_no_gc_scope.rs | 123 ++++++++++++++++++++++++ nova_lint/src/lib.rs | 3 + nova_lint/src/utils.rs | 4 +- nova_lint/ui/can_use_no_gc_scope.rs | 27 ++++++ nova_lint/ui/can_use_no_gc_scope.stderr | 0 6 files changed, 180 insertions(+), 23 deletions(-) create mode 100644 nova_lint/src/can_use_no_gc_scope.rs create mode 100644 nova_lint/ui/can_use_no_gc_scope.rs create mode 100644 nova_lint/ui/can_use_no_gc_scope.stderr diff --git a/nova_lint/Cargo.toml b/nova_lint/Cargo.toml index 17649db46..b17c4ecd7 100644 --- a/nova_lint/Cargo.toml +++ b/nova_lint/Cargo.toml @@ -12,29 +12,33 @@ publish = false [lib] crate-type = ["cdylib"] -[[example]] -name = "agent_comes_first" -path = "ui/agent_comes_first.rs" - -[[example]] -name = "gc_scope_comes_last" -path = "ui/gc_scope_comes_last.rs" - -[[example]] -name = "gc_scope_is_only_passed_by_value" -path = "ui/gc_scope_is_only_passed_by_value.rs" - -[[example]] -name = "no_it_performs_the_following" -path = "ui/no_it_performs_the_following.rs" - -[[example]] -name = "no_multipage_spec" -path = "ui/no_multipage_spec.rs" +# [[example]] +# name = "agent_comes_first" +# path = "ui/agent_comes_first.rs" [[example]] -name = "spec_header_level" -path = "ui/spec_header_level.rs" +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" +# +# [[example]] +# name = "gc_scope_is_only_passed_by_value" +# path = "ui/gc_scope_is_only_passed_by_value.rs" +# +# [[example]] +# name = "no_it_performs_the_following" +# path = "ui/no_it_performs_the_following.rs" +# +# [[example]] +# name = "no_multipage_spec" +# path = "ui/no_multipage_spec.rs" +# +# [[example]] +# name = "spec_header_level" +# path = "ui/spec_header_level.rs" [dependencies] clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "c936595d17413c1f08e162e117e504fb4ed126e4" } 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..d31b19edd --- /dev/null +++ b/nova_lint/src/can_use_no_gc_scope.rs @@ -0,0 +1,123 @@ +use clippy_utils::{diagnostics::span_lint_and_sugg, fn_def_id}; +use rustc_errors::Applicability; +use rustc_hir::{Body, ExprKind, FnDecl, Param, def_id::LocalDefId, intravisit::FnKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_span::Span; + +use crate::is_gc_scope_ty; + +dylint_linting::impl_late_lint! { + /// ### What it does + /// + /// Checks that a function which only needs `NoGcScope` uses it instead of + /// `GcScope`. + /// + /// ### Why is this bad? + /// + /// TODO + /// + /// ### 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", + CanUseNoGcScope::new() +} + +struct CanUseNoGcScope { + in_target_fn: bool, + requires_gc: bool, + param_span: Option, +} + +impl CanUseNoGcScope { + fn new() -> Self { + Self { + in_target_fn: false, + requires_gc: false, + param_span: None, + } + } + + fn enter(&mut self, param: &Param<'_>) { + self.in_target_fn = true; + self.requires_gc = false; + self.param_span = Some(param.ty_span); + } + + fn exit(&mut self, cx: &LateContext<'_>) { + if let Some(span) = self.param_span + && self.in_target_fn + && !self.requires_gc + { + span_lint_and_sugg( + cx, + CAN_USE_NO_GC_SCOPE, + span, + "you can use `NoGcScope` instead of `GcScope` here", + "use `NoGcScope`", + "NoGcScope".to_owned(), + Applicability::MachineApplicable, + ); + } + + self.in_target_fn = false; + self.requires_gc = false; + self.param_span = None; + } +} + +impl<'tcx> LateLintPass<'tcx> for CanUseNoGcScope { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + _: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + span: Span, + _: LocalDefId, + ) { + self.exit(cx); + + if span.from_expansion() { + return; + } + + let Some(gc_scope) = body.params.iter().find(|param| { + let ty = cx.typeck_results().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; + }; + + self.enter(gc_scope); + } + + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { + if !self.in_target_fn || !matches!(expr.kind, ExprKind::MethodCall(..) | ExprKind::Call(..)) + { + return; + } + + let Some(def_id) = fn_def_id(cx, expr) else { + return; + }; + let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); + + self.requires_gc = sig.inputs().iter().any(|input| { + let ty = input.skip_binder(); + is_gc_scope_ty(cx, &ty) + }); + } +} diff --git a/nova_lint/src/lib.rs b/nova_lint/src/lib.rs index 3976e9879..127bc3c83 100644 --- a/nova_lint/src/lib.rs +++ b/nova_lint/src/lib.rs @@ -10,6 +10,7 @@ extern crate rustc_data_structures; extern crate rustc_errors; extern crate rustc_hir; extern crate rustc_hir_pretty; +extern crate rustc_hir_typeck; extern crate rustc_index; extern crate rustc_infer; extern crate rustc_lexer; @@ -23,6 +24,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 +37,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..66cf70e47 100644 --- a/nova_lint/src/utils.rs +++ b/nova_lint/src/utils.rs @@ -1,7 +1,7 @@ -use rustc_hir::def_id::DefId; +use rustc_hir::{Expr, ExprKind, def_id::DefId}; use rustc_lint::LateContext; use rustc_middle::ty::{Ty, TyKind}; -use rustc_span::symbol::Symbol; +use rustc_span::{Span, symbol::Symbol}; // Copyright (c) 2014-2025 The Rust Project Developers // 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..864806458 --- /dev/null +++ b/nova_lint/ui/can_use_no_gc_scope.rs @@ -0,0 +1,27 @@ +type GcScope<'a, 'b> = nova_vm::engine::context::GcScope<'a, 'b>; +type NoGcScope<'a, 'b> = nova_vm::engine::context::NoGcScope<'a, 'b>; + +fn test_doesnt_need_gc_scope(gc: GcScope<'_, '_>) { + needs_nogc(gc.nogc()); +} + +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 needs_nogc(gc: NoGcScope<'_, '_>) { + unimplemented!() +} + +fn needs_gc(gc: GcScope<'_, '_>) { + 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..e69de29bb From e711f0e0e1de250f533aa6f1a9c81d9be1e439cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Sj=C3=B6green?= Date: Mon, 8 Dec 2025 17:53:11 +0100 Subject: [PATCH 2/6] feat(linter): Finish up lint logic --- nova_lint/src/agent_comes_first.rs | 2 +- nova_lint/src/can_use_no_gc_scope.rs | 164 ++++++++++-------- nova_lint/src/gc_scope_comes_last.rs | 2 +- .../src/gc_scope_is_only_passed_by_value.rs | 2 +- nova_lint/src/lib.rs | 1 + nova_lint/src/utils.rs | 120 ++++++++++++- nova_lint/ui/can_use_no_gc_scope.rs | 70 +++++++- nova_lint/ui/can_use_no_gc_scope.stderr | 28 +++ 8 files changed, 310 insertions(+), 79 deletions(-) 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 index d31b19edd..26d55eb59 100644 --- a/nova_lint/src/can_use_no_gc_scope.rs +++ b/nova_lint/src/can_use_no_gc_scope.rs @@ -1,12 +1,18 @@ -use clippy_utils::{diagnostics::span_lint_and_sugg, fn_def_id}; +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, ExprKind, FnDecl, Param, def_id::LocalDefId, intravisit::FnKind}; +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::Span; +use rustc_span::{BytePos, Span}; -use crate::is_gc_scope_ty; +use crate::{could_be_builtin_method_def, is_gc_scope_ty, is_no_gc_method, is_trait_item}; -dylint_linting::impl_late_lint! { +dylint_linting::declare_late_lint! { /// ### What it does /// /// Checks that a function which only needs `NoGcScope` uses it instead of @@ -29,71 +35,54 @@ dylint_linting::impl_late_lint! { /// ``` pub CAN_USE_NO_GC_SCOPE, Warn, - "you should use `NoGcScope` instead of `GcScope` if you don't need the latter", - CanUseNoGcScope::new() -} - -struct CanUseNoGcScope { - in_target_fn: bool, - requires_gc: bool, - param_span: Option, -} - -impl CanUseNoGcScope { - fn new() -> Self { - Self { - in_target_fn: false, - requires_gc: false, - param_span: None, - } - } - - fn enter(&mut self, param: &Param<'_>) { - self.in_target_fn = true; - self.requires_gc = false; - self.param_span = Some(param.ty_span); - } - - fn exit(&mut self, cx: &LateContext<'_>) { - if let Some(span) = self.param_span - && self.in_target_fn - && !self.requires_gc - { - span_lint_and_sugg( - cx, - CAN_USE_NO_GC_SCOPE, - span, - "you can use `NoGcScope` instead of `GcScope` here", - "use `NoGcScope`", - "NoGcScope".to_owned(), - Applicability::MachineApplicable, - ); - } - - self.in_target_fn = false; - self.requires_gc = false; - self.param_span = None; - } + "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>, - _: FnKind<'tcx>, + kind: FnKind<'tcx>, _: &'tcx FnDecl<'tcx>, body: &'tcx Body<'tcx>, span: Span, _: LocalDefId, ) { - self.exit(cx); - 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 = cx.typeck_results().pat_ty(param.pat); + 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 @@ -101,23 +90,60 @@ impl<'tcx> LateLintPass<'tcx> for CanUseNoGcScope { return; }; - self.enter(gc_scope); - } + 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(()); + } - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) { - if !self.in_target_fn || !matches!(expr.kind, ExprKind::MethodCall(..) | ExprKind::Call(..)) + ControlFlow::Continue(()) + }) + .is_none() { - return; + // 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, + ); } - - let Some(def_id) = fn_def_id(cx, expr) else { - return; - }; - let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); - - self.requires_gc = sig.inputs().iter().any(|input| { - let ty = input.skip_binder(); - is_gc_scope_ty(cx, &ty) - }); } } 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 127bc3c83..6d61853fd 100644 --- a/nova_lint/src/lib.rs +++ b/nova_lint/src/lib.rs @@ -9,6 +9,7 @@ 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; diff --git a/nova_lint/src/utils.rs b/nova_lint/src/utils.rs index 66cf70e47..642d1dfee 100644 --- a/nova_lint/src/utils.rs +++ b/nova_lint/src/utils.rs @@ -1,7 +1,9 @@ -use rustc_hir::{Expr, ExprKind, 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::{Span, symbol::Symbol}; +use rustc_span::symbol::Symbol; // Copyright (c) 2014-2025 The Rust Project Developers // @@ -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 index 864806458..4934012fd 100644 --- a/nova_lint/ui/can_use_no_gc_scope.rs +++ b/nova_lint/ui/can_use_no_gc_scope.rs @@ -1,10 +1,28 @@ -type GcScope<'a, 'b> = nova_vm::engine::context::GcScope<'a, 'b>; -type NoGcScope<'a, 'b> = nova_vm::engine::context::NoGcScope<'a, 'b>; +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()); } @@ -14,11 +32,57 @@ fn test_needs_both(mut gc: GcScope<'_, '_>) { 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(gc: GcScope<'_, '_>) { +fn needs_gc(mut gc: GcScope<'_, '_>) { + gc.reborrow(); unimplemented!() } diff --git a/nova_lint/ui/can_use_no_gc_scope.stderr b/nova_lint/ui/can_use_no_gc_scope.stderr index e69de29bb..2830d4ef8 100644 --- a/nova_lint/ui/can_use_no_gc_scope.stderr +++ 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:10: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:14: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:18: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:42:45 + | +LL | fn test_doesnt_need_gc_scope(&self, gc: GcScope<'_, '_>) { + | ^^^^^^^ help: replace with: `NoGcScope` + +warning: 4 warnings emitted + From 0898469076134c4cd8dcb262ac013649da3f1661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Sj=C3=B6green?= Date: Mon, 8 Dec 2025 17:56:09 +0100 Subject: [PATCH 3/6] fix: `nova_vm` unnecessary uses of `GcScope` --- .../builtins/structured_data/atomics_object.rs | 1 + nova_vm/src/engine/bytecode/vm.rs | 10 ++++++---- .../engine/bytecode/vm/execute_instructions.rs | 17 ++++++++--------- nova_vm/src/heap/heap_gc.rs | 1 + 4 files changed, 16 insertions(+), 13 deletions(-) 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..82ba89e5a 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,7 @@ impl WaitAsyncJob { self.0._has_timeout } + #[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..c85ac2f02 100644 --- a/nova_vm/src/heap/heap_gc.rs +++ b/nova_vm/src/heap/heap_gc.rs @@ -1221,6 +1221,7 @@ pub fn heap_gc(agent: &mut Agent, root_realms: &mut [Option>], gc ndt::gc_done!(|| ()); } +#[allow(unknown_lints, can_use_no_gc_scope)] fn sweep( agent: &mut Agent, bits: &HeapBits, From ad8120f4e2cd8d7d0a5ffdd4e32dc6263e782320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Sj=C3=B6green?= Date: Mon, 8 Dec 2025 18:02:52 +0100 Subject: [PATCH 4/6] fix: Re-enable the rest of the lints --- nova_lint/Cargo.toml | 44 +++++++++---------- nova_lint/src/can_use_no_gc_scope.rs | 7 +-- nova_lint/ui/can_use_no_gc_scope.rs | 2 + nova_lint/ui/can_use_no_gc_scope.stderr | 8 ++-- nova_lint/ui/gc_scope_comes_last.rs | 8 +++- nova_lint/ui/gc_scope_comes_last.stderr | 10 ++--- .../ui/gc_scope_is_only_passed_by_value.rs | 8 +++- .../gc_scope_is_only_passed_by_value.stderr | 16 +++---- 8 files changed, 59 insertions(+), 44 deletions(-) diff --git a/nova_lint/Cargo.toml b/nova_lint/Cargo.toml index b17c4ecd7..07be2709a 100644 --- a/nova_lint/Cargo.toml +++ b/nova_lint/Cargo.toml @@ -12,33 +12,33 @@ publish = false [lib] crate-type = ["cdylib"] -# [[example]] -# name = "agent_comes_first" -# path = "ui/agent_comes_first.rs" +[[example]] +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" -# -# [[example]] -# name = "gc_scope_is_only_passed_by_value" -# path = "ui/gc_scope_is_only_passed_by_value.rs" -# -# [[example]] -# name = "no_it_performs_the_following" -# path = "ui/no_it_performs_the_following.rs" -# -# [[example]] -# name = "no_multipage_spec" -# path = "ui/no_multipage_spec.rs" -# -# [[example]] -# name = "spec_header_level" -# path = "ui/spec_header_level.rs" +[[example]] +name = "gc_scope_comes_last" +path = "ui/gc_scope_comes_last.rs" + +[[example]] +name = "gc_scope_is_only_passed_by_value" +path = "ui/gc_scope_is_only_passed_by_value.rs" + +[[example]] +name = "no_it_performs_the_following" +path = "ui/no_it_performs_the_following.rs" + +[[example]] +name = "no_multipage_spec" +path = "ui/no_multipage_spec.rs" + +[[example]] +name = "spec_header_level" +path = "ui/spec_header_level.rs" [dependencies] clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "c936595d17413c1f08e162e117e504fb4ed126e4" } diff --git a/nova_lint/src/can_use_no_gc_scope.rs b/nova_lint/src/can_use_no_gc_scope.rs index 26d55eb59..b220cd2d3 100644 --- a/nova_lint/src/can_use_no_gc_scope.rs +++ b/nova_lint/src/can_use_no_gc_scope.rs @@ -20,7 +20,8 @@ dylint_linting::declare_late_lint! { /// /// ### Why is this bad? /// - /// TODO + /// You usually should use `NoGcScope` instead of `GcScope` if you don't + /// need the latter. /// /// ### Example /// @@ -58,11 +59,11 @@ impl<'tcx> LateLintPass<'tcx> for CanUseNoGcScope { } // 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 { + }; + if res { return; } diff --git a/nova_lint/ui/can_use_no_gc_scope.rs b/nova_lint/ui/can_use_no_gc_scope.rs index 4934012fd..157545ba0 100644 --- a/nova_lint/ui/can_use_no_gc_scope.rs +++ b/nova_lint/ui/can_use_no_gc_scope.rs @@ -1,3 +1,5 @@ +#![allow(dead_code, unused_variables)] + use nova_vm::{ ecmascript::{ builtins::ArgumentsList, diff --git a/nova_lint/ui/can_use_no_gc_scope.stderr b/nova_lint/ui/can_use_no_gc_scope.stderr index 2830d4ef8..3660e1d8b 100644 --- a/nova_lint/ui/can_use_no_gc_scope.stderr +++ b/nova_lint/ui/can_use_no_gc_scope.stderr @@ -1,5 +1,5 @@ warning: you can use `NoGcScope` instead of `GcScope` here - --> $DIR/can_use_no_gc_scope.rs:10:34 + --> $DIR/can_use_no_gc_scope.rs:12:34 | LL | fn test_doesnt_need_gc_scope(gc: GcScope<'_, '_>) { | ^^^^^^^ help: replace with: `NoGcScope` @@ -7,19 +7,19 @@ LL | fn test_doesnt_need_gc_scope(gc: GcScope<'_, '_>) { = 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:14:41 + --> $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:18:80 + --> $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:42:45 + --> $DIR/can_use_no_gc_scope.rs:44:45 | LL | fn test_doesnt_need_gc_scope(&self, gc: GcScope<'_, '_>) { | ^^^^^^^ help: replace with: `NoGcScope` 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<'_, '_>) { | ^^^^^^^^^^^^^^^^^^^^^^ From 30ac4b3b52c1fb3ac02e9bdd0accb350cb135cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Sj=C3=B6green?= Date: Mon, 8 Dec 2025 18:17:10 +0100 Subject: [PATCH 5/6] fix: Opt-out of the lint in nova_cli --- nova_cli/src/main.rs | 2 ++ 1 file changed, 2 insertions(+) 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; From 5cbb99ad8e2f771f755eb31e527c9cec28ddda02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20Sj=C3=B6green?= Date: Sun, 14 Dec 2025 14:27:09 +0100 Subject: [PATCH 6/6] fix: Aapos suggestions --- nova_lint/src/can_use_no_gc_scope.rs | 3 ++- .../src/ecmascript/builtins/structured_data/atomics_object.rs | 4 ++++ nova_vm/src/heap/heap_gc.rs | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/nova_lint/src/can_use_no_gc_scope.rs b/nova_lint/src/can_use_no_gc_scope.rs index b220cd2d3..a1ab881c1 100644 --- a/nova_lint/src/can_use_no_gc_scope.rs +++ b/nova_lint/src/can_use_no_gc_scope.rs @@ -21,7 +21,8 @@ dylint_linting::declare_late_lint! { /// ### Why is this bad? /// /// You usually should use `NoGcScope` instead of `GcScope` if you don't - /// need the latter. + /// 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 /// 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 82ba89e5a..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,10 @@ 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(); diff --git a/nova_vm/src/heap/heap_gc.rs b/nova_vm/src/heap/heap_gc.rs index c85ac2f02..340659b66 100644 --- a/nova_vm/src/heap/heap_gc.rs +++ b/nova_vm/src/heap/heap_gc.rs @@ -1221,6 +1221,8 @@ 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,