[webkit-reviews] review denied: [Bug 100576] Non-special escape character sequences cause JSC::Lexer::parseString to create 16 bit strings : [Attachment 171077] Patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 100576: Non-special escape character sequences cause
JSC::Lexer::parseString to create 16 bit strings
https://bugs.webkit.org/show_bug.cgi?id=100576

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

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=171077&action=review


Can you also add tests for characters < 32 and greater than 127

Essentially okay, but i'd like to see the refreshed patch before r+'ing

> Source/JavaScriptCore/parser/Lexer.cpp:654
> +	   return singleCharacterEscapeValuesForASCII[c-32];

Alas you're not using the FixedArray type (you can't because there's no nice
way to initialize it), so i would at the very least add
ASSERT(c - 32 < ARRAY_SIZE(singleCharacterEscapeValuesForASCII));

>
LayoutTests/fast/js/script-tests/normal-character-escapes-in-string-literals.js
:9
> +    shouldBe('eval(\'"' + escapedCharacter + '"\')', "expectedResult");

don't quote expectedResult here, it will make the test output much more sane.