Commit 277f5bd0 authored by littledan's avatar littledan Committed by Commit bot

Further ES2015 RegExp spec compliance fixes

- RegExp.prototype.toString() doesn't have any special handling of
  RegExp instances and simply calls the source and flags getters
- Use the original values of global and sticky, rather than based
  on the current flag getters, as specified in
  https://github.com/tc39/ecma262/pull/494

R=yangguo@chromium.org,adamk
LOG=Y
BUG=v8:4602

Review URL: https://codereview.chromium.org/1846303002

Cr-Commit-Position: refs/heads/master@{#35225}
parent 42f2261c
...@@ -194,8 +194,8 @@ function RegExpSubclassExecJS(string) { ...@@ -194,8 +194,8 @@ function RegExpSubclassExecJS(string) {
// algorithm, step 4) even if the value is discarded for non-global RegExps. // algorithm, step 4) even if the value is discarded for non-global RegExps.
var i = TO_LENGTH(lastIndex); var i = TO_LENGTH(lastIndex);
var global = TO_BOOLEAN(this.global); var global = TO_BOOLEAN(REGEXP_GLOBAL(this));
var sticky = TO_BOOLEAN(this.sticky); var sticky = TO_BOOLEAN(REGEXP_STICKY(this));
var updateLastIndex = global || sticky; var updateLastIndex = global || sticky;
if (updateLastIndex) { if (updateLastIndex) {
if (i > string.length) { if (i > string.length) {
...@@ -370,27 +370,14 @@ function TrimRegExp(regexp) { ...@@ -370,27 +370,14 @@ function TrimRegExp(regexp) {
function RegExpToString() { function RegExpToString() {
if (!IS_REGEXP(this)) { if (!IS_RECEIVER(this)) {
// RegExp.prototype.toString() returns '/(?:)/' as a compatibility fix; throw MakeTypeError(
// a UseCounter is incremented to track it. kIncompatibleMethodReceiver, 'RegExp.prototype.toString', this);
// TODO(littledan): Remove this workaround or standardize it
if (this === GlobalRegExpPrototype) {
%IncrementUseCounter(kRegExpPrototypeToString);
return '/(?:)/';
}
if (!IS_RECEIVER(this)) {
throw MakeTypeError(
kIncompatibleMethodReceiver, 'RegExp.prototype.toString', this);
}
return '/' + TO_STRING(this.source) + '/' + TO_STRING(this.flags);
} }
var result = '/' + REGEXP_SOURCE(this) + '/'; if (this === GlobalRegExpPrototype) {
if (REGEXP_GLOBAL(this)) result += 'g'; %IncrementUseCounter(kRegExpPrototypeToString);
if (REGEXP_IGNORE_CASE(this)) result += 'i'; }
if (REGEXP_MULTILINE(this)) result += 'm'; return '/' + TO_STRING(this.source) + '/' + TO_STRING(this.flags);
if (REGEXP_UNICODE(this)) result += 'u';
if (REGEXP_STICKY(this)) result += 'y';
return result;
} }
...@@ -1126,7 +1113,7 @@ function RegExpGetSource() { ...@@ -1126,7 +1113,7 @@ function RegExpGetSource() {
// TODO(littledan): Remove this RegExp compat workaround // TODO(littledan): Remove this RegExp compat workaround
if (this === GlobalRegExpPrototype) { if (this === GlobalRegExpPrototype) {
%IncrementUseCounter(kRegExpPrototypeSourceGetter); %IncrementUseCounter(kRegExpPrototypeSourceGetter);
return UNDEFINED; return "(?:)";
} }
throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.source"); throw MakeTypeError(kRegExpNonRegExp, "RegExp.prototype.source");
} }
......
...@@ -1949,7 +1949,7 @@ TEST(UseCountRegExp) { ...@@ -1949,7 +1949,7 @@ TEST(UseCountRegExp) {
// a UseCounter is incremented to track it. // a UseCounter is incremented to track it.
v8::Local<v8::Value> resultToString = v8::Local<v8::Value> resultToString =
CompileRun("RegExp.prototype.toString().length"); CompileRun("RegExp.prototype.toString().length");
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]); CHECK_EQ(2, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]);
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]); CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]);
CHECK(resultToString->IsInt32()); CHECK(resultToString->IsInt32());
CHECK_EQ(6, CHECK_EQ(6,
...@@ -1957,7 +1957,7 @@ TEST(UseCountRegExp) { ...@@ -1957,7 +1957,7 @@ TEST(UseCountRegExp) {
// .toString() works on normal RegExps // .toString() works on normal RegExps
v8::Local<v8::Value> resultReToString = CompileRun("/a/.toString().length"); v8::Local<v8::Value> resultReToString = CompileRun("/a/.toString().length");
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]); CHECK_EQ(2, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]);
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]); CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]);
CHECK(resultReToString->IsInt32()); CHECK(resultReToString->IsInt32());
CHECK_EQ( CHECK_EQ(
...@@ -1969,7 +1969,7 @@ TEST(UseCountRegExp) { ...@@ -1969,7 +1969,7 @@ TEST(UseCountRegExp) {
"try { RegExp.prototype.toString.call(null) }" "try { RegExp.prototype.toString.call(null) }"
"catch (e) { exception = e; }" "catch (e) { exception = e; }"
"exception"); "exception");
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]); CHECK_EQ(2, use_counts[v8::Isolate::kRegExpPrototypeStickyGetter]);
CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]); CHECK_EQ(1, use_counts[v8::Isolate::kRegExpPrototypeToString]);
CHECK(resultToStringError->IsObject()); CHECK(resultToStringError->IsObject());
} }
...@@ -919,7 +919,13 @@ function TestMapSetSubclassing(container, is_map) { ...@@ -919,7 +919,13 @@ function TestMapSetSubclassing(container, is_map) {
var o = Reflect.construct(RegExp, [pattern], f); var o = Reflect.construct(RegExp, [pattern], f);
assertEquals(["match", "tostring"], log); assertEquals(["match", "tostring"], log);
assertEquals(/biep/, o); // TODO(littledan): Is the RegExp constructor correct to create
// the internal slots and do these type checks this way?
assertEquals("biep", %_RegExpSource(o));
assertThrows(() => Object.getOwnPropertyDescriptor(RegExp.prototype,
'source').get(o),
TypeError);
assertEquals("/undefined/undefined", RegExp.prototype.toString.call(o));
assertTrue(o.__proto__ === p2); assertTrue(o.__proto__ === p2);
assertTrue(f.prototype === p3); assertTrue(f.prototype === p3);
})(); })();
...@@ -21,32 +21,47 @@ function should_not_be_called() { ...@@ -21,32 +21,47 @@ function should_not_be_called() {
})(); })();
(function() { (function() {
let allow = false;
class A extends RegExp { class A extends RegExp {
get source() { throw new Error("should not be called") } get source() {
get flags() { throw new Error("should not be called") } if (!allow) throw new Error("should not be called");
return super.source;
}
get flags() {
if (!allow) throw new Error("should not be called");
return super.flags
}
} }
var r = new A("biep"); var r = new A("biep");
var r2 = RegExp(r); var r2 = RegExp(r);
assertFalse(r === r2); assertFalse(r === r2);
allow = true;
assertEquals(r, r2); assertEquals(r, r2);
allow = false;
assertTrue(A.prototype === r.__proto__); assertTrue(A.prototype === r.__proto__);
assertTrue(RegExp.prototype === r2.__proto__); assertTrue(RegExp.prototype === r2.__proto__);
var r3 = RegExp(r); var r3 = RegExp(r);
assertFalse(r3 === r); assertFalse(r3 === r);
allow = true;
assertEquals(r3, r); assertEquals(r3, r);
allow = false;
var r4 = new A(r2); var r4 = new A(r2);
assertFalse(r4 === r2); assertFalse(r4 === r2);
allow = true;
assertEquals(r4, r2); assertEquals(r4, r2);
allow = false;
assertTrue(A.prototype === r4.__proto__); assertTrue(A.prototype === r4.__proto__);
r[Symbol.match] = false; r[Symbol.match] = false;
var r5 = new A(r); var r5 = new A(r);
assertFalse(r5 === r); assertFalse(r5 === r);
allow = true;
assertEquals(r5, r); assertEquals(r5, r);
allow = false;
assertTrue(A.prototype === r5.__proto__); assertTrue(A.prototype === r5.__proto__);
})(); })();
......
...@@ -50,9 +50,10 @@ assertEquals(4, get_count); ...@@ -50,9 +50,10 @@ assertEquals(4, get_count);
function testName(name) { function testName(name) {
// TODO(littledan): For web compatibility, we don't throw an exception, // Test for ES2017 RegExp web compatibility semantics
// but ES2015 expects an exception to be thrown from this getter. // https://github.com/tc39/ecma262/pull/511
assertEquals(undefined, RegExp.prototype[name]); assertEquals(name === "source" ? "(?:)" : undefined,
RegExp.prototype[name]);
assertEquals( assertEquals(
"get " + name, "get " + name,
Object.getOwnPropertyDescriptor(RegExp.prototype, name).get.name); Object.getOwnPropertyDescriptor(RegExp.prototype, name).get.name);
......
...@@ -44,3 +44,14 @@ testThrows(1); ...@@ -44,3 +44,14 @@ testThrows(1);
assertEquals("/pattern/flags", RegExp.prototype.toString.call(fake)); assertEquals("/pattern/flags", RegExp.prototype.toString.call(fake));
assertEquals(["p", "ps", "f", "fs"], log); assertEquals(["p", "ps", "f", "fs"], log);
// Monkey-patching is also possible on RegExp instances
let weird = /foo/;
Object.defineProperty(weird, 'flags', {value: 'bar'});
Object.defineProperty(weird, 'source', {value: 'baz'});
assertEquals('/baz/bar', weird.toString());
assertEquals('/(?:)/', RegExp.prototype.toString());
assertEquals('(?:)', RegExp.prototype.source);
assertEquals('', RegExp.prototype.flags);
...@@ -100,20 +100,32 @@ ...@@ -100,20 +100,32 @@
'language/asi/S7.9_A5.7_T1': [PASS, FAIL_OK], 'language/asi/S7.9_A5.7_T1': [PASS, FAIL_OK],
###### BEGIN REGEXP SUBCLASSING SECTION ###### ###### BEGIN REGEXP SUBCLASSING SECTION ######
# https://code.google.com/p/v8/issues/detail?id=4602 # Spec change in progress https://github.com/tc39/ecma262/pull/494
# Fails due to mismatching [[OriginalFlags]] and getters # RegExpBuiltinMatch reads flags from [[OriginalFlags]]
'built-ins/RegExp/prototype/exec/get-sticky-coerce': [FAIL], 'built-ins/RegExp/prototype/Symbol.match/builtin-coerce-sticky': [FAIL],
'built-ins/RegExp/prototype/Symbol.match/builtin-get-global-err': [FAIL],
'built-ins/RegExp/prototype/Symbol.match/builtin-get-sticky-err': [FAIL],
'built-ins/RegExp/prototype/Symbol.match/builtin-success-g-set-lastindex': [FAIL],
'built-ins/RegExp/prototype/Symbol.match/builtin-success-g-set-lastindex-err': [FAIL],
'built-ins/RegExp/prototype/Symbol.match/coerce-sticky': [FAIL], 'built-ins/RegExp/prototype/Symbol.match/coerce-sticky': [FAIL],
'built-ins/RegExp/prototype/Symbol.replace/get-sticky-coerce': [FAIL], 'built-ins/RegExp/prototype/Symbol.match/get-sticky-err': [FAIL],
'built-ins/RegExp/prototype/Symbol.replace/coerce-global': [FAIL], 'built-ins/RegExp/prototype/Symbol.replace/coerce-global': [FAIL],
'built-ins/RegExp/prototype/Symbol.replace/coerce-unicode': [FAIL], 'built-ins/RegExp/prototype/Symbol.replace/coerce-unicode': [FAIL],
'built-ins/RegExp/prototype/Symbol.replace/get-sticky-coerce': [FAIL],
'built-ins/RegExp/prototype/Symbol.replace/get-sticky-err': [SKIP],
'built-ins/RegExp/prototype/Symbol.search/get-sticky-coerce': [FAIL], 'built-ins/RegExp/prototype/Symbol.search/get-sticky-coerce': [FAIL],
'built-ins/RegExp/prototype/Symbol.search/get-sticky-err': [FAIL],
'built-ins/RegExp/prototype/exec/get-sticky-coerce': [FAIL],
'built-ins/RegExp/prototype/exec/get-sticky-err': [FAIL],
'built-ins/RegExp/prototype/test/get-sticky-err': [FAIL],
# Missing lastIndex support # Missing lastIndex support
'built-ins/RegExp/prototype/Symbol.split/str-result-coerce-length-err': [FAIL], 'built-ins/RegExp/prototype/Symbol.split/str-result-coerce-length-err': [FAIL],
# Times out # Times out
'built-ins/RegExp/prototype/Symbol.split/str-coerce-lastindex': [SKIP], 'built-ins/RegExp/prototype/Symbol.split/str-coerce-lastindex': [SKIP],
'built-ins/RegExp/prototype/Symbol.match/coerce-global': [SKIP],
'built-ins/RegExp/prototype/Symbol.match/builtin-coerce-global': [SKIP],
# Sticky support busted # Sticky support busted
'built-ins/RegExp/prototype/Symbol.replace/y-init-lastindex': [FAIL], 'built-ins/RegExp/prototype/Symbol.replace/y-init-lastindex': [FAIL],
...@@ -123,9 +135,6 @@ ...@@ -123,9 +135,6 @@
# happens to be thrown for some other reason. # happens to be thrown for some other reason.
'built-ins/RegExp/prototype/Symbol.split/str-result-get-length-err': [SKIP], 'built-ins/RegExp/prototype/Symbol.split/str-result-get-length-err': [SKIP],
# Spec change in progress: https://github.com/tc39/ecma262/issues/489
'built-ins/RegExp/prototype/Symbol.replace/get-sticky-err': [SKIP],
#
###### END REGEXP SUBCLASSING SECTION ###### ###### END REGEXP SUBCLASSING SECTION ######
# https://code.google.com/p/v8/issues/detail?id=4360 # https://code.google.com/p/v8/issues/detail?id=4360
......
...@@ -28,7 +28,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE ...@@ -28,7 +28,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
PASS RegExp('/').source is "\\/" PASS RegExp('/').source is "\\/"
PASS RegExp('').source is "(?:)" PASS RegExp('').source is "(?:)"
FAIL RegExp.prototype.source should be (?:) (of type string). Was undefined (of type undefined). PASS RegExp.prototype.source is "(?:)"
PASS RegExp('/').toString() is "/\\//" PASS RegExp('/').toString() is "/\\//"
PASS RegExp('').toString() is "/(?:)/" PASS RegExp('').toString() is "/(?:)/"
PASS RegExp.prototype.toString() is "/(?:)/" PASS RegExp.prototype.toString() is "/(?:)/"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment