[webkit-reviews] review granted: [Bug 223783] [JSC] Use new Apple ICU APIs to avoid C++ ICU API usage : [Attachment 424339] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 26 08:55:37 PDT 2021

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 223783: [JSC] Use new Apple ICU APIs to avoid C++ ICU API usage

Attachment 424339: Patch


--- Comment #9 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 424339
  --> https://bugs.webkit.org/attachment.cgi?id=424339

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

r=me.  Please make sure that the mac-AS-debug-wk2 bot failure is not real.

> Source/JavaScriptCore/runtime/JSDateMath.cpp:133
> +    // The above can fail if input date is invalid: NaN etc.
> +    // We can return any values in this case since later we fail when
computing non timezone offset part anyway.
> +    constexpr LocalTimeOffset failed { false, 0 };

"The above" sounds wrong since there's no code that has any effect above.  How
about changing it to "This function"?

This failed return value also seem to contradict the comment above the function
which states that "If this function is called with NaN it returns NaN."  Maybe
remove the comment above the function?

More information about the webkit-reviews mailing list