Skip to content

Commit f49b7b3

Browse files
aapoalasaapoalas
authored
fix(gc): Fix more GC usage (#600)
1 parent 260683f commit f49b7b3

File tree

25 files changed

+199
-157
lines changed

25 files changed

+199
-157
lines changed

‎nova_vm/src/ecmascript/builtins/bound_function.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,12 @@ impl<'a> InternalMethods<'a> for BoundFunction<'a> {
325325
// arguments list without changes to the bound function.
326326
call_function(agent, target, bound_this, Some(arguments_list.unbind()), gc)
327327
} else {
328-
// Note: We currently cannot optimise against an empty arguments
329-
// list, as we must create a Vec from the bound_args ElementsVector
330-
// in any case to use it as arguments. A slice pointing to it would
331-
// be unsound as calling to JS may invalidate the slice pointer.
328+
// Note: We cannot optimise against an empty arguments list, as we
329+
// must create a Vec from the bound_args ElementsVector in any case
330+
// to use it as arguments. A slice pointing to it would be unsound
331+
// as calling to JS may invalidate the slice pointer. Arguments
332+
// must also be given as exclusive slice, which we couldn't provide
333+
// if we were basing it on the ElementsVector's data in the heap.
332334
let mut args: Vec<Value<'static>> =
333335
Vec::with_capacity(bound_args.len() as usize + arguments_list.len());
334336
agent[bound_args]

‎nova_vm/src/ecmascript/builtins/builtin_function.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ impl<'slice, 'value> ArgumentsList<'slice, 'value> {
151151

152152
pub(crate) fn slice_from(self, start: usize) -> ArgumentsList<'slice, 'value> {
153153
ArgumentsList {
154-
slice: &mut self.slice[start..],
154+
slice: self
155+
.slice
156+
.split_at_mut_checked(start)
157+
.map_or(&mut [], |(_, tail)| tail),
155158
value: PhantomData,
156159
}
157160
}

‎nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_abstract_operations/promise_resolving_functions.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,18 @@ impl<'a> InternalMethods<'a> for BuiltinPromiseResolvingFunction<'a> {
253253
self,
254254
agent: &mut Agent,
255255
_this_value: Value,
256-
args: ArgumentsList,
256+
arguments_list: ArgumentsList,
257257
gc: GcScope<'gc, '_>,
258258
) -> JsResult<Value<'gc>> {
259-
let arg = args.get(0).bind(gc.nogc());
259+
let arguments_list = arguments_list.get(0).bind(gc.nogc());
260260
let promise_capability = agent[self].promise_capability;
261261
match agent[self].resolve_type {
262262
PromiseResolvingFunctionType::Resolve => {
263-
promise_capability.resolve(agent, arg.unbind(), gc)
263+
promise_capability.resolve(agent, arguments_list.unbind(), gc)
264+
}
265+
PromiseResolvingFunctionType::Reject => {
266+
promise_capability.reject(agent, arguments_list)
264267
}
265-
PromiseResolvingFunctionType::Reject => promise_capability.reject(agent, arg),
266268
};
267269
Ok(Value::Undefined)
268270
}

‎nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_constructor.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,17 @@ impl PromiseConstructor {
274274
Ok(Promise::resolve(agent, arguments.get(0), gc).into_value())
275275
}
276276

