@rmliddle and @shinglyu could you take a look at this?

Sorry for the delay. Will get back to this later today.

In working on this, I came across this comment in main.rs. How does the value get printed there? As far as I can tell, it's not any sort of option passed to compile() or run_code_obj(), so how does it happen?

Hmm, that part changed a lot since I worked on it. @cthulahoops ? ^

Closures from Python to JS! To test it out, build and serve the demo page locally, then in the console enter:

const vm = rp.vmStore.init("main");
vm.addToScope("fetch", (url, handler) => {
  fetch(url)
    .then(r => r.text())
    .then(text => {
      handler([text]);
    });
});
vm.exec(`
a = {}
def handler(text):
    a['result'] = text
fetch('https://httpbin.org/get', handler)
`);
// Wait a bit, then
vm.eval("a['result']");

Do you think there's a better way to handle args/kwargs? Maybe like for JS closures where it's the this variable, where you'd have to do .call({}, ...args). Right now, the JS signature for closures looks like (args: any[], kwargs: { [k: string]: any }) => any).

There are a few kinks to work out with borrow errors, LMK also if you can think of any better patterns than what's in convert::py_to_js.

Oh, and also up for discussion are ways of storing closure handles to prevent memory leaks.

Re: "Printed already"

In single mode the compiler inserts a PRINT_EXPR instruction into the code, so the repl doesn't need to do the print. Demo in Cpython:

>>> import dis; dis.dis(compile("1 + x", "", "single"))
  1           0 LOAD_CONST               0 (1)
              2 LOAD_NAME                0 (x)
              4 BINARY_ADD
              6 PRINT_EXPR
              8 LOAD_CONST               1 (None)
             10 RETURN_VALUE

Implemented here:

https://github.com/RustPython/RustPython/blob/master/vm/src/compile.rs#L117

@cthulahoops thanks!

I'm having trouble solving the problem of VirtualMachines with closures. Here's a scenario which would fail under all the solutions I've come up with:

Define a python closure and pass it to JS. The rust closure holds either a reference to a VM (probably not, cause it wouldn't be 'static) or the ID of the VM. Store the JS function in some arbitrary variable a.

Call a JS function from Python. Currently, the VM store is borrowed, as is the VM. In that JS function, call a. Assuming it's storing the ID of the VM it belongs to, it cannot access it, as the STORED_VMS thread-local variable is currently borrowed. Maybe a solution would be to have the HashMap for STORED_VMS be HashMap<String, Rc<RefCell<StoredVirtualMachine>>>, making the entire declaration:

static STORED_VMS: Rc<RefCell<HashMap<String, Rc<RefCell<StoredVirtualMachine>>>>> = Rc::default();

I guess I was just hashing stuff out in this comment, cause I'm going to try that now.

Wait, no, because it would still be borrowed while the vm was calling the injected JS function

Alright, I've been thinking about this for a while and I think the only way to do this is to use unsafe code and a raw pointer, so I'll probably work on that when I get home today.

HaHahahHA! I've figured out how to manage VirtualMachines without RefCell borrowing errors! (and hopefully without memory leaks.) It took a bit and I don't think it's flawless, so please review the code and see if there's anything that looks questionable.

Codecov Report

Merging #267 into master will decrease coverage by 1.76%.
The diff coverage is 1.69%.

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
- Coverage   50.08%   48.32%   -1.77%     
==========================================
  Files          68       69       +1     
  Lines       13924    14349     +425     
  Branches     3465     3582     +117     
==========================================
- Hits         6974     6934      -40     
- Misses       5096     5559     +463     
- Partials     1854     1856       +2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d19486...e0f222c. Read the comment docs.

I just realized that the fetch builtin might be a little out of the scope of this PR, so I'm going to take it out and make a separate PR for it later.

@shinglyu, could you review this again?

I'm merging this PR since it has grown to old and large, but in general I believe it is good. In future it would be better to split pull requests into smaller chunks.

Thank you! I'll keep that in mind.