Improve assertions in StringParsingBuffer
Created attachment 402838 [details]
Created attachment 402841 [details]
Comment on attachment 402841 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=402841&action=review
> + ASSERT(static_cast<std::ptrdiff_t>(end - characters) < std::numeric_limits<unsigned>::max());
Surprised that the static_cast on the left side is necessary. Surprised that a static_cast on the right side isn’t necessary to ensure we don’t get signed/unsigned comparison warnings.
> - constexpr void advanceBy(unsigned places)
> + constexpr void advanceBy(size_t places)
Not sure how I feel about this even though I suggested it. In general if we pass too large a type it will get chopped off. But if the value is too big for unsigned the code won’t work anyway. Not sure why this is more important than taking size_t in operator. Could imagine setting this to a super-large integer type and doing a RELEASE_ASSERT. Maybe we should just be leaving this unsigned? Issues don’t seem different here than in operator or the constructor.
Created attachment 402907 [details]
Comment on attachment 402907 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=402907&action=review
> + ASSERT(end - characters < std::numeric_limits<unsigned>::max());
Created attachment 402956 [details]
Committed r263620: <https://trac.webkit.org/changeset/263620>
All reviewed patches have been landed. Closing bug and clearing flags on attachment 402956 [details].