Fix lexing escapes in string literal and minor refactor

[jevancc]

This Pull Request fixes/closes #1078 and some bugs mentioned in the discussion of this PR.

It changes the following:

• Minor refactor string literal lexer (for future use in lexing unicode escape in identifier name)
• Fix lexing octal escape sequence in string literal
• Fix behavior of zero escape sequence
• Fix \ followed by a line terminator sequence
• Fix \v
• Fix lexing \8 and \9 in strict mode.
• Add more tests

[codecov]

Codecov Report

Merging #1079 (17eb410) into master (5a5061c) will increase coverage by 0.04%.
The diff coverage is 61.37%.

@@            Coverage Diff             @@
##           master    #1079      +/-   ##
==========================================
+ Coverage   58.70%   58.74%   +0.04%     
==========================================
  Files         176      176              
  Lines       12391    12459      +68     
==========================================
+ Hits         7274     7319      +45     
- Misses       5117     5140      +23     

Impacted Files	Coverage Δ
boa/src/syntax/lexer/template.rs	48.78% <ø> (ø)
boa/src/syntax/lexer/string.rs	60.74% <60.00%> (+7.65%)	⬆️
boa/src/builtins/map/mod.rs	71.08% <70.00%> (-1.29%)	⬇️
boa/src/builtins/map/ordered_map.rs	57.57% <0.00%> (-6.07%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a5061c...6fa0642. Read the comment docs.

[Razican]

Test262 conformance changes:

Test result	master count	PR count	difference
Total	78,497	78,497	0
Passed	24,813	24,836	+23
Ignored	15,587	15,587	0
Failed	38,097	38,074	-23
Panics	20	20	0
Conformance	31.61	31.64	+0.03%

[jevancc]

Test262 conformance changes:

Test result master count PR count difference
Total 78,497 78,497 0
Passed 24,805 24,811 +6
Ignored 15,587 15,587 0
Failed 38,105 38,099 -6
Panics 26 27 +1
Conformance 31.60 31.61 +0.01%

The new panic is fixed. Now the changes become Passed +7.

[RageKnify]

LGTM, good job!

[tofpie]

Thanks a lot. This looks good to me.

There is a very last tiny corner case that is not handled: when a string contains \8 or \9. This results into a 8 or 9 in non-strict mode, but in strict mode (or in template literals), this should throw a syntax error.

And this is not related to octal escape sequence, but this is part of escape sequence, there is an issue with \v that is currently not handled correctly, it produces v instead of Vertical Tabulation (Unicode 0x000B). Maybe you could also add this fix to this PR.

[jevancc]

Thanks a lot. This looks good to me.

There is a very last tiny corner case that is not handled: when a string contains \8 or \9. This results into a 8 or 9 in non-strict mode, but in strict mode (or in template literals), this should throw a syntax error.

And this is not related to octal escape sequence, but this is part of escape sequence, there is an issue with \v that is currently not handled correctly, it produces v instead of Vertical Tabulation (Unicode 0x000B). Maybe you could also add this fix to this PR.

Ok, I just renamed this PR and will do all the fixes in it. I will re-request reviews after I finish it.

For recording the issue that will be fixed in this PR: \ followed by a line terminator is also wrong.

[jevancc]

@tofpie the bugs you mentioned are addressed. I cannot request your review through github so I tagged you in this comment :)

[RageKnify]

Really like the refactor, separating into functions helps a lot in understanding the code.
Don't like the loose unsafe, either we remove it for a minor performance penalty or add a comment (I'm fine with either solution).

Updated @Razican's comment with the conformance numbers, improved a bit more.