[Webkit-unassigned] [Bug 155795] [JSC] implement String.prototype.padStart() and String.prototype.padEnd() proposal

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #274904|review?                     |review-
              Flags|                            |

--- Comment #15 from Darin Adler <darin at apple.com> ---
Comment on attachment 274904
  --> https://bugs.webkit.org/attachment.cgi?id=274904
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=274904&action=review

> Source/JavaScriptCore/runtime/StringPrototype.cpp:801
> +        if (!ropeBuilder.append(string))
> +            return throwOutOfMemoryError(&exec), nullptr;

We should use a more conventional two line body here:

    if (!ropeBuilder.append(string)) {
        throwOutOfMemoryError(&exec);
        return nullptr;
    }

> Source/JavaScriptCore/runtime/StringPrototype.cpp:806
> +        if (!substr || !ropeBuilder.append(substr))
> +            return throwOutOfMemoryError(&exec), nullptr;

Ditto.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:861
> +enum class StringPaddingLocation {
> +    Start,
> +    End
> +};

I think this reads better on a single line.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:903
> +        UChar character = filler ? filler->view(&exec)[0] : ' ';

This should use the unsafeView function for better efficiency rather than the view function.

> Source/JavaScriptCore/runtime/StringPrototype.cpp:909
> +        filler = repeatStringPattern(exec, fillLength, filler, ropeBuilder);

I think you misunderstood my suggestion. I suggested passing the ropeBuilder in and create a function that would append to it, not so a function that will use it and then call "release". The point of the optimization is to not do a second release.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160325/01d05f52/attachment.html>