Implement workarounds for regex parsing known issues

[adams85]

Implements workarounds for regex parsing known issues.

Also, fixes a bug in unicode mode regex handling (code point -> code unit index translation is not needed as .NET regexes return code unit indices).

[lahma]

Nice, thanks for helping out here! Seems that following test cases can also removed from Test262Harness.settings.json exclude now as they pass after the fixes:

"built-ins/RegExp/match-indices/indices-array-unicode-property-names.js",
"built-ins/RegExp/named-groups/non-unicode-property-names.js",
"built-ins/RegExp/named-groups/unicode-property-names.js",
"built-ins/RegExp/prototype/Symbol.replace/named-groups.js",

[adams85]

PR updated. There's a failing macos test but doesn't seem to have anything to do with my changes:

[xUnit.net 00:00:01.75]     Jint.Tests.PublicInterface.PublicInterfaceTests.BindFunctionInstancesArePublic [FAIL]

Error: System.AggregateException : One or more errors occurred. (Unable to cast object of type 'Jint.Runtime.Environments.GlobalEnvironmentRecord' to type 'Jint.Runtime.Environments.FunctionEnvironmentRecord'.)
---- System.InvalidCastException : Unable to cast object of type 'Jint.Runtime.Environments.GlobalEnvironmentRecord' to type 'Jint.Runtime.Environments.FunctionEnvironmentRecord'.

Is this really something unrelated?

[lahma]

PR updated. There's a failing macos test but doesn't seem to have anything to do with my changes:

[xUnit.net 00:00:01.75]     Jint.Tests.PublicInterface.PublicInterfaceTests.BindFunctionInstancesArePublic [FAIL]

Error: System.AggregateException : One or more errors occurred. (Unable to cast object of type 'Jint.Runtime.Environments.GlobalEnvironmentRecord' to type 'Jint.Runtime.Environments.FunctionEnvironmentRecord'.)
---- System.InvalidCastException : Unable to cast object of type 'Jint.Runtime.Environments.GlobalEnvironmentRecord' to type 'Jint.Runtime.Environments.FunctionEnvironmentRecord'.

Is this really something unrelated?

Unrelated. Engine is not thread safe and for some reason that test case is racy on macos.

[lahma]

Looks good. I guess we can merge?

[adams85]

Yep, I think this one is ready.

However, I just discovered another issue:

• "bab".replace(/.(?<a>a)(.)/, "$1$2") results "ab", while
• new Regex(".(?<a>a)(.)", RegexOptions.ECMAScript).Replace("bab", "$1$2") results "ba".

It seems that capturing group numbering is messed up in .NET, at least it doesn't work in the same way as in JS (which assigns strictly increasing numbers to capturing groups, regardless of being a named group or not). This is a pretty sad situation and I'm not sure if we can do anything about it. I'll try to find a workaround but will do that in a separate PR (if it exists at all - but probably that will need to be fixed in Esprima...)

[lahma]

It's almost like that RegexOptions.ECMAScript wasn't doing it's job 😉

[lahma]

For some reason I cannot neither update or rebase this branch via GitHub UI, could you please do the rebase locally and force push?

[lahma]

Thanks once again!

[adams85]

Glad to help!

I did some investigation on capturing group numbering. According to MSDN, this "anomaly" is by design:

[...] Captures that use parentheses are numbered automatically from left to right based on the order of the opening parentheses in the regular expression, starting from 1. However, named capture groups are always ordered last, after non-named capture groups. The capture that's numbered 0 is the text matched by the entire regular expression pattern.

Crazy design decision. I never thought I would say this, but in this regard JS is the sane one ... If only MS had got the ECMAScript compatibility mode right at least... But that ship has sailed for good.

I can see only one viable workaround: in the regex parser we rewrite all named capturing groups to numbered ones (and, of course, named backrefs as well), like /.(?<a>a)(.)\k<a>/ -> @".(a)(.)\1". Only problem is that Jint will (and other consumers may) want to look up matches by group names, so we also need to return that information in some form (probably a group name -> group number lookup), besides the Regex instance. The regex parser gathers this information, so technically this workaround should be possible.

I think we can't get away without addressing this issues because it's a big one. Without fixing this, we can't even call legacy regex handling okayish... So looks like we're in for another RC. I'll get back to you soon.