In r198698, for the purpose of enabling feature testing, I changed the RegExp.prototype flag property getters to always return undefined instead of throwing when "this" is not a RegExp object. However, for the purpose of feature testing, we only need to apply this change for the cases where the getters are called on the RegExp.prototype itself. If called with any other "this" values that is not a RegExp object, they should throw a TypeError as the ES6 spec expects.
Created attachment 274977 [details] proposed patch.
Comment on attachment 274977 [details] proposed patch. r=me
Comment on attachment 274977 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=274977&action=review > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:249 > + if (thisValue == exec->lexicalGlobalObject()->regExpPrototype()) > + return JSValue::encode(jsUndefined()); Actually, you should check inherits(RegExpPrototype::info()) instead. There's no reason to tie this to a global object.
I'm going to hold off on implementing this for now. This issue is being discussed at https://github.com/tc39/ecma262/issues/262?, and there doesn't appear to be an agreed on solution yet.
For the record, Chrome is only special casing RegExp.prototype and returning undefined for it (see https://github.com/v8/v8/blob/master/src/js/regexp.js#L1091). For everything else, it throws the ES6 spec'ed TypeError.
From a recent TC39 meeting, we were told that "the committee also agreed to Daniel Ehrenberg’s pull request #262 (https://github.com/tc39/ecma262/issues/262), slides. Daniel will write up the details. The end result is that we will no longer throw for “RegExp like” accesses to RegExp.prototype, e.g. RegExp.prototype.sticky. In the case of individual flags, we return undefined. For .flags, .source and .toString(), we return what we do now, “”, "(?:)" and /(?:)/“ respectively. RegExp.prototype however IS NOT an instance of RegExp."
<rdar://problem/25717387>
Created attachment 276403 [details] proposed patch.
Created attachment 276404 [details] proposed patch: rebased to ToT.
Comment on attachment 276404 [details] proposed patch: rebased to ToT. r=me.
Did you run benchmarks on this?
(In reply to comment #11) > Did you run benchmarks on this? No. But the only changes in this patch are on error paths. If these results in a perf regression, then we have a serious problem with code cache lines. If it's ok with you, I'll put UNLIKELY() on the "if (!thisValue.inherits(RegExpObject::info()))" checks, and just monitor the perf bots for regressions.
(In reply to comment #12) > (In reply to comment #11) > > Did you run benchmarks on this? > > No. But the only changes in this patch are on error paths. If these > results in a perf regression, then we have a serious problem with code cache > lines. > > If it's ok with you, I'll put UNLIKELY() on the "if > (!thisValue.inherits(RegExpObject::info()))" checks, and just monitor the > perf bots for regressions. Seems reasonable.
Thanks. Landed in r199545: <http://trac.webkit.org/r199545>.