887676 - RegExp lastIndex not set to 0 after exec() doesn't match

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[tashmore]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130511120803

Steps to reproduce:

In SpiderMonkey v1.8.5, I had the following session:

js> r = /a/
/a/
js> r.lastIndex = 999
999
js> r.exec('bc')
null
js> r.lastIndex
999
js> 


Actual results:

Even though the "exec" call returns null, the "r.lastIndex" property is unchanged (999). This behavior contradicts the ECMA standards on regular expressions.


Expected results:

According to ECMA-262 section 15.10.6.2, the "r.lastIndex" property should be 0.

Comment 1

[Sankha Narayan Guria [:sankha]]

Duplicate of bug 501739?

Comment 2

[Boris Zbarsky [:bzbarsky]]

No, this is not about the _global_ RegExp.lastIndex.

Comment 3

[Jeff Walden [:Waldo]]

Yeah, my initial skim of the algorithm says the bug report's totally valid.  Which is crazy, because haven't we touched this and looked at it more than a few times recently?  :-)  But it's what I'm seeing at a quick glance.

I'm happy to take this if we're fine waiting til the Intl API is enabled in desktop builds -- otherwise someone else should grab.

Comment 4

[Sean Stangl [:sstangl]]

I'll grab, since I looked at regexp code recently. Thanks for reporting.

Comment 5

[Till Schneidereit [:till]]

Isn't that exactly what this message is about? https://groups.google.com/forum/#!topic/mozilla.dev.tech.js-engine.internals/T0B9VBa8byI

It seems to have been a deliberate change in bug 646490.

Comment 6

[Sean Stangl [:sstangl]]

Attachment: Always zero lastIndex upon failure to match.

As Till pointed out, this does appear to be deliberate behavior. On the other hand, my reading of 15.10.6.2 does suggest that the deliberate behavior is wrong.

This patch implements the change -- I'll leave it to Waldo to make a decision on how to interpret the specification.

Comment 7

[tashmore]

Any update on whether this deviation from the spec is intentional?

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 768667 [details] [diff] [review]
Always zero lastIndex upon failure to match.

Review of attachment 768667 [details] [diff] [review]:
-----------------------------------------------------------------

I *think* this change was just an oversight in that bug, given there's no discussion of this at all there.  So this is a fine change.  Dunno about the sticky-change here, but given we have no spec to follow there, whatever.  :-\

::: js/src/builtin/RegExp.cpp
@@ +586,5 @@
>          return RegExpRunStatus_Error;
>  
> +    /* Steps 9a and 11 (with sticky extension). */
> +    if (status == RegExpRunStatus_Success_NotFound)
> +        reobj->zeroLastIndex();

It's unclear to me that the lastIndex should be zeroed if this is a sticky RegExp, when no more matches are found.  Our own documentation trolls us here, providing a demo that stops *just short* of saying what should happen:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/sticky

Good times with non-standard, underspecified features as usual.  (ES6 says nothing about the semantics of sticky RegExps yet, at least not in the draft I have.)

Be on the lookout for this change, as concerns sticky RegExps specifically, breaking some extension or another.

Comment 9

[Sean Stangl [:sstangl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b678c591bb5

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3b678c591bb5