[Webkit-unassigned] [Bug 131704] Simple ES6 feature:String prototype additions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=131704


Geoffrey Garen <ggaren at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #236762|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #9 from Geoffrey Garen <ggaren at apple.com>  2014-08-18 11:30:43 PST ---
(From update of attachment 236762)
View in context: https://bugs.webkit.org/attachment.cgi?id=236762&action=review

Looks pretty good, but I think I see some bugs.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:1561
> +    if (a0.isObject() && a0.inherits(RegExpObject::info()))

jsDynamicCast<RegExpObject> is the best way to do this.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:1564
> +    String str = exec->thisValue().toString(exec)->value(exec);

You need to do some kind of type check on exec->thisValue(), since it can be anything, including an environment record (JSActivation), which you should not use directly. Perhaps the specification calls for something like "checkObjectCoercible"?

You should add a test for the case where this function is called with an environment record as 'this'. You can do so by doing something like "(function() { var f = String.prototype.startsWith; (function() { startsWith("a"); })(); })()".

> Source/JavaScriptCore/runtime/StringPrototype.cpp:1582
> +        if (str.characterAt(i) != searchStr.characterAt(j))

You should use bracket access instead, as the WTFString.h header indicates:

    // Workaround for a compiler bug. Use operator[] instead.
    UChar characterAt(unsigned index) const

> Source/JavaScriptCore/runtime/StringPrototype.cpp:1588
> +EncodedJSValue JSC_HOST_CALL stringProtoFuncEndsWith(ExecState* exec)

Same comments as above.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:1623
> +EncodedJSValue JSC_HOST_CALL stringProtoFuncContains(ExecState* exec)

Ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.