Last Comment Bug 582717 - Make regular expressions not callable
: Make regular expressions not callable
Status: RESOLVED FIXED
fixed-in-tracemonkey
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Future
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
:
Mentors:
Depends on: 646509
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-28 13:02 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-06-26 17:23 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (13.57 KB, patch)
2010-07-28 13:02 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
Do the deed (16.95 KB, patch)
2011-03-02 13:05 PST, Jeff Walden [:Waldo] (remove +bmo to email)
brendan: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2010-07-28 13:02:20 PDT
Created attachment 460982 [details] [diff] [review]
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] 2010-07-28 13:15:38 PDT
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] (remove +bmo to email) 2010-07-28 13:19:27 PDT
(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] 2010-07-28 13:19:52 PDT
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] 2010-07-28 17:09:25 PDT
(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] 2010-08-02 21:01:54 PDT
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] 2010-10-01 15:18:58 PDT
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 2010-10-01 19:35:13 PDT
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 2010-10-02 03:31:58 PDT
http://code.google.com/p/v8/issues/detail?id=617#c6
Comment 9 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-02 13:05:05 PST
Created attachment 516362 [details] [diff] [review]
Do the deed
Comment 10 Brendan Eich [:brendan] 2011-03-02 14:53:58 PST
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] (remove +bmo to email) 2011-03-02 17:24:41 PST
/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] (remove +bmo to email) 2011-03-03 15:08:40 PST
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) 2011-03-31 14:32:27 PDT
http://hg.mozilla.org/mozilla-central/rev/001bfbd35ed9
Comment 14 Jo Hermans 2011-04-08 12:56:35 PDT
note that this caused bug 646509
Comment 15 Eric Shepherd [:sheppy] 2011-04-12 12:07:07 PDT
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 2011-04-12 12:12:16 PDT
WebKit bug#: https://bugs.webkit.org/show_bug.cgi?id=28285
Comment 17 Dimitri Saltanov 2011-06-26 13:04:38 PDT
(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 2011-06-26 17:23:58 PDT
(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).

Note You need to log in before you can comment on or make changes to this bug.