[webkit-reviews] review denied: [Bug 67095] Add support for checked arithmetic : [Attachment 105449] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Oliver Hunt <oliver at apple.com>'s request
for review:
Bug 67095: Add support for checked arithmetic
https://bugs.webkit.org/show_bug.cgi?id=67095

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105449&action=review


It would be awesome if this had tests in TestWebKitAPI.

> Source/JavaScriptCore/wtf/CheckedArithmetic.h:90
> +    void overflowed()
> +    {
> +	   m_overflowed = true;
> +    }

I think this would read cleaner if it had the word set in it.

> Source/JavaScriptCore/wtf/CheckedArithmetic.h:101
> +    unsigned char m_overflowed;

This is quite odd that it is an unsigned char here and not a bool.  If you need
this for some reason, it should be well documented.