From 29a970a27319a7e1c193c026a817f037bb4ef71f Mon Sep 17 00:00:00 2001 From: peterwmwong Date: Tue, 11 Dec 2018 07:42:08 -0600 Subject: [PATCH] [esnext] Update String.p.matchAll as per spec changes As per (https://github.com/tc39/proposal-string-matchall/pull/41), String.p.matchAll's fallback was removed. Additionally, removed a IsNullOrUndefined check that was already covered by MaybeCallFunctionAtSymbol. Updates to Test262 has been submitted: https://github.com/tc39/test262/pull/1990 Bug: v8:6890 Change-Id: I246cbbcb4641ebded704c5f772809f182deaa30e Reviewed-on: https://chromium-review.googlesource.com/c/1369091 Commit-Queue: Peter Wong Reviewed-by: Sathya Gunasekaran Reviewed-by: Jakob Gruber Reviewed-by: Mathias Bynens Cr-Commit-Position: refs/heads/master@{#58172} --- src/builtins/builtins-string-gen.cc | 82 ++++++++----------- .../string-matchAll-deleted-matchAll.js | 9 ++ test/test262/test262.status | 4 + 3 files changed, 46 insertions(+), 49 deletions(-) create mode 100644 test/mjsunit/harmony/string-matchAll-deleted-matchAll.js diff --git a/src/builtins/builtins-string-gen.cc b/src/builtins/builtins-string-gen.cc index 9ca4d0502673..4d7c700dc403 100644 --- a/src/builtins/builtins-string-gen.cc +++ b/src/builtins/builtins-string-gen.cc @@ -1521,57 +1521,41 @@ TF_BUILTIN(StringPrototypeMatchAll, StringBuiltinsAssembler) { RequireObjectCoercible(context, receiver, method_name); // 2. If regexp is neither undefined nor null, then - Label tostring_and_create_regexp_string_iterator(this, Label::kDeferred); - TVARIABLE(String, var_receiver_string); - GotoIf(IsNullOrUndefined(maybe_regexp), - &tostring_and_create_regexp_string_iterator); - { - // a. Let matcher be ? GetMethod(regexp, @@matchAll). - // b. If matcher is not undefined, then - // i. Return ? Call(matcher, regexp, « O »). - auto if_regexp_call = [&] { - // MaybeCallFunctionAtSymbol guarantees fast path is chosen only if - // maybe_regexp is a fast regexp and receiver is a string. - var_receiver_string = CAST(receiver); - CSA_ASSERT(this, IsString(var_receiver_string.value())); - - RegExpMatchAllAssembler regexp_asm(state()); - regexp_asm.Generate(context, native_context, maybe_regexp, - var_receiver_string.value()); - }; - auto if_generic_call = [=](Node* fn) { - Callable call_callable = CodeFactory::Call(isolate()); - Return(CallJS(call_callable, context, fn, maybe_regexp, receiver)); - }; - MaybeCallFunctionAtSymbol( - context, maybe_regexp, receiver, - isolate()->factory()->match_all_symbol(), - DescriptorIndexAndName{JSRegExp::kSymbolMatchAllFunctionDescriptorIndex, - RootIndex::kmatch_all_symbol}, - if_regexp_call, if_generic_call); - Goto(&tostring_and_create_regexp_string_iterator); - } - BIND(&tostring_and_create_regexp_string_iterator); - { + // a. Let matcher be ? GetMethod(regexp, @@matchAll). + // b. If matcher is not undefined, then + // i. Return ? Call(matcher, regexp, « O »). + auto if_regexp_call = [&] { + // MaybeCallFunctionAtSymbol guarantees fast path is chosen only if + // maybe_regexp is a fast regexp and receiver is a string. + TNode s = CAST(receiver); + RegExpMatchAllAssembler regexp_asm(state()); + regexp_asm.Generate(context, native_context, maybe_regexp, s); + }; + auto if_generic_call = [=](Node* fn) { + Callable call_callable = CodeFactory::Call(isolate()); + Return(CallJS(call_callable, context, fn, maybe_regexp, receiver)); + }; + MaybeCallFunctionAtSymbol( + context, maybe_regexp, receiver, isolate()->factory()->match_all_symbol(), + DescriptorIndexAndName{JSRegExp::kSymbolMatchAllFunctionDescriptorIndex, + RootIndex::kmatch_all_symbol}, + if_regexp_call, if_generic_call); - // 3. Let S be ? ToString(O). - var_receiver_string = ToString_Inline(context, receiver); - - // 4. Let matcher be ? RegExpCreate(R, "g"). - TNode regexp = regexp_asm.RegExpCreate( - context, native_context, maybe_regexp, StringConstant("g")); - - // 5. Let global be true. - // 6. Let fullUnicode be false. - // 7. Return ! CreateRegExpStringIterator(matcher, S, global, fullUnicode). - TNode global = Int32Constant(1); - TNode full_unicode = Int32Constant(0); - TNode iterator = regexp_asm.CreateRegExpStringIterator( - native_context, regexp, var_receiver_string.value(), global, - full_unicode); - Return(iterator); - } + RegExpMatchAllAssembler regexp_asm(state()); + + // 3. Let S be ? ToString(O). + TNode s = ToString_Inline(context, receiver); + + // 4. Let rx be ? RegExpCreate(R, "g"). + TNode rx = regexp_asm.RegExpCreate(context, native_context, + maybe_regexp, StringConstant("g")); + + // 5. Return ? Invoke(rx, @@matchAll, « S »). + Callable callable = CodeFactory::Call(isolate()); + TNode match_all_func = + GetProperty(context, rx, isolate()->factory()->match_all_symbol()); + Return(CallJS(callable, context, match_all_func, rx, s)); } class StringPadAssembler : public StringBuiltinsAssembler { diff --git a/test/mjsunit/harmony/string-matchAll-deleted-matchAll.js b/test/mjsunit/harmony/string-matchAll-deleted-matchAll.js new file mode 100644 index 000000000000..5d2985f31803 --- /dev/null +++ b/test/mjsunit/harmony/string-matchAll-deleted-matchAll.js @@ -0,0 +1,9 @@ +// Copyright 2018 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --harmony-string-matchall + +delete RegExp.prototype[Symbol.matchAll]; +const str = 'a'; +assertThrows(() => str.matchAll(/\w/g), TypeError); diff --git a/test/test262/test262.status b/test/test262/test262.status index ecab026c989c..4f515c7ca454 100644 --- a/test/test262/test262.status +++ b/test/test262/test262.status @@ -734,6 +734,10 @@ 'harness/detachArrayBuffer': [SKIP], 'harness/detachArrayBuffer-host-detachArrayBuffer': [SKIP], + # Test needs to be updated to spec changes. + # https://github.com/tc39/test262/pull/1990 + 'built-ins/String/prototype/matchAll/regexp-prototype-has-no-matchAll': [FAIL], + ############################ SKIPPED TESTS ############################# # These tests take a looong time to run.