Last Comment Bug 655641 - Expose RegExp flags and source in JS API
: Expose RegExp flags and source in JS API
Status: RESOLVED FIXED
[spidernode][fixed-in-tracemonkey]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Philipp von Weitershausen [:philikon]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-08 19:47 PDT by Philipp von Weitershausen [:philikon]
Modified: 2011-05-23 14:10 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (3.04 KB, patch)
2011-05-08 20:28 PDT, Philipp von Weitershausen [:philikon]
no flags Details | Diff | Splinter Review
v2 (2.87 KB, patch)
2011-05-08 21:16 PDT, Philipp von Weitershausen [:philikon]
gal: review+
Details | Diff | Splinter Review

Description Philipp von Weitershausen [:philikon] 2011-05-08 19:47:47 PDT
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.
Comment 1 Philipp von Weitershausen [:philikon] 2011-05-08 20:28:43 PDT
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)
Comment 2 Andreas Gal :gal 2011-05-08 20:32:30 PDT
Comment on attachment 530973 [details] [diff] [review]
v1

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

What about unicode?
Comment 3 Mike Shaver (:shaver -- probably not reading bugmail closely) 2011-05-08 20:34:03 PDT
Yeah, the NewRegExpObject signature is a bug that shouldn't be copied, IMO.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-08 20:48:54 PDT
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*.
Comment 5 Philipp von Weitershausen [:philikon] 2011-05-08 21:16:20 PDT
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.
Comment 6 Philipp von Weitershausen [:philikon] 2011-05-08 21:24:16 PDT
(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.)
Comment 7 Philipp von Weitershausen [:philikon] 2011-05-17 20:06:18 PDT
http://hg.mozilla.org/tracemonkey/rev/8376b783d165
Comment 8 Chris Leary [:cdleary] (not checking bugmail) 2011-05-23 14:10:37 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8376b783d165

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