At present, jsapi.h declares: JS_CompileScript JS_CompileScriptForPrincipals JS_CompileScriptForPrincipalsVersion JS_CompileUCScript JS_CompileUCScriptForPrincipals JS_CompileUCScriptForPrincipalsVersion JS_CompileUCScriptForPrincipalsVersionOrigin JS_CompileUTF8File JS_CompileUTF8FileHandle JS_CompileUTF8FileHandleForPrincipals JS_CompileUTF8FileHandleForPrincipalsVersion However, bug 765993 would like to provide source map information, and bug 637572 would like to provide origin information. The current pattern is clearly not what we want to follow. Instead, we should have a JS::CompileOptions type, with member functions for setting various options (and checking for consistency). Then, we should have a JS::Compile function, overloaded for each way of passing text: - char *, length - jschar *, length - FILE * This would reduce the length of many argument lists within SM itself, as well.

Yes please! For bug 462300, I'll most likely want to add yet another set of these functions in the current scheme of things, making this change even more desirable.

Here's something that compiles...

Looks good.

Rebased.

Try: https://tbpl.mozilla.org/?tree=Try&rev=cdecb369e786

I screwed up the version handling; turns out that AutoVersionAPI *modifies* the version passed to it. New try: https://tbpl.mozilla.org/?tree=Try&rev=69ff744bd1f4

Use of AutoVersionAPI updated.

Third try push: I mis-read the code that reads a file into memory, assuming that 'len' was the length. Shoot me now. Rewritten using Vector<char, ...>. https://tbpl.mozilla.org/?tree=Try&rev=3d2987f423fa

All looks good. Full try: https://tbpl.mozilla.org/?tree=Try&rev=5020615a1861

Updated patch.

Comment on attachment 643497 [details] [diff] [review] Pull out compilation variants into a CompileOptions structure. Review of attachment 643497 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, although you might want to solicit njn's opinion since he has been touching the same area recently. ::: js/src/jsapi.h @@ +4937,5 @@ > +Compile(JSContext *cx, JSHandleObject obj, const CompileOptions &options, > + const char *bytes, size_t length); > + > +extern JS_PUBLIC_API(JSScript *) > +Compile(JSContext *cx, JSHandleObject obj, CompileOptions options, This API passes CompileOptions by value while most of the others use const &. In general, I doubt perf here matters enough to warrant const & over passing by value (we're about to parse, after all), so I'd be just as happy if you converted all the other APIs to pass by value. By value also is a bit shorter to read :) Whichever you choose, I see a few other const&/value inconsistencies (e.g., below), so it'd be good to give the patch a quick scan to make sure they are all one way or the other.

On IRC it was resolved that both George Sand and T.S. Eliot would prefer that CompileOptions be consistently passed by value.

ONE MORE TIME: https://tbpl.mozilla.org/?tree=Try&rev=948fba659d35

That looks okay. https://hg.mozilla.org/integration/mozilla-inbound/rev/6e7fa061e61a

Huzzah!