Last Comment Bug 650931 - No API to determine if a JSObject is a RegExp
: No API to determine if a JSObject is a RegExp
Status: RESOLVED FIXED
[spidernode][fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-18 14:42 PDT by Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
Modified: 2011-05-23 14:14 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v0.1 (no comments, no tests) (995 bytes, patch)
2011-04-18 14:42 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
gal: review+
Details | Diff | Splinter Review
tests v1 (1.38 KB, patch)
2011-05-08 19:40 PDT, Philipp von Weitershausen [:philikon]
gal: review+
Details | Diff | Splinter Review

Description Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-04-18 14:42:23 PDT
Created attachment 526833 [details] [diff] [review]
Patch v0.1 (no comments, no tests)

JS_NewRegExpObject (and such) return a JSObject, but there's no way to determine at a later point if that object is a RegExp. For dates we have JS_ObjectIsDate, functions JS_ObjectIsFunction.

Attached is a patch to make this work - no comments or tests though. If somebody wants to take this and run, go for it.
Comment 1 David Mandelin [:dmandelin] 2011-04-19 15:00:06 PDT
I don't think we need comments or tests for something so small. Feel free to request review.
Comment 2 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-04-19 15:24:21 PDT
That was easy... I'll have more of these for exposing other parts of a RegExp which should probably have tests (but that'll be a different bug so I'll bring it up when I file).

What's the process for small JS patches like this? Land in TM and just wait for a merge or land in m-c and it'll get merged to TM?
Comment 3 David Mandelin [:dmandelin] 2011-04-19 15:47:41 PDT
(In reply to comment #2)
> What's the process for small JS patches like this? Land in TM and just wait for
> a merge or land in m-c and it'll get merged to TM?

Either way is fine, so just do whichever is more convenient for you. You could also double-land if you want to land first on TM but get the change without waiting for the next merge.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-04-19 15:50:59 PDT
So, um, should this work like JS_IsArrayObject and also return true for wrapped regular expressions?  And I do think this needs a little bit of testing for corner-ish cases like that, at least if we're r+ing patches that are not considering such subtilties.

JS patches land in TM first as a general rule.
Comment 5 Andreas Gal :gal 2011-04-19 15:51:44 PDT
RegExps don't work well across compartments. You are supposed to copy them, not wrap them.
Comment 6 Philipp von Weitershausen [:philikon] 2011-05-08 19:40:07 PDT
Created attachment 530972 [details] [diff] [review]
tests v1

Tests for JS_ObjectIsRegExp. Assuming r+ for those, is this good to go or do we need sr?
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-08 19:52:49 PDT
I think that should be JS_IsRegExpObject for consistency with JS_IsArrayObject.  (Although, if that precedent weren't there, I'd say make it just JS_IsRegExp as "Object" is part of the method signature.)
Comment 8 Andreas Gal :gal 2011-05-08 20:25:25 PDT
Comment on attachment 530972 [details] [diff] [review]
tests v1

This is a minor API change, I am confident Brendan is ok with it. No sr needed.
Comment 9 Andreas Gal :gal 2011-05-08 20:26:52 PDT
I have no opinion on 7. JS_IsRegExpObject sounds pretty good too.
Comment 10 Philipp von Weitershausen [:philikon] 2011-05-08 20:48:42 PDT
(In reply to comment #7)
> I think that should be JS_IsRegExpObject for consistency with
> JS_IsArrayObject.

I was thinking that too, but then I saw there's also JS_ObjectIsFunction and JS_ObjectIsDate, so I didn't propose changing that. I really don't care either way.
Comment 11 Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-08 20:58:50 PDT
("consistency", said I?  Ha!)

IsFoo seems better to me because if you're asking the question, you probably have an object in mind, so the "subject" (as it were) of the inquiry can and should be implied.  (Example: in the JSON.parse Walk spec method, you first get a property from an object, then if it's an object you check whether it's an array or not and proceed accordingly.  The object to be inspected is your primary thought when you need to make the is-array check.)  Probably it doesn't matter overmuch, and if the name stays as-is, when jsapi2 comes we should be more careful to be consistent with the updated methods' names.
Comment 13 Chris Leary [:cdleary] (not checking bugmail) 2011-05-23 14:14:04 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/4b5ba020745e
http://hg.mozilla.org/mozilla-central/rev/adceec18aea0

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