Last Comment Bug 643532 - Implement JS_ParseJSON, JS_ParseJSONWithReviver
: Implement JS_ParseJSON, JS_ParseJSONWithReviver
Status: RESOLVED FIXED
fixed-in-tracemonkey
: dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla5
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: 589664
  Show dependency treegraph
 
Reported: 2011-03-21 13:17 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-04-14 14:28 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, also add some simple-ish JSAPI tests for them (12.25 KB, patch)
2011-03-21 13:17 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
Fix thinko (12.38 KB, patch)
2011-03-21 13:54 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
luke: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-21 13:17:35 PDT
Created attachment 520727 [details] [diff] [review]
Patch, also add some simple-ish JSAPI tests for them

Almost all callers of JS_BeginJSONParse/JS_ConsumeJSONText/JS_EndJSONParse would really rather have an API where they can just provide source characters and get back the object you get by parsing it.  We should give them such an API, rather than requiring a tedious three-step.  And since so far no one uses the reviver functionality, we should add a separate API to use it: keep the most common API simple, and leave the less-common case under its own separate heading.

There's more refactoring to come in this area, and eventually these method implementations won't just directly implement the begin-consume-finish idiom.  But it seems best to separate out that cleanup from the initial additions: more patches, but smaller and simpler patches.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2011-03-21 13:54:31 PDT
Created attachment 520738 [details] [diff] [review]
Fix thinko
Comment 2 Brendan Eich [:brendan] 2011-04-01 15:27:28 PDT
Comment on attachment 520738 [details] [diff] [review]
Fix thinko

This is one I will duck due to Servo ramping up. Trying Luke.

/be
Comment 3 Luke Wagner [:luke] 2011-04-01 21:57:40 PDT
Comment on attachment 520738 [details] [diff] [review]
Fix thinko

Nice observation.

>+extern JS_FRIEND_API(JSBool)
>+ParseJSONWithReviver(JSContext *cx, const jschar *chars, uint32 length, const Value &filter,
>+                    Value *vp, DecodingMode decodingMode = STRICT);

Alignment.

Also, we have this new jsfriendapi.h... should we use it as such (in support of the eventual goal of not having code outside js/src #include into js/src)?
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2011-04-06 11:34:00 PDT
http://hg.mozilla.org/tracemonkey/rev/9d41622b29f4

There's entanglement of the friend API with enums defined in json.h, and it looked messy disentangling things, and we really really really don't want to encourage use of this anyway, and the APIs here are changing anyway to make the JSON parsing work easier, so I didn't make that change.
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2011-04-13 14:40:57 PDT
This landed in TM and later got merged to m-c:

http://hg.mozilla.org/mozilla-central/rev/9d41622b29f4
Comment 6 Jeff Walden [:Waldo] (remove +bmo to email) 2011-04-14 14:28:21 PDT
https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_ParseJSON (which also documents the *WithReviver special-case)

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