[regexp] Add UseCounter for matchAll with non-g RegExp

Per the July TC39 meeting consensus, we'd like to make the
upcoming String.prototype.replaceAll proposal throw for
non-global RegExp searchValues. However,
String.prototype.matchAll currently does not throw in this
case, causing consistency concerns.

This patch adds a use counter for String.prototype.matchAll
with a non-global RegExp as the searchValue. Hopefully, this
pattern isn't too common in real-world code today, in which case
we can both a) change matchAll and b) proceed with the desired
replaceAll semantics.

https://github.com/tc39/proposal-string-replaceall/issues/16

V8 CL: https://chromium-review.googlesource.com/c/v8/v8/+/1718145
Chromium CL: https://chromium-review.googlesource.com/c/chromium/src/+/1718367

BUG=v8:9551

Change-Id: Ica660a0a6189d84c3d33398c98305d0bcb9f8c23
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1718145
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62913}
This commit is contained in:
Mathias Bynens 2019-07-25 13:30:36 +02:00 committed by Commit Bot
parent 61a3f827ee
commit dd7190a979
3 changed files with 28 additions and 4 deletions

View File

@ -7664,9 +7664,10 @@ class V8_EXPORT Isolate {
kStringNormalize = 75,
kCallSiteAPIGetFunctionSloppyCall = 76,
kCallSiteAPIGetThisSloppyCall = 77,
kRegExpMatchAllWithNonGlobalRegExp = 78,
// If you add new values here, you'll also need to update Chromium's:
// web_feature.mojom, UseCounterCallback.cpp, and enums.xml. V8 changes to
// web_feature.mojom, use_counter_callback.cc, and enums.xml. V8 changes to
// this list need to be landed first, then changes on the Chromium side.
kUseCounterFeatureCount // This enum value must be last.
};

View File

@ -2214,6 +2214,17 @@ void RegExpMatchAllAssembler::Generate(TNode<Context> context,
BIND(&create_iterator);
{
{
// UseCounter for matchAll with non-g RegExp.
// https://crbug.com/v8/9551
Label next(this);
GotoIf(var_global.value(), &next);
CallRuntime(Runtime::kIncrementUseCounter, context,
SmiConstant(v8::Isolate::kRegExpMatchAllWithNonGlobalRegExp));
Goto(&next);
BIND(&next);
}
// 13. Return ! CreateRegExpStringIterator(matcher, S, global, fullUnicode).
TNode<Object> iterator =
CreateRegExpStringIterator(native_context, var_matcher.value(), string,

View File

@ -1694,8 +1694,7 @@ void MockUseCounterCallback(v8::Isolate* isolate,
}
}
// Test that ES2015 RegExp compatibility fixes are in place, that they
// Test that ES2015+ RegExp compatibility fixes are in place, that they
// are not overly broad, and the appropriate UseCounters are incremented
TEST(UseCountRegExp) {
v8::Isolate* isolate = CcTest::isolate();
@ -1717,7 +1716,7 @@ TEST(UseCountRegExp) {
CHECK_EQ(0, use_counts[v8::Isolate::kRegExpPrototypeToString]);
CHECK(resultReSticky->IsFalse());
// When the getter is caleld on another object, throw an exception
// When the getter is called on another object, throw an exception
// and don't increment the UseCounter
v8::Local<v8::Value> resultStickyError = CompileRun(
"var exception;"
@ -1759,6 +1758,19 @@ TEST(UseCountRegExp) {
CHECK_EQ(2, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]);
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]);
CHECK(resultToStringError->IsObject());
// Increment a UseCounter when .matchAll() is used with a non-global
// regular expression.
CHECK_EQ(0, use_counts[v8::Isolate::kRegExpMatchAllWithNonGlobalRegExp]);
v8::Local<v8::Value> resultReMatchAllNonGlobal =
CompileRun("'a'.matchAll(/./)");
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpMatchAllWithNonGlobalRegExp]);
CHECK(resultReMatchAllNonGlobal->IsObject());
// Don't increment the counter for global regular expressions.
v8::Local<v8::Value> resultReMatchAllGlobal =
CompileRun("'a'.matchAll(/./g)");
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpMatchAllWithNonGlobalRegExp]);
CHECK(resultReMatchAllGlobal->IsObject());
}
class UncachedExternalString