1095015 - Bring sanity to the compileandgo stuff

Status: RESOLVED DUPLICATE of bug 679939

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Per IRC discussion just now, it seems like the real meaning of compile-n-go at this point is "I promise to run this script with only a global on the scope chain".  Note that this does not guarantee that only a single global will be used; that's OK because we'll clone the script if that happens, right?

Is that correct?

If so, I would like to propose the following changes:

1)  Rename the flag to make this clear.
2)  Change the JS_ExecuteScript API to come in two flavors: one taking a scope
    chain (in the form of an AutoObjectVector) and one using the global of the
    cx compartment, similar to what we did for function compilation.  This will
    incidentally avoid weird script scope chains.
3)  Make the scope chain form of JS_ExecuteScript throw if passed a script
    that's compileAndGo() (or whatever the new name is) and the scope chain is
    nonempty.
4)  Make the version of CloneFunctionObject that takes a scope chain throw if
    the function was compiled compileAndGo and the scope chain is nonempty.
5)  Make the versions of CompileFunction that take a scope chain throw if the
    compile is compileAndGo and the scope chain is nonempty.

Thoughts?

Comment 1

[Jason Orendorff [:jorendorff]]

I don't think the premise is correct; eval-in-frame uses compile-and-go compilation.

Suppose the premise is wrong. Nonetheless,

1) yes to renaming the flag to say whatever it means these days

2) yes to requiring weird scope chains to be specified explicitly and in full at the API entry point

3) 4) 5) yes to making the Execute entry point understand and enforce the invariants

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK.  So who can tell me what the flag actually means these days?

Comment 3

[Jason Orendorff [:jorendorff]]

Unless I'm very much mistaken, nobody. Search for compileAndGo. I see 65 mentions, but most of them are like

    bool compileAndGo() const {
        return compileAndGo_;
    }

Comment 4

[Jason Orendorff [:jorendorff]]

To complete the sentiment in comment 1:

0) Keep looking to delete the damn thing. I'll dance with you on its grave.

Comment 5

[Luke Wagner [:luke]]

Yes, please, kill it.  Here's a bug with some relevant details: bug 679939.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I'm just going to follow up here in bug 679939.  In particular, see bug 679939 comment 29.