[webkit-reviews] review granted: [Bug 226514] Rename Checked::unsafeGet() to Checked::value() : [Attachment 430306] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 226514: Rename Checked::unsafeGet() to Checked::value()
https://bugs.webkit.org/show_bug.cgi?id=226514

Attachment 430306: Patch

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




--- Comment #15 from Darin Adler <darin at apple.com> ---
Comment on attachment 430306
  --> https://bugs.webkit.org/attachment.cgi?id=430306
Patch

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

Now that you added operator T, presumably many of these value() calls could
have been left out; but I noticed that some you left in and others you removed.
How did you decide where to use value()? Is it only used where it’s needed to
compile, or was there some other rule?

> Source/JavaScriptCore/runtime/StringPrototype.cpp:315
> +    ASSERT(totalLength <= static_cast<int>(String::MaxLength));

Can we use Checked<uint32_t, AssertNoOverflow> throughout this function instead
of Checked<int, AssertNoOverflow>?

Also, what’s the point of using AssertNoOverflow instead of RecordOverflow? Is
that for better efficiency?