endojs
Use Compartment.p.evaluate instead of eight magic lines on XS
Currently, even on xsnap, we're building SES essentially the same way we do on normal JS, by running the full SES shim including the eight magic lines built on direct eval. Currently, XS eval does not understand the sourceURL directive, so all SES code runs in XS without any tie to a filename. Also, using the eight magic lines, we cannot fix #31
Instead, we could switch to using the XS builtin Compartment.prototype.evaluate as our safe evaluator, replacing the eight magic lines, while still building the rest of SES using code from the SES shim. This should immediately fix #31 .
In addition, perhaps it is easier to get XS to accept a filename via Compartment.prototype.evaluate rather than the sourceURL directive in an eval? Attn @phoddie
@erights - We'd like to see xsnap switch over to the XS implementation of Compartments. It should be more efficient and it would let us get some experience together on Compartments. However, recall that thee snapshot capability of xsnap and Compartments are mutually exclusive at the moment. There's a non-trivial investigation needed to see if/how those can be used together.
The sourceURL directive embedded at the end of the source text is a little ugly for us to support, which is why it isn't there already. Extending Compartment.prototype.evaluate to accept the equivalent of a sourceURL is simple by comparison. That would behave the same way -- e.g. aa hint of where the source text came from, but not a path for the engine to load the source text from. The cleanest way to pass that in is less obvious. Do you have a suggestion?
How about as a sourceURL option in a options bag as the second argument to evaluate
compartment.evaluate(src, { sourceURL: 'file:stuff' });
Attn @kriskowal @dckc
... recall that thee snapshot capability of xsnap and Compartments are mutually exclusive at the moment.
Interesting.
I had certainly forgotten that, if I ever knew it.
@dckc - FWIW, email from September 30, 2020:
...there is a fundamental conflict between the way XS implements compartments and snapshots...
@erights - Another options bag! If @kriskowal and @dckc are OK with that approach too, we can take a look later this week.
Adding sourceURL to the compartment.evaluate options bag and surfacing that URL in stack traces should work, provided compartments and snapshots can be made to work together.
Adding sourceURL to the compartment.evaluate options bag and surfacing that URL in stack traces should work, provided compartments and snapshots can be made to work together.
OK. So then we should not spend time on the sourceURL until snapshots and Compartments can work together.
Looks like this is a dup of https://github.com/Agoric/agoric-sdk/issues/2480 ; I was pretty sure we already had an issue for this.
Maybe I'll close the one in agoric-sdk in favor of this...
Agreed. This is the repository where the integration work will have to land.
Actually, I was just having second thoughts; the most recent xsnap work is in agoric-sdk, so it seems likely to land there, unless / until we move it (#681 ). What suggests that the integration has to land here, @kriskowal ?
We’ll need a follow-up change in SES to feature-detect the suitable XS Compartment and switch the behavior of performEval to use that instead of the “three wrongs of with + sloppy + eval that make a right”.
We currently depend on the 8 magic lines proxy for two features that we can't emulate using compartment.evaluate:
- So-called "sloppy globals" where strict a top level assignment to an unknown global variable creates it rather than throwing. We use this in our REPL for a much more pleasant interactive experience.
- Emulating live bindings by translation. Our StaticModuleRecord translation of esm code into evaluable script translates live-bound variables into assignment to free variables, depending on the proxy to update all importing sites.
In addition, there is another potential gotcha with the live binding transformation. Currently we consider an exported variable to be a live binding if there are any statically visible assignments to that variable. However, the direct eval expression enables assignments that are not statically visible. Fortunately, a direct eval expression is itself statically visible. Under the 8 magic lines, we did not need to worry about this because we could not support direct eval anyway. If we switch to compartment.evaluate, we might support direct eval (though it would still be tricky). Therefore, our translation should consider all assignable variables in the scope of an occurrence of the direct eval expression syntax to be a live binding.
For the "sloppy globals" issue, we could painlessly add a dirt simple translator to our repl, replacing a first line matching
/^(\s*)(\w*\s*=)/
with
`${m[1]}globalThis.${m[2])`
If the price of moving to compartment.evaluate is to give up live bindings, that price is easily worth paying. Alternatively, iff a translated compartment says it exports a live binding, we put it within a with on an object with just those accessor properties reflecting the live bound variables. But only if we still have access to sloppy mode :(
Noting that there is yet another bug under 8 magic lines that would be fixed by moving to compartment.evaluate
ReferenceError vs typeof. Using only the 8 magic lines, it is impossible to both
- get all normal use occurrences of a non-existent variable, say
foo, to throw a ReferenceError as it should - get
typeof foo, for unknown variablefooto evaluate toundefinedas it should
Moving to compartment.evaluate should fix this without further work.
See also https://github.com/Agoric/agoric-sdk/issues/2480 which is dup-ish, but we're keeping both open for now.
@kriskowal @mhofman , has this been completed? Can we close this and https://github.com/Agoric/agoric-sdk/issues/2480 now?
This is not completed. We made a mountain of progress toward this in 2024, then a mountain of progress toward the requisite Xsnap upgrade in the first half of 2025. See https://github.com/endojs/endo/issues/400