Closures

[DuncanUszkay1]

A Closure implementation which works as follows:

• Closures get an extra context argument appended to them
• While compiling a closure, we point to offsets from that context pointer in linear memory to access closed over variables
• After compiling a closure, we generate a class for that closure of this form:

class Closure|X { // X is unique per closure 
  fn: usize // holds function ptr
  x: i32 // outside lexical scope which is referenced from the closure
  ...
}

• The object which is returned from defining a closure is an instantiation of the above object that was generated
• When this object is called in scope:
  • We copy in the values of the local scope into the Closure|X class instance
  • We load the closure context argument, followed by the rest of the normal arguments
  • We call the anonymous method
• When this object is converted into a function type (a type only signified by a type signature). This typically occurs because we're returning it from a function or passing it as an argument to a function
  • We copy in the values of the local scope into the Closure|X class instance one more time
  • We right-shift the bits of the address by 4, removing the 0s that result from 16-bit address alignment
  • We set the MSB
• When an anonymous function is called
  • We check to see if the MSB is set, that implies that it's a closure
  • Apply the same procedure as when it's called in scope, except without copying in local values (since we don't have access to them here)

There are still some improvements to be made, but I think we're in a pretty stable state and things seem to all be working.

cc @willemneal

CURRENT LIMITATIONS

In the interest of time, I'm leaving in some limitations intentionally. They should be represented in the tests closure-limitations and closure-limitations-runtime. Here's a list for reference:

• writing to a closed value
• creating a closure within an anonymous function
• returning a closure from an exported function
• Calling a closure in the same scope it was defined in within a function, i.e.:

class Foo {
  field: i32;
}
var compClosuresSameScope = (): void => {
  var foo: Foo = {
    field: 8
  }
  var withFoo1 = (): void => {
    foo.field += 1;
  }
  var withFoo2 = (): void => {
    withFoo1();
  }
}

[MaxGraey]

you should refresh all tests due to new builtin global var

[dcodeIO]

Change the current approach of setting a global into passing an object alongside the function

Would that mean changing the signature of the function? Asking because doing so can become a problem where normal functions and closures can be used interchangeably.

Call anonymous functions using an invoke method

Can you elaborate how that'd look? Wondering if it implies that we need one invoke function per unique function signature.

so that we can remove the 16 bit alignment hack

An alternative suggestion to avoid the hack has been to use fat pointers for function references, i.e. an i64 with N bits closure address and M bits function index and alignments bits truncated. Has the advantage that signatures remain unchanged (all function refs are i64, even if not a closure), but the disadvantage that i64 on the boundary is not well supported yet.

[willemneal]

This would not change the signature of the function rather, any path where an anonymous function could be a closure it must become an empty one with an invoke method. For example, a function that could return a closure or a normal anon function would then expect the output to be a closure and then when compiling a call site the invoke method would be used.

With this method you wouldn't need to use the original or your new suggested trick.

[dcodeIO]

Can you give me an example how this mechanism would look like? Let's say we have

function doAdd(fn: (a: i32, b: i32) => i32, a: i32, b: i32): i32 {
  return fn(a, b); // may be a closure, or may not be one
}

[MaxGraey]

@DuncanUszkay1 Does it mean that any function (not closure) should wrap by EmptyClosure object?

[MaxGraey]

Any function that can be called indirectly, yes

This make things more complicated if we want efficiency. Like extra analysis which try determine could we simplify this closure object without free vars (captured variables) to simple indirect call which highly required for father optimization like directize and inlining which very important especially for eliminate unnecessary retain / release calls for ARC

[DuncanUszkay1]

Talked about this a bit on Slack- here's the result:

