Closed
Bug 582717
Opened 14 years ago
Closed 14 years ago
Make regular expressions not callable
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
(Keywords: dev-doc-complete, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
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.
Attachment #460982 -
Flags: review?(brendan)
Comment 1•14 years ago
|
||
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
Assignee | ||
Comment 2•14 years ago
|
||
(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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
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
Attachment #460982 -
Flags: review?(brendan)
Comment 6•14 years ago
|
||
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
Target Milestone: mozilla2.0b3 → Future
Comment 7•14 years ago
|
||
Just to be clear, I didn't actually say that they would. But I will relay the news and help coordinate. Thanks!
Assignee | ||
Comment 9•14 years ago
|
||
Attachment #460982 -
Attachment is obsolete: true
Attachment #516362 -
Flags: review?(brendan)
Comment 10•14 years ago
|
||
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
Attachment #516362 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 11•14 years ago
|
||
/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.
Assignee | ||
Comment 12•14 years ago
|
||
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).
Keywords: dev-doc-needed
Whiteboard: fixed-in-tracemonkey
Comment 13•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 14•14 years ago
|
||
note that this caused bug 646509
Comment 15•14 years ago
|
||
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
Updated•14 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 16•14 years ago
|
||
WebKit bug#: https://bugs.webkit.org/show_bug.cgi?id=28285
Comment 17•13 years ago
|
||
(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•13 years ago
|
||
(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).
You need to log in
before you can comment on or make changes to this bug.
Description
•