From 306d59276af3b92475fdafb418fbe62152b409c2 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Wed, 7 Jul 2021 19:15:52 +0300 Subject: [PATCH] LibJS: Stop using a native property for RegExp's lastIndex property This is not a functional change, the exposed (incorrect) behaviour is the same as it was before, this simply removes the last user of NativeProperties, allowing us to remove them completely from LibJS. --- Userland/Libraries/LibJS/AST.cpp | 2 +- .../LibJS/Runtime/RegExpConstructor.cpp | 16 +---- .../Libraries/LibJS/Runtime/RegExpObject.cpp | 61 +++++-------------- .../Libraries/LibJS/Runtime/RegExpObject.h | 3 - .../LibJS/Runtime/RegExpPrototype.cpp | 55 ++++++++++++++--- 5 files changed, 65 insertions(+), 72 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index f1e874c450..b0d95fbcaa 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -1993,7 +1993,7 @@ void RegExpLiteral::dump(int indent) const Value RegExpLiteral::execute(Interpreter& interpreter, GlobalObject& global_object) const { InterpreterNodeScope node_scope { interpreter, *this }; - return RegExpObject::create(global_object, pattern(), flags()); + return regexp_create(global_object, js_string(interpreter.heap(), pattern()), js_string(interpreter.heap(), flags())); } void ArrayExpression::dump(int indent) const diff --git a/Userland/Libraries/LibJS/Runtime/RegExpConstructor.cpp b/Userland/Libraries/LibJS/Runtime/RegExpConstructor.cpp index 534bc2227e..73ea609d88 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpConstructor.cpp @@ -43,20 +43,8 @@ Value RegExpConstructor::call() Value RegExpConstructor::construct(FunctionObject&) { auto& vm = this->vm(); - String pattern = ""; - String flags = ""; - if (!vm.argument(0).is_undefined()) { - pattern = vm.argument(0).to_string(global_object()); - if (vm.exception()) - return {}; - } - if (!vm.argument(1).is_undefined()) { - flags = vm.argument(1).to_string(global_object()); - if (vm.exception()) - return {}; - } - // FIXME: Use RegExpAlloc (which uses OrdinaryCreateFromConstructor) - return RegExpObject::create(global_object(), pattern, flags); + // FIXME: This is non-conforming + return regexp_create(global_object(), vm.argument(0), vm.argument(1)); } // 22.2.4.2 get RegExp [ @@species ], https://tc39.es/ecma262/#sec-get-regexp-@@species diff --git a/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp b/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp index 608ad3950d..749f477c2c 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp @@ -100,75 +100,42 @@ RegExpObject::RegExpObject(String pattern, String flags, Object& prototype) } } -void RegExpObject::initialize(GlobalObject& global_object) -{ - auto& vm = this->vm(); - Object::initialize(global_object); - - define_native_property(vm.names.lastIndex, last_index, set_last_index, Attribute::Writable); -} - RegExpObject::~RegExpObject() { } -static RegExpObject* regexp_object_from(VM& vm, GlobalObject& global_object) +void RegExpObject::initialize(GlobalObject& global_object) { - auto* this_object = vm.this_value(global_object).to_object(global_object); - if (!this_object) - return nullptr; - if (!is(this_object)) { - vm.throw_exception(global_object, ErrorType::NotA, "RegExp"); - return nullptr; - } - return static_cast(this_object); -} - -JS_DEFINE_NATIVE_GETTER(RegExpObject::last_index) -{ - auto regexp_object = regexp_object_from(vm, global_object); - if (!regexp_object) - return {}; - - return Value((unsigned)regexp_object->regex().start_offset); -} - -JS_DEFINE_NATIVE_SETTER(RegExpObject::set_last_index) -{ - auto regexp_object = regexp_object_from(vm, global_object); - if (!regexp_object) - return; - - auto index = value.to_i32(global_object); - if (vm.exception()) - return; - - if (index < 0) - index = 0; - - regexp_object->regex().start_offset = index; + auto& vm = this->vm(); + Object::initialize(global_object); + define_direct_property(vm.names.lastIndex, {}, Attribute::Writable); } // 22.2.3.2.4 RegExpCreate ( P, F ), https://tc39.es/ecma262/#sec-regexpcreate RegExpObject* regexp_create(GlobalObject& global_object, Value pattern, Value flags) { + auto& vm = global_object.vm(); String p; if (pattern.is_undefined()) { p = String::empty(); } else { p = pattern.to_string(global_object); - if (p.is_null()) - return nullptr; + if (vm.exception()) + return {}; } String f; if (flags.is_undefined()) { f = String::empty(); } else { f = flags.to_string(global_object); - if (f.is_null()) - return nullptr; + if (vm.exception()) + return {}; } - return RegExpObject::create(global_object, move(p), move(f)); + auto* object = RegExpObject::create(global_object, move(p), move(f)); + object->set(vm.names.lastIndex, Value(0), true); + if (vm.exception()) + return {}; + return object; } } diff --git a/Userland/Libraries/LibJS/Runtime/RegExpObject.h b/Userland/Libraries/LibJS/Runtime/RegExpObject.h index 640c429067..4ee57d3764 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpObject.h +++ b/Userland/Libraries/LibJS/Runtime/RegExpObject.h @@ -36,9 +36,6 @@ public: const Regex& regex() const { return m_regex; } private: - JS_DECLARE_NATIVE_GETTER(last_index); - JS_DECLARE_NATIVE_SETTER(set_last_index); - String m_pattern; String m_flags; Flags m_active_flags; diff --git a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp index a391ee04ac..d566de2c26 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp @@ -165,10 +165,25 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::exec) StringView str_to_match = str; // RegExps without "global" and "sticky" always start at offset 0. - if (!regexp_object->regex().options().has_flag_set((ECMAScriptFlags)regex::AllFlags::Internal_Stateful)) - regexp_object->regex().start_offset = 0; + if (!regexp_object->regex().options().has_flag_set((ECMAScriptFlags)regex::AllFlags::Internal_Stateful)) { + regexp_object->set(vm.names.lastIndex, Value(0), true); + if (vm.exception()) + return {}; + } + + auto last_index = regexp_object->get(vm.names.lastIndex); + if (vm.exception()) + return {}; + regexp_object->regex().start_offset = last_index.to_length(global_object); + if (vm.exception()) + return {}; auto result = do_match(regexp_object->regex(), str_to_match); + + regexp_object->set(vm.names.lastIndex, Value(regexp_object->regex().start_offset), true); + if (vm.exception()) + return {}; + if (!result.success) return js_null(); @@ -217,10 +232,25 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::test) return {}; // RegExps without "global" and "sticky" always start at offset 0. - if (!regexp_object->regex().options().has_flag_set((ECMAScriptFlags)regex::AllFlags::Internal_Stateful)) - regexp_object->regex().start_offset = 0; + if (!regexp_object->regex().options().has_flag_set((ECMAScriptFlags)regex::AllFlags::Internal_Stateful)) { + regexp_object->set(vm.names.lastIndex, Value(0), true); + if (vm.exception()) + return {}; + } + + auto last_index = regexp_object->get(vm.names.lastIndex); + if (vm.exception()) + return {}; + regexp_object->regex().start_offset = last_index.to_length(global_object); + if (vm.exception()) + return {}; auto result = do_match(regexp_object->regex(), str); + + regexp_object->set(vm.names.lastIndex, Value(regexp_object->regex().start_offset), true); + if (vm.exception()) + return {}; + return Value(result.success); } @@ -301,8 +331,11 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::symbol_replace) return {}; bool global = global_value.to_boolean(); - if (global) - rx->regex().start_offset = 0; + if (global) { + rx->set(vm.names.lastIndex, Value(0), true); + if (vm.exception()) + return {}; + } // FIXME: Implement and use RegExpExec - https://tc39.es/ecma262/#sec-regexpexec auto* exec = Value(rx).get_method(global_object, vm.names.exec); @@ -336,7 +369,15 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::symbol_replace) if (match_str.is_empty()) { // FIXME: Implement AdvanceStringIndex to take Unicode code points into account - https://tc39.es/ecma262/#sec-advancestringindex // Once implemented, step (8a) of the @@replace algorithm must also be implemented. - rx->regex().start_offset += 1; + auto last_index = rx->get(vm.names.lastIndex); + if (vm.exception()) + return {}; + auto this_index = last_index.to_length(global_object); + if (vm.exception()) + return {}; + rx->set(vm.names.lastIndex, Value(this_index + 1), true); + if (vm.exception()) + return {}; } }