Bug 170687

Summary: test262: test262/test/language/literals/regexp/u-dec-esc.js
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: JavaScriptCoreAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, joepeck, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
[PATCH] For Bots
none
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2017-04-10 12:38:03 PDT
Summary:
Unicode DecimalEscape with # > BackReferenceLimit should throw a SyntaxError
Non-Unicode has fallback behavior to an IdentityEscape.

Test:
js> /\8/.exec("a8b")
8

js> /\8/u.exec("a8b")
null

Expected:
The second case (unicode) should throw a SyntaxError.

Spec:

B.1.4 Regular Expressions Patterns
https://tc39.github.io/ecma262/#sec-regular-expressions-patterns

> DecimalEscape::
>   NonZeroDigit DecimalDigits(opt) [lookahead ∉ DecimalDigit]
>
> AtomEscape[U]::
>   [+U]DecimalEscape
>   [~U]DecimalEscape but only if the integer value of DecimalEscape is <= _NcapturingParens_
>   CharacterClassEscape
>   CharacterEscape[~U]

In the non-Unicode case we only go to DecimalEscape if # < capturing parens. Otherwise it falls down to character escapes (and ultimate Identity Escape).
  => non-Unicode case just escapes the character

In the Unicode case we go to DecimalEscape...

21.2.2.11DecimalEscape
https://tc39.github.io/ecma262/#sec-decimalescape

> The production DecimalEscape::NonZeroDigit evaluates as follows:
> 
>     Return the MV of NonZeroDigit.
>
> The production DecimalEscape::NonZeroDigitDecimalDigits evaluates as follows:
> 
>     Let n be the number of code points in DecimalDigits.
>     Return (the MV of NonZeroDigit × 10n) plus the MV of DecimalDigits.
>
> The definitions of “the MV of NonZeroDigit” and “the MV of DecimalDigits” are in 11.8.3.
>
> NOTE
> If \ is followed by a decimal number n whose first digit is not 0, then the escape sequence
> is considered to be a backreference. It is an error if n is greater than the total number
> of left-capturing parentheses in the entire regular expression.

The NOTE says to throw an Error if n > capturing parens.
  => Unicode case should throw an error

Notes:
- Chrome throws a syntax error for /\2/u
- Firefox throws a syntax error for /\2/u
Comment 1 Joseph Pecoraro 2017-04-10 23:15:07 PDT
Seems there are some test262 tests that cover this, and we do fail them:
test262/test/language/literals/regexp/u-dec-esc.js
test262/test/language/literals/regexp/u-invalid-legacy-octal-escape.js
test262/test/language/literals/regexp/u-invalid-oob-decimal-escape.js
Comment 2 Joseph Pecoraro 2017-04-11 02:13:04 PDT
Created attachment 306789 [details]
[PATCH] For Bots
Comment 3 Build Bot 2017-04-11 02:15:04 PDT
Attachment 306789 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:898:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:899:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:900:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:901:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:902:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:903:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:904:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:905:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:906:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:907:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:908:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:909:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:910:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:911:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:912:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:913:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:914:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 17 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Joseph Pecoraro 2017-04-11 08:54:37 PDT
Created attachment 306821 [details]
[PATCH] Proposed Fix
Comment 5 Build Bot 2017-04-11 08:57:13 PDT
Attachment 306821 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:898:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:899:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:900:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:901:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:902:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:903:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:904:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:905:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:906:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:907:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:908:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:909:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:910:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:911:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:912:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:913:  One space before end of line comments  [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/yarr/YarrPattern.cpp:914:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 17 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Michael Saboff 2017-04-12 19:23:26 PDT
Comment on attachment 306821 [details]
[PATCH] Proposed Fix

r=me
Comment 7 WebKit Commit Bot 2017-04-12 19:51:21 PDT
Comment on attachment 306821 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 306821

Committed r215311: <http://trac.webkit.org/changeset/215311>
Comment 8 WebKit Commit Bot 2017-04-12 19:51:23 PDT
All reviewed patches have been landed.  Closing bug.