No API to determine if a JSObject is a RegExp

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: zpao, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [spidernode][fixed-in-tracemonkey])

Attachments

(2 attachments)

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.
I don't think we need comments or tests for something so small. Feel free to request review.

Updated

6 years ago
Attachment #526833 - Flags: review+
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?
Whiteboard: [spidernode]
(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.
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

6 years ago
RegExps don't work well across compartments. You are supposed to copy them, not wrap them.
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?
Attachment #530972 - Flags: review?(gal)
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

6 years ago
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.
Attachment #530972 - Flags: review?(gal) → review+

Comment 9

6 years ago
I have no opinion on 7. JS_IsRegExpObject sounds pretty good too.
(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.
("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.
http://hg.mozilla.org/tracemonkey/rev/4b5ba020745e
http://hg.mozilla.org/tracemonkey/rev/adceec18aea0
Whiteboard: [spidernode] → [spidernode][fixed-in-tracemonkey]
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/4b5ba020745e
http://hg.mozilla.org/mozilla-central/rev/adceec18aea0
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.