Skip to content

Commit ab5705d

Browse files
KaustubhOGKaustubhOG
committed
fix(object/operations,string): convert remaining panics to EngineError::Panic
1 parent d6d76d8 commit ab5705d

4 files changed

Lines changed: 83 additions & 26 deletions

File tree

‎core/engine/src/object/builtins/jsdataview.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
use crate::{
33
Context, JsExpect, JsNativeError, JsResult, JsValue,
44
builtins::{DataView, array_buffer::BufferObject},
5+
error::PanicError,
56
object::{JsArrayBuffer, JsObject},
67
value::TryFromJs,
78
};
@@ -60,7 +61,9 @@ impl JsDataView {
6061
let offset = offset.unwrap_or_default();
6162

6263
let (buf_byte_len, is_fixed_len) = {
63-
let buffer = buffer.borrow();
64+
let buffer = buffer
65+
.try_borrow()
66+
.map_err(|e| PanicError::new(e.to_string()))?;
6467
let buffer = buffer.data();
6568

6669
// 4. If IsDetachedBuffer(buffer) is true, throw a TypeError exception.
@@ -110,7 +113,13 @@ impl JsDataView {
110113

111114
// 11. If IsDetachedBuffer(buffer) is true, throw a TypeError exception.
112115
// 12. Set bufferByteLength to ArrayBufferByteLength(buffer, seq-cst).
113-
let Some(buf_byte_len) = buffer.borrow().data().bytes().map(|s| s.len() as u64) else {
116+
let Some(buf_byte_len) = buffer
117+
.try_borrow()
118+
.map_err(|e| PanicError::new(e.to_string()))?
119+
.data()
120+
.bytes()
121+
.map(|s| s.len() as u64)
122+
else {
114123
return Err(JsNativeError::typ()
115124
.with_message("ArrayBuffer is detached")
116125
.into());

‎core/engine/src/object/internal_methods/mod.rs‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use super::{
1414
use crate::{
1515
Context, JsNativeError, JsResult,
1616
context::intrinsics::{StandardConstructor, StandardConstructors},
17+
error::PanicError,
1718
object::JsObject,
1819
property::{DescriptorKind, PropertyDescriptor, PropertyKey},
1920
value::JsValue,
@@ -588,7 +589,10 @@ pub(crate) fn ordinary_set_prototype_of(
588589
#[allow(clippy::unnecessary_wraps)]
589590
pub(crate) fn ordinary_is_extensible(obj: &JsObject, _context: &mut Context) -> JsResult<bool> {
590591
// 1. Return O.[[Extensible]].
591-
Ok(obj.borrow().extensible)
592+
Ok(obj
593+
.try_borrow()
594+
.map_err(|e| PanicError::new(e.to_string()))?
595+
.extensible)
592596
}
593597

594598
/// Abstract operation `OrdinaryPreventExtensions`.
@@ -603,7 +607,9 @@ pub(crate) fn ordinary_prevent_extensions(
603607
_context: &mut Context,
604608
) -> JsResult<bool> {
605609
// 1. Set O.[[Extensible]] to false.
606-
obj.borrow_mut().extensible = false;
610+
obj.try_borrow_mut()
611+
.map_err(|e| PanicError::new(e.to_string()))?
612+
.extensible = false;
607613

608614
// 2. Return true.
609615
Ok(true)
@@ -635,7 +641,11 @@ pub(crate) fn ordinary_get_own_property(
635641
// 7. Set D.[[Enumerable]] to the value of X's [[Enumerable]] attribute.
636642
// 8. Set D.[[Configurable]] to the value of X's [[Configurable]] attribute.
637643
// 9. Return D.
638-
Ok(obj.borrow().properties.get_with_slot(key, context.slot()))
644+
Ok(obj
645+
.try_borrow()
646+
.map_err(|e| PanicError::new(e.to_string()))?
647+
.properties
648+
.get_with_slot(key, context.slot()))
639649
}
640650

641651
/// Abstract operation `OrdinaryDefineOwnProperty`.
@@ -945,7 +955,9 @@ pub(crate) fn ordinary_delete(
945955
// 4. If desc.[[Configurable]] is true, then
946956
Some(desc) if desc.expect_configurable() => {
947957
// a. Remove the own property with name P from O.
948-
obj.borrow_mut().remove(key);
958+
obj.try_borrow_mut()
959+
.map_err(|e| PanicError::new(e.to_string()))?
960+
.remove(key);
949961
// b. Return true.
950962
true
951963
}
@@ -972,7 +984,12 @@ pub(crate) fn ordinary_own_property_keys(
972984
let mut keys = Vec::new();
973985

974986
let ordered_indexes = {
975-
let mut indexes: Vec<_> = obj.borrow().properties.index_property_keys().collect();
987+
let mut indexes: Vec<_> = obj
988+
.try_borrow()
989+
.map_err(|e| PanicError::new(e.to_string()))?
990+
.properties
991+
.index_property_keys()
992+
.collect();
976993
indexes.sort_unstable();
977994
indexes
978995
};
@@ -986,7 +1003,13 @@ pub(crate) fn ordinary_own_property_keys(
9861003
//
9871004
// 4. For each own property key P of O such that Type(P) is Symbol, in ascending chronological order of property creation, do
9881005
// a. Add P as the last element of keys.
989-
keys.extend(obj.borrow().properties.shape.keys());
1006+
keys.extend(
1007+
obj.try_borrow()
1008+
.map_err(|e| PanicError::new(e.to_string()))?
1009+
.properties
1010+
.shape
1011+
.keys(),
1012+
);
9901013

9911014
// 5. Return keys.
9921015
Ok(keys)

‎core/engine/src/object/internal_methods/string.rs‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::{
22
Context, JsExpect, JsResult, JsString,
3+
error::PanicError,
34
object::{JsData, JsObject},
45
property::{PropertyDescriptor, PropertyKey},
56
};
@@ -39,7 +40,7 @@ pub(crate) fn string_exotic_get_own_property(
3940
Ok(desc)
4041
} else {
4142
// 4. Return ! StringGetOwnProperty(S, P).
42-
Ok(string_get_own_property(obj, key))
43+
string_get_own_property(obj, key)
4344
}
4445
}
4546

@@ -57,12 +58,15 @@ pub(crate) fn string_exotic_define_own_property(
5758
) -> JsResult<bool> {
5859
// 1. Assert: IsPropertyKey(P) is true.
5960
// 2. Let stringDesc be ! StringGetOwnProperty(S, P).
60-
let string_desc = string_get_own_property(obj, key);
61+
let string_desc = string_get_own_property(obj, key)?;
6162

6263
// 3. If stringDesc is not undefined, then
6364
if let Some(string_desc) = string_desc {
6465
// a. Let extensible be S.[[Extensible]].
65-
let extensible = obj.borrow().extensible;
66+
let extensible = obj
67+
.try_borrow()
68+
.map_err(|e| PanicError::new(e.to_string()))?
69+
.extensible;
6670
// b. Return ! IsCompatiblePropertyDescriptor(extensible, Desc, stringDesc).
6771
Ok(super::is_compatible_property_descriptor(
6872
extensible,
@@ -106,7 +110,8 @@ pub(crate) fn string_exotic_own_property_keys(
106110
// and ! ToIntegerOrInfinity(P) ≥ len, in ascending numeric index order, do
107111
// a. Add P as the last element of keys.
108112
let mut remaining_indices: Vec<_> = obj
109-
.borrow()
113+
.try_borrow()
114+
.map_err(|e| PanicError::new(e.to_string()))?
110115
.properties
111116
.index_property_keys()
112117
.filter(|idx| (*idx as usize) >= len)
@@ -121,7 +126,13 @@ pub(crate) fn string_exotic_own_property_keys(
121126
// 8. For each own property key P of O such that Type(P) is Symbol, in ascending
122127
// chronological order of property creation, do
123128
// a. Add P as the last element of keys.
124-
keys.extend(obj.borrow().properties.shape.keys());
129+
keys.extend(
130+
obj.try_borrow()
131+
.map_err(|e| PanicError::new(e.to_string()))?
132+
.properties
133+
.shape
134+
.keys(),
135+
);
125136

126137
// 9. Return keys.
127138
Ok(keys)
@@ -133,7 +144,10 @@ pub(crate) fn string_exotic_own_property_keys(
133144
/// - [ECMAScript reference][spec]
134145
///
135146
/// [spec]: https://tc39.es/ecma262/#sec-stringgetownproperty
136-
fn string_get_own_property(obj: &JsObject, key: &PropertyKey) -> Option<PropertyDescriptor> {
147+
fn string_get_own_property(
148+
obj: &JsObject,
149+
key: &PropertyKey,
150+
) -> JsResult<Option<PropertyDescriptor>> {
137151
// 1. Assert: S is an Object that has a [[StringData]] internal slot.
138152
// 2. Assert: IsPropertyKey(P) is true.
139153
// 3. If Type(P) is not String, return undefined.
@@ -143,20 +157,23 @@ fn string_get_own_property(obj: &JsObject, key: &PropertyKey) -> Option<Property
143157
// 7. If index is -0𝔽, return undefined.
144158
let pos = match key {
145159
PropertyKey::Index(index) => index.get() as usize,
146-
_ => return None,
160+
_ => return Ok(None),
147161
};
148162

149163
// 8. Let str be S.[[StringData]].
150164
// 9. Assert: Type(str) is String.
151165
let string = obj
152166
.downcast_ref::<JsString>()
153-
.expect("string exotic method should only be callable from string objects")
167+
.js_expect("string exotic method should only be callable from string objects")?
154168
.clone();
155169

156170
// 10. Let len be the length of str.
157171
// 11. If ℝ(index) < 0 or len ≤ ℝ(index), return undefined.
158-
// 12. Let resultStr be the String value of length 1, containing one code unit from str, specifically the code unit at index ℝ(index).
159-
let result_str = string.get(pos..=pos)?;
172+
// 12. Let resultStr be the String value of length 1, containing one code unit from str,
173+
// specifically the code unit at index ℝ(index).
174+
let Some(result_str) = string.get(pos..=pos) else {
175+
return Ok(None);
176+
};
160177

161178
// 13. Return the PropertyDescriptor { [[Value]]: resultStr, [[Writable]]: false, [[Enumerable]]: true, [[Configurable]]: false }.
162179
let desc = PropertyDescriptor::builder()
@@ -166,5 +183,5 @@ fn string_get_own_property(obj: &JsObject, key: &PropertyKey) -> Option<Property
166183
.configurable(false)
167184
.build();
168185

169-
Some(desc)
186+
Ok(Some(desc))
170187
}

‎core/engine/src/object/operations.rs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
function::{BoundFunction, ClassFieldDefinition, OrdinaryFunction, set_function_name},
99
},
1010
context::intrinsics::{StandardConstructor, StandardConstructors},
11-
error::JsNativeError,
11+
error::{JsNativeError, PanicError},
1212
native_function::NativeFunctionObject,
1313
object::{CONSTRUCTOR, JsObject, PROTOTYPE, PrivateElement, PrivateName},
1414
property::{PropertyDescriptor, PropertyDescriptorBuilder, PropertyKey, PropertyNameKind},
@@ -656,7 +656,9 @@ impl JsObject {
656656
// NOTE: This is an optimization, most of the cases that `LengthOfArrayLike` will be called
657657
// is for arrays. The "length" property of an array is stored in the first index.
658658
if self.is_array() {
659-
let borrowed_object = self.borrow();
659+
let borrowed_object = self
660+
.try_borrow()
661+
.map_err(|e| PanicError::new(e.to_string()))?;
660662
// NOTE: using `to_u32` instead of `to_length` is an optimization,
661663
// since arrays are limited to [0, 2^32 - 1] range.
662664
return borrowed_object.properties().storage[0]
@@ -906,7 +908,7 @@ impl JsObject {
906908
// 4. Let from be ! ToObject(source).
907909
let from = source
908910
.to_object(context)
909-
.expect("function ToObject should never complete abruptly here");
911+
.js_expect("function ToObject should never complete abruptly here")?;
910912

911913
// 5. Let keys be ? from.[[OwnPropertyKeys]]().
912914
// 6. For each element nextKey of keys, do
@@ -939,7 +941,9 @@ impl JsObject {
939941

940942
// 2. Perform ! CreateDataPropertyOrThrow(target, nextKey, propValue).
941943
self.create_data_property_or_throw(key, prop_value, context)
942-
.expect("CreateDataPropertyOrThrow should never complete abruptly here");
944+
.js_expect(
945+
"CreateDataPropertyOrThrow should never complete abruptly here",
946+
)?;
943947
}
944948
}
945949
}
@@ -1019,7 +1023,8 @@ impl JsObject {
10191023
}
10201024

10211025
// 5. Append PrivateElement { [[Key]]: P, [[Kind]]: field, [[Value]]: value } to O.[[PrivateElements]].
1022-
self.borrow_mut()
1026+
self.try_borrow_mut()
1027+
.map_err(|e| PanicError::new(e.to_string()))?
10231028
.private_elements
10241029
.push((name.clone(), PrivateElement::Field(value)));
10251030

@@ -1090,7 +1095,8 @@ impl JsObject {
10901095
}
10911096

10921097
// 5. Append method to O.[[PrivateElements]].
1093-
self.borrow_mut()
1098+
self.try_borrow_mut()
1099+
.map_err(|e| PanicError::new(e.to_string()))?
10941100
.append_private_element(name.clone(), method.clone());
10951101

10961102
// 6. Return unused.
@@ -1155,7 +1161,9 @@ impl JsObject {
11551161
) -> JsResult<()> {
11561162
// 1. Let entry be PrivateElementFind(O, P).
11571163
// Note: This function is inlined here for mutable access.
1158-
let mut object_mut = self.borrow_mut();
1164+
let mut object_mut = self
1165+
.try_borrow_mut()
1166+
.map_err(|e| PanicError::new(e.to_string()))?;
11591167
let entry = object_mut
11601168
.private_elements
11611169
.iter_mut()

0 commit comments

Comments
 (0)