• A function will be identified as a closure when it has it's MSB set. Functions will remain 32 bits long
  • This limits the number of possible functionts to 2147483647
  • When a closure is assigned to a function reference, we bitshift it right by 4 (which doesn't destroy any information because of 16 bit alignment), then set the MSB to 1
  • When a function reference turns out to be a closure (MSB is 1) we bit shift left by 4 to get the closure pointer, then get the function reference (also the first field of the closure, so we don't need to know what specific class the closure has)
• We'll indirectly ref count closures through their function pointers
• Attempting to define a closure within a closure will result in a Not Implemented (for now)
• When calling indirectly, we will save the value of the current global context in a local so that we don't lose its value when making the call.

Some things that weren't mentioned that also need to be fixed:

• let statements can't be closed over

[DuncanUszkay1]

Should let statements be able to be closed over? Seems like given that they're supposed to be bound to the block they live in maybe not- Right now it won't even resolve if you try to reference a let variable in a closure

example:

function createClosure(arg: i32): () => i32 {
  let foo = 2
  var f = (): i32 => { return foo; }
  return f;
}

[DuncanUszkay1]

Discovered a flaw in how I read from the closure context- I don't have access to the class offsets during compilation because we are unaware of the closure before compilation. I had a system in place which made an assumption about where a field would be stored in the class by making some assumptions, but this won't work in a case where you use the same closed variable twice, i.e:

function createClosure(arg: i32): () => i32 {
  var f = (): i32 => { return arg * arg; }
}

Going to look to see how hard it'd be to fix this without the ScopeAnalyzer, then if there isn't a way that makes sense maybe look over the analyzer again to see if we ought to get that in first

[DuncanUszkay1]

Last push should've fixed it- the offset is now being stuck onto the closedLocal as it's created, so we can reuse them.

It does however bring to mind the scope analyzer and whether or not this should be blocked on that or not

[MaxGraey]

How about extends ClosedLocal from Local?

[DuncanUszkay1]

Only reason I didn't do that initially is that the constructor of Local hardcodes its type to LOCAL whereas we need the type CLOSEDLOCAL- and passing in an ElementKind argument to the constructor didn't seem right. You're probably right though and it should extend Local.

Not super opinionated on that though, definitely something I can come back to once I start refactoring/reorganizing when all the functionality is there

[DuncanUszkay1]

One thing that would be greatly appreciated is if someone could try compiling this in 64 bit address mode and let me know if that works. Going to move onto other tasks for now

[dcodeIO]

Attempting that at the current state would yield a broken Wasm binary unfortunately, because the respective instructions ultimately consuming the pointer do not recognize 64-bit inputs yet. It's also likely that there is other code unrelated to your changes not doing it right already, but it's still a good thing to at least try early so an eventual Wasm64 implementation will be easier.

[DuncanUszkay1]

PR which extracts the additional argument added to makeRetain/makeRelease: https://github.com/AssemblyScript/assemblyscript/pull/1287/files

If we can get that into master it should make this diff easier to read, hopefully we can since the PR makes no functional changes

[DuncanUszkay1]

The diff is slowly decreasing in size- if we can get #1287 merged then the diff goes down to just 501 lines (excluding compiled test changes)

[torch2424]

@DuncanUszkay1 As promised, I Wrote a nice test, that I think finds some really nice edge cases, in my advanced but common uses of Closures in Javascript. 😄

But can you give me contributor access to your AssemblyScript fork?

https://github.com/DuncanUszkay1/assemblyscript/tree/closures

In the meantime, heres the test I wrote (tests/compiler/closure-common-js-patterns.ts). The comments explain everything weird I'm trying to do haha! 😂

// Common use cases / concepts for closures, as covered in articles like:
// https://medium.com/@dis_is_patrick/practical-uses-for-closures-c65640ae7304
// https://stackoverflow.com/questions/2728278/what-is-a-practical-use-for-a-closure-in-javascript
// https://softwareengineering.stackexchange.com/questions/285941/why-would-a-program-use-a-closure
// https://medium.com/@prashantramnyc/what-is-an-iife-in-javascript-24baf0febf08

// Currently, IIFEs and simple Function Factories work
// But my advanced Function Factory Pub Sub, and weird function namespacing does not
// Due to runtime and/or compilation errors :)

// IIFE (Immediately Invoked function expressions)
// Used for encapsulating data usually

// Simple IIFE
let myData = ((): boolean => {
  return true;
})();

assert(myData == true)

// Constructor IIFE?
// Don't know why someone wouldn't just use their class, but yeah

class IIFEReturn {
  myBool: boolean
  myFunc: (x: i32) => i32
}

let myInstanceThing = ((): IIFEReturn => {
  return {
    myBool: true,
    myFunc: (x: i32) => {
      return x + 1;
    }
  }
})();

assert(myInstanceThing.myBool == true);
assert(myInstanceThing.myFunc(24) == 25);

// Function Factories
// Closures that create specific functions

// Simple function that will change depending on input
type generatedFunc = () => i32;
let myFactory = (x: i32): generatedFunc => {
  let myFunc = (): i32 => {
    return 24 + x;
  }

  return myFunc;
}

let generatedPlusOne: generatedFunc = myFactory(1);
let generatedPlusTwo: generatedFunc = myFactory(2);

// For some reason, couldn't do
// Cannot invoke an expression whose type lacks a call signature. Type 'closure-common-js-patterns/myFactory' has no compatible call signatures.
// assert(myFactory(1)() == 25);
assert(generatedPlusOne() == 25);
assert(generatedPlusTwo() == 26);

// I often will use this for like Pub/Sub stuff

// Commenting all of this out, as it doesn't work yet due to runtime, or compilation errors :)

/*
type SubFunc = () => void;
type UnSubFunc = () => void;
let subscriptions = new Array<SubFunc>();
let globalSubVar: i32 = 0;

function subscribe(funcToCallOnPub: SubFunc): UnSubFunc {
  subscriptions.push(funcToCallOnPub);
  return (): void => {
    let funcIndex = subscriptions.indexOf(funcToCallOnPub);
    subscriptions.splice(funcIndex, 1);
  }
}

function publish(): void {
  for(let i = 0; i < subscriptions.length; i++) {
    // Can't call directly? Get a Type error
    // ERROR TS2757: Type '() => void' has no call signatures.
    // Noticed some other weird type errors if I don't declare the function type
    // But I also am meh at typescripte signatures haha!
    // subscriptions[i]();

    let subFunc = subscriptions[i];
    subFunc();
  }
}

let plusOne = (): void => {
  globalSubVar += 1;
}

let plusTwo = (): void => {
  globalSubVar += 1;
}


let unsubPlusOne: () => void = subscribe(plusOne);
let unsubPlusTwo: () => void = subscribe(plusTwo);

assert(globalSubVar == 0);
assert(subscriptions.length == 2);

publish();

assert(globalSubVar == 3);
assert(subscriptions.length == 2);

unsubPlusOne();

assert(globalSubVar == 3);
assert(subscriptions.length == 1);

publish();

assert(globalSubVar == 5);
assert(subscriptions.length == 1);

unsubPlusTwo();

assert(globalSubVar == 5);
assert(subscriptions.length == 0);

publish();

assert(globalSubVar == 5);
assert(subscriptions.length == 0);

// Namespacing private functions
// Again, kind of weird, they should probably just make a class, but it's another interesting test

class Chunk {
  totalSize: i32;
  usedSize: i32;
  write: (size: i32) => void;
}

let getChunk = (): Chunk => {
  let chunk: Chunk = {
    totalSize: 1024,
    usedSize: 0,
    write: (x: i32): void => {}
  };
  
  let growChunk = (): void => {
    chunk.totalSize += 1024;
  }

  let allocateForChunk = (amount: i32): void => {
    if (chunk.usedSize + amount <= chunk.totalSize) {
      chunk.usedSize += amount;
    } else {
      // growChunk(chunk);
      // allocateForChunk(chunk, amount);
    }
  }

  chunk.write = (x: i32) => {
    allocateForChunk(x);
  }

  return chunk;
  
}

let myChunk: Chunk = getChunk();

assert(myChunk.totalSize == 1024);
assert(myChunk.usedSize == 0);

myChunk.write(1025);

assert(myChunk.totalSize == 2048);
assert(myChunk.usedSize == 1025);
*/

[DuncanUszkay1]

Appreciate the additional test cases!

As for this issue:

// Cannot invoke an expression whose type lacks a call signature. Type 'closure-common-js-patterns/myFactory' has no compatible call signatures.
// assert(myFactory(1)() == 25);

This appears to not be limited to closures, but a problem with calling functions in general which is present on master. I've cut a ticket.

[DuncanUszkay1]

Isolated a problem from one of your test cases, made a minimal repro:

class Foo {
  field: i32;
}

var compClosuresSameScope = (): void => {
  var foo: Foo = {
    field: 8
  }
  var withFoo1 = (): void => {
    foo.field += 1;
  }

  var withFoo2 = (): void => {
    withFoo1();
  }
}

I think the problem here is that the field being closed over (foo) is still in scope when calling withFoo1 within withFoo2. It tries to read in the value of foo in from a local variable, but it's assuming we're still in compClosuresSameScope not withFoo2, which has a separate set of locals. Need to think a bit on how to resolve this.

Update: I've written out a fix for this locally, we just need to lookup the locals we're injecting/calling rather than assuming that they're present locally and load them from context as necessary. I think given that this is fixable, it might make sense to just block this with unimplemented for now and list it as a limitation we can fix later in interest of limiting the diff

[torch2424]

@DuncanUszkay1 Thanks for the invite to your repo! Let me know if you want me to open a PR with my test file 😄

This appears to not be limited to closures, but a problem with calling functions in general which is present on master. I've cut a ticket.

Awesome! Thanks for opening that issue! Agree that it probably shouldn't be in this PR if it's a general function calling issue 😄

I think the problem here is that the field being closed over (foo) is still in scope when calling withFoo1 within withFoo2. It tries to read in the value of foo in from a local variable, but it's assuming we're still in compClosuresSameScope not withFoo2, which has a separate set of locals. Need to think a bit on how to resolve this. Update: I've written out a fix for this locally, we just need to lookup the locals we're injecting/calling rather than assuming that they're present locally and load them from context as necessary. I think given that this is fixable, it might make sense to just block this with unimplemented for now and list it as a limitation we can fix later in interest of limiting the diff.

Thanks for isolating that! 😄 I appreciate you taking the time to do that. Agreed, anything that isn't a fundamental design thing like we discussed in the meeting, I think it would be cool to just open a ticket for, that way we can start playing with closures, and keep the diff small(er). Since it seems like you already know the fix, I'm sure we could do a patch after this lands as well easily 😄

---

So! Honestly, that was all the tests I could think of. I actually tend not to use closures much outside of the tests that are already written, and the patterns I showed in my test. Let me know what I can help with next, would be more than happy to keep helping here 😄

[DuncanUszkay1]

Added in a warning flag to indicate that closure support is experimental:

WARNING TS6190: Closure support is experimental.

     return ((): i32 => { return arg + foo + bar; } )();
             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   in closure.ts(41,11)

[dcodeIO]

Looking at the bootstrap test, there appear to be these two new warnings:

WARNING AS201: Conversion from type 'usize' to 'u32' will require an explicit cast when switching between 32/64-bit.

         local.closureContextOffset
         ~~~~~~~~~~~~~~~~~~~~~~~~~~
 in src/compiler.ts(6457,9)

WARNING AS201: Conversion from type 'usize' to 'u32' will require an explicit cast when switching between 32/64-bit.

             localClosureContextOffset
             ~~~~~~~~~~~~~~~~~~~~~~~~~
 in src/compiler.ts(8480,13)

[DuncanUszkay1]

Warnings should be resolved now. Anything else before we fork @dcodeIO ?

[MaxGraey]

@DuncanUszkay1 We seem to have talked about this. What about replacing marking ~nonClosure suffixes to ~closure for closure functions?

[DuncanUszkay1]

I don't think we really came to a conclusion on the recompilation step, but I suppose I may as well throw that in for now

[dcodeIO]

Can we close this PR in favor of the more recent #1308 covering the official beta branch?