Bug 198471

Summary:

Finish cleanup of String::number for floating point

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

Web Template Framework

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, sam, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

178319

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none

Darin Adler

Reported 2019-06-02 17:17:07 PDT

Finish cleanup of String::number for floating point

Attachments
Patch (81.02 KB, patch) 2019-06-02 17:42 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Patch (82.11 KB, patch) 2019-06-02 17:49 PDT, Darin Adler	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2019-06-02 17:42:53 PDT Comment hidden (obsolete)

Created attachment 371172 [details] Patch

Darin Adler

Comment 2 2019-06-02 17:46:45 PDT Comment hidden (obsolete)

Comment on attachment 371172 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371172&action=review > Source/WTF/wtf/text/WTFString.h:132 > + // We have verified empirically that we need to, but it would be good understand exactly why. Oops, missing word here in my edit to this comment. In my local patch I have now deleted the entire FIXME comment instead and that’s what I’ll land unless I get some feedback to the contrary during review.

Darin Adler

Comment 3 2019-06-02 17:49:29 PDT

Created attachment 371173 [details] Patch

EWS Watchlist

Comment 4 2019-06-02 19:41:04 PDT

Comment on attachment 371173 [details] Patch Attachment 371173 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/12359319 New failing tests: mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases apiTests

Darin Adler

Comment 5 2019-06-02 21:33:13 PDT

(In reply to Build Bot from comment #4) > Comment on attachment 371173 [details] > mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit- > validate-phases > apiTests Seems obvious from reading the error messages that this is not a real failure. Just a slow run on the test bot.

Yusuke Suzuki

Comment 6 2019-06-03 04:03:58 PDT

Comment on attachment 371173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371173&action=review r=me > Source/WebCore/rendering/RenderListMarker.cpp:139 > + // The asterisks list-style-type is the worst case; we show |number| asterisks. > + CharacterType symbol = symbols[(number - 1) % symbolsSize]; > + unsigned count = (number - 1) / symbolsSize + 1; > + > + CharacterType* characters; > + String result = String::createUninitialized(count, characters); > + for (unsigned i = 0; i < count; ++i) > + characters[i] = symbol; > + return result; Nice! The code becomes much cleaner.

WebKit Commit Bot

Comment 7 2019-06-03 04:34:26 PDT

Comment on attachment 371173 [details] Patch Clearing flags on attachment: 371173 Committed r246034: <https://trac.webkit.org/changeset/246034>

WebKit Commit Bot

Comment 8 2019-06-03 04:34:28 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 9 2019-06-03 04:35:16 PDT

<rdar://problem/51353064>

Note You need to log in before you can comment on or make changes to this bug.