277-
/// Defined in the [`Promise.try` proposal](https://tc39.es/proposal-promise-try)
277+
/// ### [1 Promise.try ( callbackfn, ...args )](https://tc39.es/proposal-promise-try)
278+
///
279+
/// `Promise.try` proposal.
278280
fn r#try<'gc>(
279281
agent: &mut Agent,
280282
this_value: Value,
281283
arguments: ArgumentsList,
282284
mut gc: GcScope<'gc, '_>,
283285
) -> JsResult<Value<'gc>> {
286+
let callback_fn = arguments.get(0).bind(gc.nogc());
287+
let args = arguments.slice_from(1).bind(gc.nogc());
284288
// 1. Let C be the this value.
285289
// 2. If C is not an Object, throw a TypeError exception.
286290
if is_constructor(agent, this_value).is_none() {
@@ -300,9 +304,9 @@ impl PromiseConstructor {
300304
// 4. Let status be Completion(Call(callbackfn, undefined, args)).
301305
let status = call(
302306
agent,
303-
arguments.get(0),
307+
callback_fn.unbind(),
304308
Value::Undefined,
305-
Some(arguments.slice_from(1)),
309+
Some(args.unbind()),
306310
gc.reborrow(),
307311
);
308312
let promise = match status {

‎nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_prototype.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,16 @@ impl PromisePrototype {
8585
todo!()
8686
}
8787

88+
/// ### [27.2.5.4 Promise.prototype.then ( onFulfilled, onRejected )](https://tc39.es/ecma262/#sec-promise.prototype.then)
8889
fn then<'gc>(
8990
agent: &mut Agent,
9091
this_value: Value,
9192
args: ArgumentsList,
9293
gc: GcScope<'gc, '_>,
9394
) -> JsResult<Value<'gc>> {
9495
let gc = gc.into_nogc();
96+
let on_fulfilled = args.get(0).bind(gc);
97+
let on_rejected = args.get(1).bind(gc);
9598
// 1. Let promise be the this value.
9699
// 2. If IsPromise(promise) is false, throw a TypeError exception.
97100
let Value::Promise(promise) = this_value else {
@@ -111,8 +114,8 @@ impl PromisePrototype {
111114
perform_promise_then(
112115
agent,
113116
promise,
114-
args.get(0),
115-
args.get(1),
117+
on_fulfilled,
118+
on_rejected,
116119
Some(result_capability),
117120
);
118121
Ok(result_capability.promise().into_value())

‎nova_vm/src/ecmascript/builtins/ecmascript_function.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -547,13 +547,13 @@ impl<'a> InternalMethods<'a> for ECMAScriptFunction<'a> {
547547
fn internal_construct<'gc>(
548548
self,
549549
agent: &mut Agent,
550-
arguments_list: ArgumentsList,
550+
arguments: ArgumentsList,
551551
new_target: Function,
552552
mut gc: GcScope<'gc, '_>,
553553
) -> JsResult<Object<'gc>> {
554554
let mut self_fn = self.bind(gc.nogc());
555555
let mut new_target = new_target.bind(gc.nogc());
556-
let mut arguments_list = arguments_list.bind(gc.nogc());
556+
let mut arguments_list = arguments.bind(gc.nogc());
557557
// 2. Let kind be F.[[ConstructorKind]].
558558
let is_base = !agent[self_fn]
559559
.ecmascript_function

‎nova_vm/src/ecmascript/builtins/fundamental_objects/boolean_objects/boolean_constructor.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl BooleanConstructor {
4646
new_target: Option<Object>,
4747
gc: GcScope<'gc, '_>,
4848
) -> JsResult<Value<'gc>> {
49-
let value = arguments.get(0);
49+
let value = arguments.get(0).bind(gc.nogc());
5050
let b = to_boolean(agent, value);
5151
let Some(new_target) = new_target else {
5252
return Ok(b.into());

‎nova_vm/src/ecmascript/builtins/fundamental_objects/error_objects/aggregate_error_constructors.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ impl AggregateErrorConstructor {
5050
new_target: Option<Object>,
5151
mut gc: GcScope<'gc, '_>,
5252
) -> JsResult<Value<'gc>> {
53-
let errors = arguments.get(0);
54-
let message = arguments.get(1);
53+
let errors = arguments.get(0).scope(agent, gc.nogc());
54+
let message = arguments.get(1).scope(agent, gc.nogc());
5555
let options = arguments.get(2);
5656
// 1. If NewTarget is undefined, let newTarget be the active function object; else let newTarget be NewTarget.
5757
let new_target = new_target.map_or_else(
@@ -67,10 +67,11 @@ impl AggregateErrorConstructor {
6767
)?;
6868
let o = Error::try_from(o.unbind()).unwrap();
6969
// 3. If message is not undefined, then
70+
let message = message.get(agent).bind(gc.nogc());
7071
let message = if !message.is_undefined() {
7172
// a. Let msg be ? ToString(message).
7273
Some(
73-
to_string(agent, message, gc.reborrow())?
74+
to_string(agent, message.unbind(), gc.reborrow())?
7475
.unbind()
7576
.scope(agent, gc.nogc()),
7677
)
@@ -86,7 +87,7 @@ impl AggregateErrorConstructor {
8687
heap_data.message = message;
8788
heap_data.cause = cause.map(|c| c.unbind());
8889
// 5. Let errorsList be ? IteratorToList(? GetIterator(errors, sync)).
89-
let Some(iterator_record) = get_iterator(agent, errors.unbind(), false, gc.reborrow())?
90+
let Some(iterator_record) = get_iterator(agent, errors.get(agent), false, gc.reborrow())?
9091
else {
9192
return Err(throw_not_callable(agent, gc.into_nogc()));
9293
};

‎nova_vm/src/ecmascript/builtins/fundamental_objects/object_objects/object_prototype.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,12 @@ impl ObjectPrototype {
101101
arguments: ArgumentsList,
102102
gc: GcScope<'gc, '_>,
103103
) -> JsResult<Value<'gc>> {
104-
let v = arguments.get(0);
104+
let v = arguments.get(0).bind(gc.nogc());
105105
let Ok(v) = Object::try_from(v) else {
106106
return Ok(false.into());
107107
};
108108
let o = to_object(agent, this_value, gc.nogc())?;
109-
let result = is_prototype_of_loop(agent, o.unbind(), v, gc)?;
109+
let result = is_prototype_of_loop(agent, o.unbind(), v.unbind(), gc)?;
110110
Ok(result.into())
111111
}
112112

‎nova_vm/src/ecmascript/builtins/fundamental_objects/symbol_objects/symbol_constructor.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ impl SymbolConstructor {
7373
gc.nogc(),
7474
));
7575
}
76-
let description = arguments.get(0);
76+
let description = arguments.get(0).bind(gc.nogc());
7777
let desc_string = if description.is_undefined() {
7878
None
7979
} else {
80-
Some(to_string(agent, description, gc)?.unbind())
80+
Some(to_string(agent, description.unbind(), gc)?.unbind())
8181
};
8282

8383
Ok(agent

‎nova_vm/src/ecmascript/builtins/global_object.rs

+15-12
Original file line numberDiff line numberDiff line change
@@ -729,10 +729,10 @@ impl GlobalObject {
729729
arguments: ArgumentsList,
730730
mut gc: GcScope<'gc, '_>,
731731
) -> JsResult<Value<'gc>> {
732-
let x = arguments.get(0);
732+
let x = arguments.get(0).bind(gc.nogc());
733733

734734
// 1. Return ? PerformEval(x, false, false).
735-
perform_eval(agent, x, false, false, gc.reborrow()).map(|v| v.unbind())
735+
perform_eval(agent, x.unbind(), false, false, gc.reborrow()).map(|v| v.unbind())
736736
}
737737

738738
/// ### [19.2.2 isFinite ( number )](https://tc39.es/ecma262/#sec-isfinite-number)
@@ -744,9 +744,9 @@ impl GlobalObject {
744744
arguments: ArgumentsList,
745745
mut gc: GcScope<'gc, '_>,
746746
) -> JsResult<Value<'gc>> {
747-
let number = arguments.get(0);
747+
let number = arguments.get(0).bind(gc.nogc());
748748
// 1. Let num be ? ToNumber(number).
749-
let num = to_number(agent, number, gc.reborrow())?;
749+
let num = to_number(agent, number.unbind(), gc.reborrow())?;
750750
// 2. If num is not finite, return false.
751751
// 3. Otherwise, return true.
752752
Ok(num.is_finite(agent).into())
@@ -765,9 +765,9 @@ impl GlobalObject {
765765
arguments: ArgumentsList,
766766
mut gc: GcScope<'gc, '_>,
767767
) -> JsResult<Value<'gc>> {
768-
let number = arguments.get(0);
768+
let number = arguments.get(0).bind(gc.nogc());
769769
// 1. Let num be ? ToNumber(number).
770-
let num = to_number(agent, number, gc.reborrow())?;
770+
let num = to_number(agent, number.unbind(), gc.reborrow())?;
771771
// 2. If num is NaN, return true.
772772
// 3. Otherwise, return false.
773773
Ok(num.is_nan(agent).into())
@@ -787,10 +787,10 @@ impl GlobalObject {
787787
return Ok(Value::nan());
788788
}
789789

790-
let string = arguments.get(0);
790+
let string = arguments.get(0).bind(gc.nogc());
791791

792792
// 1. Let inputString be ? ToString(string).
793-
let input_string = to_string(agent, string, gc.reborrow())?;
793+
let input_string = to_string(agent, string.unbind(), gc.reborrow())?;
794794

795795
// 2. Let trimmedString be ! TrimString(inputString, start).
796796
let trimmed_string = input_string
@@ -852,8 +852,8 @@ impl GlobalObject {
852852
arguments: ArgumentsList,
853853
mut gc: GcScope<'gc, '_>,
854854
) -> JsResult<Value<'gc>> {
855-
let string = arguments.get(0);
856-
let radix = arguments.get(1);
855+
let string = arguments.get(0).bind(gc.nogc());
856+
let radix = arguments.get(1).bind(gc.nogc());
857857

858858
// OPTIMIZATION: If the string is empty, undefined, null or a boolean, return NaN.
859859
if string.is_undefined()
@@ -872,12 +872,15 @@ impl GlobalObject {
872872
}
873873
}
874874

875+
let radix = radix.scope(agent, gc.nogc());
876+
875877
// 1. Let inputString be ? ToString(string).
876-
let mut s = to_string(agent, string, gc.reborrow())?
878+
let mut s = to_string(agent, string.unbind(), gc.reborrow())?
877879
.unbind()
878880
.bind(gc.nogc());
879881

880882
// 6. Let R be ℝ(? ToInt32(radix)).
883+
let radix = radix.get(agent).bind(gc.nogc());
881884
let r = if let Value::Integer(radix) = radix {
882885
radix.into_i64() as i32
883886
} else if radix.is_undefined() {
@@ -887,7 +890,7 @@ impl GlobalObject {
887890
to_int32_number(agent, radix)
888891
} else {
889892
let s_root = s.scope(agent, gc.nogc());
890-
let radix = to_int32(agent, radix, gc.reborrow())?;
893+
let radix = to_int32(agent, radix.unbind(), gc.reborrow())?;
891894
s = s_root.get(agent).bind(gc.nogc());
892895
radix
893896
};

‎nova_vm/src/ecmascript/builtins/numbers_and_dates/bigint_objects/bigint_constructor.rs

+21-7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use num_traits::Pow;
88
use crate::ecmascript::abstract_operations::testing_and_comparison::is_integral_number;
99
use crate::ecmascript::abstract_operations::type_conversion::PreferredType;
1010
use crate::ecmascript::abstract_operations::type_conversion::to_big_int;
11+
use crate::ecmascript::abstract_operations::type_conversion::to_big_int_primitive;
1112
use crate::ecmascript::abstract_operations::type_conversion::to_index;
1213
use crate::ecmascript::abstract_operations::type_conversion::to_primitive;
1314
use crate::ecmascript::builders::builtin_function_builder::BuiltinFunctionBuilder;
@@ -31,6 +32,7 @@ use crate::ecmascript::types::{String, Value};
3132

3233
use crate::SmallInteger;
3334
use crate::engine::context::{Bindable, GcScope};
35+
use crate::engine::rootable::Scopable;
3436
use crate::heap::CreateHeapData;
3537
use crate::heap::IntrinsicConstructorIndexes;
3638

@@ -74,8 +76,13 @@ impl BigIntConstructor {
7476
gc.nogc(),
7577
));
7678
}
77-
let value = arguments.get(0);
78-
let prim = to_primitive(agent, value, Some(PreferredType::Number), gc.reborrow())?;
79+
let value = arguments.get(0).bind(gc.nogc());
80+
let prim = to_primitive(
81+
agent,
82+
value.unbind(),
83+
Some(PreferredType::Number),
84+
gc.reborrow(),
85+
)?;
7986
if let Ok(prim) = Number::try_from(prim) {
8087
if !prim.is_integer(agent) {
8188
return Err(agent.throw_exception_with_static_message(
@@ -87,25 +94,29 @@ impl BigIntConstructor {
8794

8895
Ok(BigInt::from_i64(agent, prim.into_i64(agent)).into_value())
8996
} else {
90-
to_big_int(agent, value, gc.reborrow()).map(|result| result.into_value().unbind())
97+
to_big_int_primitive(agent, prim.unbind(), gc.into_nogc())
98+
.map(|result| result.into_value())
9199
}
92100
}
93101

102+
/// ### [21.2.2.1 BigInt.asIntN ( bits, bigint )](https://tc39.es/ecma262/#sec-bigint.asintn)
94103
fn as_int_n<'gc>(
95104
agent: &mut Agent,
96105
_this_value: Value,
97106
arguments: ArgumentsList,
98107
mut gc: GcScope<'gc, '_>,
99108
) -> JsResult<Value<'gc>> {
100-
let bits = to_index(agent, arguments.get(0), gc.reborrow())?;
109+
let bits = arguments.get(0).bind(gc.nogc());
110+
let bigint = arguments.get(1).scope(agent, gc.nogc());
111+
let bits = to_index(agent, bits.unbind(), gc.reborrow())?;
101112
let Ok(bits) = u32::try_from(bits) else {
102113
return Err(agent.throw_exception_with_static_message(
103114
ExceptionType::RangeError,
104115
"Ridiculous bits value for BigInt.asIntN",
105116
gc.nogc(),
106117
));
107118
};
108-
let bigint = to_big_int(agent, arguments.get(1), gc.reborrow())?;
119+
let bigint = to_big_int(agent, bigint.get(agent), gc.reborrow())?;
109120
if bits == 0 {
110121
return Ok(BigInt::zero().into_value());
111122
}
@@ -169,21 +180,24 @@ impl BigIntConstructor {
169180
}
170181
}
171182

183+
/// ### [21.2.2.2 BigInt.asUintN ( bits, bigint )](https://tc39.es/ecma262/#sec-bigint.asuintn)
172184
fn as_uint_n<'gc>(
173185
agent: &mut Agent,
174186
_this_value: Value,
175187
arguments: ArgumentsList,
176188
mut gc: GcScope<'gc, '_>,
177189
) -> JsResult<Value<'gc>> {
178-
let bits = to_index(agent, arguments.get(0), gc.reborrow())?;
190+
let bits = arguments.get(0).bind(gc.nogc());
191+
let bigint = arguments.get(1).scope(agent, gc.nogc());
192+
let bits = to_index(agent, bits.unbind(), gc.reborrow())?;
179193
let Ok(bits) = u32::try_from(bits) else {
180194
return Err(agent.throw_exception_with_static_message(
181195
ExceptionType::RangeError,
182196
"Ridiculous bits value for BigInt.asUintN",
183197
gc.nogc(),
184198
));
185199
};
186-
let bigint = to_big_int(agent, arguments.get(1), gc.reborrow())?;
200+
let bigint = to_big_int(agent, bigint.get(agent), gc.reborrow())?;
187201
match bigint {
188202
BigInt::BigInt(_) => todo!(),
189203
BigInt::SmallBigInt(int) => {

‎nova_vm/src/ecmascript/builtins/numbers_and_dates/bigint_objects/bigint_prototype.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ impl BigIntPrototype {
6060
gc: GcScope<'gc, '_>,
6161
) -> JsResult<Value<'gc>> {
6262
let _x = this_big_int_value(agent, this_value, gc.nogc())?;
63-
let radix = arguments.get(0);
63+
let radix = arguments.get(0).bind(gc.nogc());
6464
if radix.is_undefined() || radix == Value::from(10u8) {
6565
// BigInt::to_string_radix_10(agent, x).map(|result| result.into_value())
6666
todo!();

0 commit comments

Comments
 (0)