[webkit-reviews] review canceled: [Bug 224797] [clang] Remove implicit cast related warnings. : [Attachment 426513] PATCH

[bugzilla-daemon at webkit.org]

Basuke Suzuki <Basuke.Suzuki at sony.com> has canceled Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 224797: [clang] Remove implicit cast related warnings.
https://bugs.webkit.org/show_bug.cgi?id=224797

Attachment 426513: PATCH

https://bugs.webkit.org/attachment.cgi?id=426513&action=review




--- Comment #4 from Basuke Suzuki <Basuke.Suzuki at sony.com> ---
Comment on attachment 426513
  --> https://bugs.webkit.org/attachment.cgi?id=426513
PATCH

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

>> Source/WTF/wtf/CheckedArithmetic.h:820
>> +	    long double result = rhs * m_value;
> 
> This looks like a costly solution to this problem, and possibly not portable.
Is there a way to do this checking without involving a "long double".

As the warning says: "'unsigned long') to 'double' changes value from
18446744073709551615 to 18446744073709551616", double is not as precise as to
hold unsigned long. What if the value is 18446744073709551616, then the value
will pass through this check and won't overflow. I cannot judge what range of
error can be allowed from this generic implementation, so that it has to be
strict. But I didn't think about the portability. Humm, I don't have other
solution right now. Let's think about this.

>> Source/WebCore/ChangeLog:10
>> +	    No new tests because there's no practical change.
> 
> I don’t understand what "compatibility" means here. Compatibility with what?

I mean it it compatible with the result of implicit cast with explicit cast.
That's why technically there's no change in the compilation result.