Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Const-only capture/closure #563

Closed
singpolyma-shopify opened this issue Mar 28, 2019 · 5 comments
Closed

Const-only capture/closure #563

singpolyma-shopify opened this issue Mar 28, 2019 · 5 comments
Labels

Comments

@singpolyma-shopify
Copy link
Contributor

I think it would be very useful if functions could capture variables from their lexical scope as a constant. I know that enabling mutation from inside the closure makes ownership rules a bit hard and I read elsewhere that's why there are no closures yet, but perhaps limiting to constants would help enough to get us part way there?

@dcodeIO
Copy link
Member

dcodeIO commented Mar 28, 2019

Yeah, this has been discussed a while ago. To make this work, a function enclosing a variable would have to take it as an additional argument. To give a bit of background for others wondering:

WASM does not have nested functions as one is used to in JS, so

// TS
function foo(): i32 {
  var a = 1;
  function bar(): i32 {
    return a;
  }
  return bar();
}

would have to compile to something like

// WASM
function foo(): i32 {
  var a = 1;
  return foo_bar(a);
}

function foo_bar(__closure_a: i32): i32 {
  return __closure_a;
}

which obviously detaches the enclosed variable, leaving it technically mutable, but not propagating changes back to the closure, but is still a ok-ish placeholder until there's something better.

A proper implementation that allows mutating the variable would need to allocate a closure on the heap that contains a and redirect all accesses in both foo and bar to the closure. Issue here is lifetime because a function isn't represented by a heap object itself, making it necessary to find another way to free the closure's memory once the lifetime of all functions referencing it ends. This problem can be illustrated in

function foo(): () => i32 {
  var a = 1;
  function bar(): i32 {
    return a;
  }
  return bar;
}

where the lifetime of the reference to bar returned by foo is the reference for the closure's lifetime. Also note that foo could be called multiple times, returning multiple closures that must become collected somehow.

@singpolyma-shopify
Copy link
Contributor Author

I'm working around it by using a Closure class for now, but it makes me feel like an old-timey Java dev ;)

@MaxGraey
Copy link
Member

MaxGraey commented Mar 28, 2019

We already discussed about closures and I think it definitely could implement before function references proposal. One main question is how handle closure's lifetime without GC/RC

@dcodeIO
Copy link
Member

dcodeIO commented Mar 28, 2019

Thinking about this a bit more: One first step to solve the GC issue could be to wrap a function table index in a heap object and returning a pointer to that instead of just a bare function index, of the form

class Closure {
  functionIndex: u32;
  data: ClosureData | null
}

where ClosureData is just a blob of data with no static type information. Instead, static type information goes into compiled functions in the form of loads and their offsets (if that's possible, still thinking). Now, whenever a function is called indirectly, the function index is loaded and data appended to the call. The reference to a function then becomes a reference to the closure (indexes aren't collected anyway), making the closure itself and its data collectible once not referenced anymore.

One missing building block for this is that the compiler must evaluate the conditions leading to a closure in a pre-pass, so it can replace accesses to locals before it sees the closure using them. That's the hard part (because the compiler is single-pass currently) :)

@stale
Copy link

stale bot commented Apr 27, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 27, 2019
@stale stale bot closed this as completed May 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants