system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-09-18 15:32:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "LibJS: Get rid of unnecessary work from canonical_numeric_index_string"

Browse source 
This reverts commit 3a184f7841.

This broke a number of test262 tests under "TypedArrayConstructors".
The issue is that the CanonicalNumericIndexString AO should not fail
for inputs like "1.1", despite them not being integral indices.
This commit is contained in:
Andreas Kling 2022-02-13 15:35:15 +01:00
parent b193351a99
commit 4b412e8fee
9 changed files with 105 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator.cpp
View file

@ -2501,7 +2501,7 @@ JS::ThrowCompletionOr<Optional<JS::PropertyDescriptor>> @class_name@::legacy_pla
if (interface.supports_indexed_properties()) {
// ...and P is an array index, then:
get_own_property_generator.append(R"~~~(
if (IDL::is_an_array_index(property_name)) {
if (IDL::is_an_array_index(global_object, property_name)) {
// 1. Let index be the result of calling ToUint32(P).
u32 index = property_name.as_number();
@ -2776,7 +2776,7 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_set(JS::PropertyKey const& pr
if (interface.indexed_property_setter.has_value()) {
// ...and P is an array index, then:
scoped_generator.append(R"~~~(
if (IDL::is_an_array_index(property_name)) {
if (IDL::is_an_array_index(global_object, property_name)) {
// 1. Invoke the indexed property setter on O with P and V.
TRY(invoke_indexed_property_setter(global_object, impl(), property_name, value));
@ -2821,14 +2821,14 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_set(JS::PropertyKey const& pr
JS::ThrowCompletionOr<bool> @class_name@::internal_define_own_property(JS::PropertyKey const& property_name, JS::PropertyDescriptor const& property_descriptor)
{
[[maybe_unused]] auto& vm = this->vm();
[[maybe_unused]] auto& global_object = this->global_object();
auto& global_object = this->global_object();
)~~~");
// 1. If O supports indexed properties...
if (interface.supports_indexed_properties()) {
// ...and P is an array index, then:
scoped_generator.append(R"~~~(
if (IDL::is_an_array_index(property_name)) {
if (IDL::is_an_array_index(global_object, property_name)) {
// 1. If the result of calling IsDataDescriptor(Desc) is false, then return false.
if (!property_descriptor.is_data_descriptor())
return false;
@ -2940,14 +2940,14 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_define_own_property(JS::Prope
scoped_generator.append(R"~~~(
JS::ThrowCompletionOr<bool> @class_name@::internal_delete(JS::PropertyKey const& property_name)
{
[[maybe_unused]] auto& global_object = this->global_object();
auto& global_object = this->global_object();
)~~~");
// 1. If O supports indexed properties...
if (interface.supports_indexed_properties()) {
// ...and P is an array index, then:
scoped_generator.append(R"~~~(
if (IDL::is_an_array_index(property_name)) {
if (IDL::is_an_array_index(global_object, property_name)) {
// 1. Let index be the result of calling ToUint32(P).
u32 index = property_name.as_number();

24
Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
View file

@ -1031,7 +1031,7 @@ Object* create_mapped_arguments_object(GlobalObject& global_object, FunctionObje
}
// 7.1.21 CanonicalNumericIndexString ( argument ), https://tc39.es/ecma262/#sec-canonicalnumericindexstring
Optional<u32> canonical_numeric_index_string(PropertyKey const& property_key)
Value canonical_numeric_index_string(GlobalObject& global_object, PropertyKey const& property_key)
{
// NOTE: If the property name is a number type (An implementation-defined optimized
// property key type), it can be treated as a string property that has already been
@ -1039,11 +1039,25 @@ Optional<u32> canonical_numeric_index_string(PropertyKey const& property_key)
VERIFY(property_key.is_string() || property_key.is_number());
// If property_key is a string containing a canonical numeric index
// the act of calling is_number() will return true
if (property_key.is_number())
return property_key.as_number();
return {};
return Value(property_key.as_number());
// 1. Assert: Type(argument) is String.
auto argument = Value(js_string(global_object.vm(), property_key.as_string()));
// 2. If argument is "-0", return -0𝔽.
if (argument.as_string().string() == "-0")
return Value(-0.0);
// 3. Let n be ! ToNumber(argument).
auto n = MUST(argument.to_number(global_object));
// 4. If SameValue(! ToString(n), argument) is false, return undefined.
if (!same_value(MUST(n.to_primitive_string(global_object)), argument))
return js_undefined();
// 5. Return n.
return n;
}
// 22.1.3.17.1 GetSubstitution ( matched, str, position, captures, namedCaptures, replacement ), https://tc39.es/ecma262/#sec-getsubstitution

3
Userland/Libraries/LibJS/Runtime/AbstractOperations.h
View file

@ -39,9 +39,8 @@ bool validate_and_apply_property_descriptor(Object*, PropertyKey const&, bool ex
ThrowCompletionOr<Object*> get_prototype_from_constructor(GlobalObject&, FunctionObject const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)());
Object* create_unmapped_arguments_object(GlobalObject&, Span<Value> arguments);
Object* create_mapped_arguments_object(GlobalObject&, FunctionObject&, Vector<FunctionNode::Parameter> const&, Span<Value> arguments, Environment&);
Optional<u32> canonical_numeric_index_string(PropertyKey const&);
Value canonical_numeric_index_string(GlobalObject&, PropertyKey const&);
ThrowCompletionOr<String> get_substitution(GlobalObject&, Utf16View const& matched, Utf16View const& str, size_t position, Span<Value> captures, Value named_captures, Value replacement);
double string_to_number(const PrimitiveString&);
enum class CallerMode {
Strict,

10
Userland/Libraries/LibJS/Runtime/PrimitiveString.cpp
View file

@ -65,14 +65,16 @@ Optional<Value> PrimitiveString::get(GlobalObject& global_object, PropertyKey co
return Value(static_cast<double>(length));
}
}
auto index = canonical_numeric_index_string(property_key);
if (!index.has_value())
auto index = canonical_numeric_index_string(global_object, property_key);
if (index.type() != JS::Value::Type::Int32)
return {};
if (index.as_i32() < 0)
return {};
auto str = utf16_string_view();
auto length = str.length_in_code_units();
if (length <= *index)
if (static_cast<i32>(length) <= index.as_i32())
return {};
return js_string(vm(), str.substring_view(*index, 1));
return js_string(vm(), str.substring_view(index.as_i32(), 1));
}
PrimitiveString* js_string(Heap& heap, Utf16View const& view)

27
Userland/Libraries/LibJS/Runtime/StringObject.cpp
View file

@ -44,7 +44,7 @@ void StringObject::visit_edges(Cell::Visitor& visitor)
}
// 10.4.3.5 StringGetOwnProperty ( S, P ), https://tc39.es/ecma262/#sec-stringgetownproperty
static Optional<PropertyDescriptor> string_get_own_property(StringObject const& string, PropertyKey const& property_key)
static Optional<PropertyDescriptor> string_get_own_property(GlobalObject& global_object, StringObject const& string, PropertyKey const& property_key)
{
// 1. Assert: S is an Object that has a [[StringData]] internal slot.
// 2. Assert: IsPropertyKey(P) is true.
@ -57,14 +57,17 @@ static Optional<PropertyDescriptor> string_get_own_property(StringObject const&
return {};
// 4. Let index be ! CanonicalNumericIndexString(P).
auto index = canonical_numeric_index_string(property_key);
if (!index.has_value()) {
// 5. If index is undefined, return undefined.
// 6. If IsIntegralNumber(index) is false, return undefined.
// 7. If index is -0𝔽, return undefined.
auto index = canonical_numeric_index_string(global_object, property_key);
// 5. If index is undefined, return undefined.
if (index.is_undefined())
return {};
}
// 6. If IsIntegralNumber(index) is false, return undefined.
if (!index.is_integral_number())
return {};
// 7. If index is -0𝔽, return undefined.
if (index.is_negative_zero())
return {};
// 8. Let str be S.[[StringData]].
// 9. Assert: Type(str) is String.
auto str = string.primitive_string().utf16_string_view();
@ -73,11 +76,11 @@ static Optional<PropertyDescriptor> string_get_own_property(StringObject const&
auto length = str.length_in_code_units();
// 11. If ℝ(index) < 0 or len ≤ ℝ(index), return undefined.
if (length <= static_cast<u64>(*index))
if (index.as_double() < 0 || length <= index.as_double())
return {};
// 12. Let resultStr be the String value of length 1, containing one code unit from str, specifically the code unit at index ℝ(index).
auto result_str = js_string(string.vm(), str.substring_view(*index, 1));
auto result_str = js_string(string.vm(), str.substring_view(index.as_double(), 1));
// 13. Return the PropertyDescriptor { [[Value]]: resultStr, [[Writable]]: false, [[Enumerable]]: true, [[Configurable]]: false }.
return PropertyDescriptor {
@ -101,7 +104,7 @@ ThrowCompletionOr<Optional<PropertyDescriptor>> StringObject::internal_get_own_p
return descriptor;
// 4. Return ! StringGetOwnProperty(S, P).
return string_get_own_property(*this, property_key);
return string_get_own_property(global_object(), *this, property_key);
}
// 10.4.3.2 [[DefineOwnProperty]] ( P, Desc ), https://tc39.es/ecma262/#sec-string-exotic-objects-defineownproperty-p-desc
@ -111,7 +114,7 @@ ThrowCompletionOr<bool> StringObject::internal_define_own_property(PropertyKey c
VERIFY(property_key.is_valid());
// 2. Let stringDesc be ! StringGetOwnProperty(S, P).
auto string_descriptor = string_get_own_property(*this, property_key);
auto string_descriptor = string_get_own_property(global_object(), *this, property_key);
// 3. If stringDesc is not undefined, then
if (string_descriptor.has_value()) {

51
Userland/Libraries/LibJS/Runtime/TypedArray.h
View file

@ -74,7 +74,7 @@ private:
};
// 10.4.5.9 IsValidIntegerIndex ( O, index ), https://tc39.es/ecma262/#sec-isvalidintegerindex
inline bool is_valid_integer_index(TypedArrayBase const& typed_array, u32 property_index)
inline bool is_valid_integer_index(TypedArrayBase const& typed_array, Value property_index)
{
if (typed_array.viewed_array_buffer()->is_detached())
return false;
@ -82,8 +82,15 @@ inline bool is_valid_integer_index(TypedArrayBase const& typed_array, u32 proper
// TODO: This can be optimized by skipping the following 3 out of 4 checks if property_index
// came from a number-type PropertyKey instead of a canonicalized string-type PropertyKey
// If ! IsIntegralNumber(index) is false, return false.
if (!property_index.is_integral_number())
return false;
// If index is -0𝔽, return false.
if (property_index.is_negative_zero())
return false;
// If ℝ(index) < 0 or ℝ(index) ≥ O.[[ArrayLength]], return false.
if (property_index >= typed_array.array_length())
if (property_index.as_double() < 0 || property_index.as_double() >= typed_array.array_length())
return false;
return true;
@ -91,7 +98,7 @@ inline bool is_valid_integer_index(TypedArrayBase const& typed_array, u32 proper
// 10.4.5.10 IntegerIndexedElementGet ( O, index ), https://tc39.es/ecma262/#sec-integerindexedelementget
template<typename T>
inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, i64 property_index)
inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, Value property_index)
{
// 1. Assert: O is an Integer-Indexed exotic object.
@ -105,7 +112,7 @@ inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, i64
// 4. Let arrayTypeName be the String value of O.[[TypedArrayName]].
// 5. Let elementSize be the Element Size value specified in Table 64 for arrayTypeName.
// 6. Let indexedPosition be (ℝ(index) × elementSize) + offset.
Checked<size_t> indexed_position = property_index;
Checked<size_t> indexed_position = (i64)property_index.as_double();
indexed_position *= typed_array.element_size();
indexed_position += offset;
// FIXME: Not exactly sure what we should do when overflow occurs.
@ -123,7 +130,7 @@ inline Value integer_indexed_element_get(TypedArrayBase const& typed_array, i64
// 10.4.5.11 IntegerIndexedElementSet ( O, index, value ), https://tc39.es/ecma262/#sec-integerindexedelementset
// NOTE: In error cases, the function will return as if it succeeded.
template<typename T>
inline ThrowCompletionOr<void> integer_indexed_element_set(TypedArrayBase& typed_array, i64 property_index, Value value)
inline ThrowCompletionOr<void> integer_indexed_element_set(TypedArrayBase& typed_array, Value property_index, Value value)
{
VERIFY(!value.is_empty());
auto& global_object = typed_array.global_object();
@ -150,7 +157,7 @@ inline ThrowCompletionOr<void> integer_indexed_element_set(TypedArrayBase& typed
// b. Let arrayTypeName be the String value of O.[[TypedArrayName]].
// c. Let elementSize be the Element Size value specified in Table 64 for arrayTypeName.
// d. Let indexedPosition be (ℝ(index) × elementSize) + offset.
Checked<size_t> indexed_position = property_index;
Checked<size_t> indexed_position = (i64)property_index.as_double();
indexed_position *= typed_array.element_size();
indexed_position += offset;
// FIXME: Not exactly sure what we should do when overflow occurs.
@ -191,10 +198,11 @@ public:
// NOTE: This includes an implementation-defined optimization, see note above!
if (property_key.is_string() || property_key.is_number()) {
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
// b. If numericIndex is not undefined, then
if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
if (!numeric_index.is_undefined()) {
// i. Let value be ! IntegerIndexedElementGet(O, numericIndex).
auto value = integer_indexed_element_get<T>(*this, *numeric_index);
auto value = integer_indexed_element_get<T>(*this, numeric_index);
// ii. If value is undefined, return undefined.
if (value.is_undefined())
@ -230,9 +238,10 @@ public:
// NOTE: This includes an implementation-defined optimization, see note above!
if (property_key.is_string() || property_key.is_number()) {
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
// b. If numericIndex is not undefined, return ! IsValidIntegerIndex(O, numericIndex).
if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value())
return is_valid_integer_index(*this, *numeric_index);
if (!numeric_index.is_undefined())
return is_valid_integer_index(*this, numeric_index);
}
// 4. Return ? OrdinaryHasProperty(O, P).
@ -255,10 +264,11 @@ public:
// NOTE: This includes an implementation-defined optimization, see note above!
if (property_key.is_string() || property_key.is_number()) {
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
// b. If numericIndex is not undefined, then
if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
if (!numeric_index.is_undefined()) {
// i. If ! IsValidIntegerIndex(O, numericIndex) is false, return false.
if (!is_valid_integer_index(*this, *numeric_index))
if (!is_valid_integer_index(*this, numeric_index))
return false;
// ii. If Desc has a [[Configurable]] field and if Desc.[[Configurable]] is false, return false.
@ -279,7 +289,7 @@ public:
// vi. If Desc has a [[Value]] field, perform ? IntegerIndexedElementSet(O, numericIndex, Desc.[[Value]]).
if (property_descriptor.value.has_value())
TRY(integer_indexed_element_set<T>(*this, *numeric_index, *property_descriptor.value));
TRY(integer_indexed_element_set<T>(*this, numeric_index, *property_descriptor.value));
// vii. Return true.
return true;
@ -305,10 +315,11 @@ public:
// NOTE: This includes an implementation-defined optimization, see note above!
if (property_key.is_string() || property_key.is_number()) {
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
// b. If numericIndex is not undefined, then
if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
if (!numeric_index.is_undefined()) {
// i. Return ! IntegerIndexedElementGet(O, numericIndex).
return integer_indexed_element_get<T>(*this, *numeric_index);
return integer_indexed_element_get<T>(*this, numeric_index);
}
}
@ -332,10 +343,11 @@ public:
// NOTE: This includes an implementation-defined optimization, see note above!
if (property_key.is_string() || property_key.is_number()) {
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
// b. If numericIndex is not undefined, then
if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
if (!numeric_index.is_undefined()) {
// i. Perform ? IntegerIndexedElementSet(O, numericIndex, V).
TRY(integer_indexed_element_set<T>(*this, *numeric_index, value));
TRY(integer_indexed_element_set<T>(*this, numeric_index, value));
// ii. Return true.
return true;
@ -361,10 +373,11 @@ public:
// NOTE: This includes an implementation-defined optimization, see note above!
if (property_key.is_string() || property_key.is_number()) {
// a. Let numericIndex be ! CanonicalNumericIndexString(P).
auto numeric_index = canonical_numeric_index_string(global_object(), property_key);
// b. If numericIndex is not undefined, then
if (auto numeric_index = canonical_numeric_index_string(property_key); numeric_index.has_value()) {
if (!numeric_index.is_undefined()) {
// i. If ! IsValidIntegerIndex(O, numericIndex) is false, return true; else return false.
if (!is_valid_integer_index(*this, *numeric_index))
if (!is_valid_integer_index(*this, numeric_index))
return true;
return false;
}

1
Userland/Libraries/LibJS/Runtime/Value.h
View file

@ -54,7 +54,6 @@ public:
bool is_undefined() const { return m_type == Type::Undefined; }
bool is_null() const { return m_type == Type::Null; }
bool is_number() const { return m_type == Type::Int32 || m_type == Type::Double; }
bool is_int32() const { return m_type == Type::Int32; }
bool is_string() const { return m_type == Type::String; }
bool is_object() const { return m_type == Type::Object; }
bool is_boolean() const { return m_type == Type::Boolean; }

36
Userland/Libraries/LibWeb/Bindings/IDLAbstractOperations.cpp
View file

@ -17,25 +17,39 @@
namespace Web::Bindings::IDL {
// https://webidl.spec.whatwg.org/#is-an-array-index
bool is_an_array_index(JS::PropertyKey const& property_name)
bool is_an_array_index(JS::GlobalObject& global_object, JS::PropertyKey const& property_name)
{
// 1. If Type(P) is not String, then return false.
if (!property_name.is_number())
return false;
// 2. Let index be ! CanonicalNumericIndexString(P).
auto index = JS::canonical_numeric_index_string(property_name);
auto index = JS::canonical_numeric_index_string(global_object, property_name);
if (!index.has_value()) {
// All of these are handled by canonical_numeric_index_string
// 3. If index is undefined, then return false.
// 4. If IsInteger(index) is false, then return false.
// 5. If index is −0, then return false.
// 6. If index < 0, then return false.
// 7. If index ≥ 2 ** 32 − 1, then return false.
// Note: 2 ** 32 − 1 is the maximum array length allowed by ECMAScript.
// 3. If index is undefined, then return false.
if (index.is_undefined())
return false;
// 4. If IsInteger(index) is false, then return false.
// NOTE: IsInteger is the old name of IsIntegralNumber.
if (!index.is_integral_number())
return false;
// 5. If index is −0, then return false.
if (index.is_negative_zero())
return false;
// FIXME: I'm not sure if this is correct.
auto index_as_double = index.as_double();
// 6. If index < 0, then return false.
if (index_as_double < 0)
return false;
// 7. If index ≥ 2 ** 32 − 1, then return false.
// Note: 2 ** 32 − 1 is the maximum array length allowed by ECMAScript.
if (index_as_double >= NumericLimits<u32>::max())
return false;
}
// 8. Return true.
return true;

2
Userland/Libraries/LibWeb/Bindings/IDLAbstractOperations.h
View file

@ -16,7 +16,7 @@
namespace Web::Bindings::IDL {
bool is_an_array_index(JS::PropertyKey const&);
bool is_an_array_index(JS::GlobalObject&, JS::PropertyKey const&);
Optional<ByteBuffer> get_buffer_source_copy(JS::Object const& buffer_source);
// https://webidl.spec.whatwg.org/#call-user-object-operation-return