[webkit-reviews] review denied: [Bug 80009] [Forms] Introduce Decimal arithmetic to fix rounding errors in number/range input types : [Attachment 143753] Patch 9

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 80009: [Forms] Introduce Decimal arithmetic to fix rounding errors in
number/range input types
https://bugs.webkit.org/show_bug.cgi?id=80009

Attachment 143753: Patch 9
https://bugs.webkit.org/attachment.cgi?id=143753&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143753&action=review


I agree with this strategy, but the patch is too large. 197KB code change in
one patch is not acceptable.

Please make another patch with only Decimal.h, Decimal.cpp, and DecimalTest.cpp
as the first step.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:92
> +    // FIXME: UINT64_C(17976931348623159) should be used but it did not
compile on Qt bots.
> +#if COMPILER(MSVC)
> +    const uint64_t leadingDigitsOf2Power1024 = 17976931348623159ui64;
> +#else
> +    const uint64_t leadingDigitsOf2Power1024 = 17976931348623159ull;
> +#endif

You use this workaround many times.  We should fix this issue before this
patch.
I think we can add UINT64_C in wtf/MathExtras.h.

> LayoutTests/fast/forms/range/range-value-rounding-expected.txt:119
> +PASS createSample("0.000001", "0.000009", "1e-5").valueAsNumber is 0.000009
> +FAIL createSample("0.0000001", "0.0000001", "1e-6").value should be
0.0000001. Was 1e-7.
> +PASS createSample("0.0000001", "0.0000001", "1e-6").valueAsNumber is
0.0000001
> +FAIL createSample("0.0000001", "0.0000002", "1e-6").value should be
0.0000002. Was 2e-7.
> +PASS createSample("0.0000001", "0.0000002", "1e-6").valueAsNumber is
0.0000002
> +FAIL createSample("0.0000001", "0.0000003", "1e-6").value should be
0.0000003. Was 3e-7.

This has a lot of FAIL lines.