The default bug view has changed. See this FAQ.

Expose RegExp flags and source in JS API

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: philikon, Assigned: philikon)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

v8monkey needs to access RegExp flags and source, would be good to have those exposed in the public API. I propose adding JS_GetRegExpFlags and JS_GetRegExpSource. Patch coming up.
Created attachment 530973 [details] [diff] [review]
v1

I debated for a while with myself whether JS_GetRegExpSource should return char*, jschar* or JSString*. In the end I chose consistency with JS_NewRegExpObject and hence made it char*. I don't feel strongly about this, though.

(N.B.: Patch is on top of bug 650931)
Assignee: general → philipp
Status: NEW → ASSIGNED
Attachment #530973 - Flags: review?(gal)

Comment 2

6 years ago
Comment on attachment 530973 [details] [diff] [review]
v1

Review of attachment 530973 [details] [diff] [review]:
-----------------------------------------------------------------

What about unicode?
Yeah, the NewRegExpObject signature is a bug that shouldn't be copied, IMO.
JSString*, I think, because it flows naturally into other code that needs JS values.  We should not use char* in APIs as a general rule because it has conversion costs, it has no obvious encoding, and it's potentially lossy.  It also seems worth noting that returning stuff like JSString* that's "automatically" kept alive and is "automatically" released when no longer needed is better than returning stuff that requires manual deallocation like char*.
Created attachment 530977 [details] [diff] [review]
v2

Good to know that the JS_NewRegExpObject API is busted. I do agree with Waldo about the conversion cost and allocation. In any case, JSString would be easier for us in v8monkey, so here we go: JS_GetRegExpSource now returns JSString.
Attachment #530977 - Flags: review?(gal)
(Also, I now see there's JS_NewUCRegExpObjectNoStatics which takes jschar instead of char, so the consistency argument wasn't valid anyway. Pretty clear now that JS_NewRegExpObject isn't a great API.)

Updated

6 years ago
Attachment #530977 - Flags: review?(gal) → review+
(Assignee)

Updated

6 years ago
Attachment #530973 - Attachment is obsolete: true
Attachment #530973 - Flags: review?(gal)
http://hg.mozilla.org/tracemonkey/rev/8376b783d165
Whiteboard: [spidernode] → [spidernode][fixed-in-tracemonkey]
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8376b783d165
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.