582717 - Make regular expressions not callable

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

JägerMonkey is running into bugs and failures due solely to regular expression callability.  The feature is of dubious value (just call exec directly on the regular expression for identical effect), and nobody else does it, so we should remove this obstacle to success rather than spend time making it work.

Comment 1

[Brendan Eich [:brendan]]

Nobody else does it?

$ ./jsc
> /hi/("hi")
hi

V8 too (typeof difference last noted in es-discuss, maybe they've fixed).

Your opinion on dubious value is noted (I called callable regexps a mistake, so I see and raise :-P) but not relevant to whether we can afford to get rid of this. If we try and fail we will waste time, potentially of a lot of people.

Google codesearch is hard to use for this -- regular expression matching a regexp called with ( right after the closing / is tricky to write. Anyone have greater regex skilz succeed in finding /re/(s) instances?

Cc'ing Ollie again. There may be webkit history not in bugs.webkit.org (or not easily found there) showing sites that use callable regexps.

/be

Comment 2

[Jeff Walden [:Waldo]]

(In reply to comment #1)
> Nobody else does it?
> 
> $ ./jsc
> > /hi/("hi")
> hi
> 
> V8 too (typeof difference last noted in es-discuss, maybe they've fixed).

Hum, last I remembered no one else had done it.  I guess that's changed in the meantime.

Comment 3

[Brendan Eich [:brendan]]

Anyway, it sounds like dmandelin is making JM call regexps so why would we take on more risk here for Firefox 4? Let's stay on target.

For Firefox 5 and (this is && so both are needed) in concert with JSC, we can try in early nightlies.

/be

Comment 4

[David Mandelin [:dmandelin]]

(In reply to comment #3)
> Anyway, it sounds like dmandelin is making JM call regexps so why would we take
> on more risk here for Firefox 4?

As an aside, it turns out there is nothing magical at all about having regexps callable in JM--they are already callable--the bug there is something more like the function getting called twice, when it shouldn't, so that must be fixed anyway.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 460982 [details] [diff] [review]
Patch

We do not need to spend time on this right now. Comment 0 is not well-founded per comment 4. My position is the same as it was in comment 3.

/be

Comment 6

[Brendan Eich [:brendan]]

Ollie wants to get rid of callable regexps from JSC. Mark says V8 will do the same. We should coordinate on schedule. It'll be after our current releases in beta...

/be

Comment 7

[Mark S. Miller]

Just to be clear, I didn't actually say that they would. But I will relay the news and help coordinate. Thanks!

Comment 8

[Mark S. Miller]

http://code.google.com/p/v8/issues/detail?id=617#c6

Comment 9

[Jeff Walden [:Waldo]]

Attachment: Do the deed

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 516362 [details] [diff] [review]
Do the deed

Nice. js_TypeOf is (apart from being a C-smelling function not a method) finally a thing of beauty.

MEGO at the tests, rs=me there.

dev-doc-needed and cite the WebKit change for a united front. Is v8 fixing too?

Is tm open for post-moz2/fx4 changes now?

/be

Comment 11

[Jeff Walden [:Waldo]]

/topic in #jsapi, and denizens of #jsapi, say it is.  Out of an abundance of caution (and a relative lack of patches ready for immediate pushing, plus a bunch awaiting review) I'm going to hold off on pushing for a little bit longer, tho.

Comment 12

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/001bfbd35ed9

I'll get to docs soon, although I expect an eventual curator will come around to update the inevitable "What's New in Firefox 5.0" documents whose location I don't know (and which likely don't exist yet).

Comment 13

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/001bfbd35ed9

Comment 14

[Jo Hermans]

note that this caused bug 646509

Comment 15

[Eric Shepherd [:sheppy]]

Looking through documentation, it *appears* that regular expressions being callable was never documented; if it is, it's well obscured.

Do we have a WebKit bug number for the corresponding change to WebKit?

A note has been placed for now on the Firefox 5 for developers page:

https://developer.mozilla.org/en/Firefox_5_for_developers#JavaScript

Comment 16

[Oliver Hunt]

WebKit bug#: https://bugs.webkit.org/show_bug.cgi?id=28285

Comment 17

[Dimitri Saltanov]

(In reply to comment #15)
> Looking through documentation, it *appears* that regular expressions being
> callable was never documented; if it is, it's well obscured.

This feature is actually explicitly documented on MDN, so that should be corrected now too:

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Regexp/exec

Comment 18

[Dimitri Saltanov]

(In reply to comment #17)
I just went through MDN docs for RegExp::exec to see if I can help with the corrections and noticed that some changes were already made to reflect this bug there, but probably because they were kind of sporadic there are still traces of the old syntax there. May be it would be better to revert to the original version with the callable syntax and just mark it as obsolete as opposed to trying to silently exterminate every visual trace of it now? This syntax was on that page for some 5 years and even though it appears to be not extremely popular, it has been a very well known Mozilla extension to JS (to the extent of being replicated in the majority of other engines, obviously).